From d99fb24b820231e24a1a5635b717fcf5af3f679a Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Wed, 3 Apr 2024 11:38:22 -0400 Subject: [PATCH] fix: Create API usage notification butter bar (#3698) --- api/organisations/models.py | 6 +- api/organisations/permissions/permissions.py | 12 +- api/organisations/serializers.py | 6 + api/organisations/urls.py | 10 +- api/organisations/views.py | 30 +++++ .../test_unit_organisations_models.py | 41 +++++- .../test_unit_organisations_views.py | 118 ++++++++++++++++++ 7 files changed, 219 insertions(+), 4 deletions(-) diff --git a/api/organisations/models.py b/api/organisations/models.py index e9967161d83c..bb377ae1ea07 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -424,7 +424,7 @@ class Meta: ordering = ("id",) # explicit ordering to prevent pagination warnings -class OrganisationSubscriptionInformationCache(models.Model): +class OrganisationSubscriptionInformationCache(LifecycleModelMixin, models.Model): """ Model to hold a cache of an organisation's API usage and their Chargebee plan limits. """ @@ -450,6 +450,10 @@ class OrganisationSubscriptionInformationCache(models.Model): chargebee_email = models.EmailField(blank=True, max_length=254, null=True) + @hook(AFTER_SAVE, when="allowed_30d_api_calls", has_changed=True) + def erase_api_notifications(self): + self.organisation.api_usage_notifications.all().delete() + class OranisationAPIUsageNotification(models.Model): organisation = models.ForeignKey( diff --git a/api/organisations/permissions/permissions.py b/api/organisations/permissions/permissions.py index 82e42bf32420..be39851d669c 100644 --- a/api/organisations/permissions/permissions.py +++ b/api/organisations/permissions/permissions.py @@ -6,7 +6,8 @@ from django.db.models import Model from django.views import View from rest_framework.exceptions import PermissionDenied -from rest_framework.permissions import BasePermission +from rest_framework.permissions import BasePermission, IsAuthenticated +from rest_framework.request import Request from organisations.models import Organisation @@ -173,3 +174,12 @@ def has_object_permission(self, request, view, obj): return request.user.is_organisation_admin( self.get_organisation_from_object_callable(obj) ) + + +class OrganisationAPIUsageNotificationPermission(IsAuthenticated): + def has_permission(self, request: Request, view: View) -> bool: + if not super().has_permission(request, view): + return False + + # All organisation users can see api usage notifications. + return request.user.belongs_to(view.kwargs.get("organisation_pk")) diff --git a/api/organisations/serializers.py b/api/organisations/serializers.py index 2bcbdbc93e06..a51bf55641b8 100644 --- a/api/organisations/serializers.py +++ b/api/organisations/serializers.py @@ -246,3 +246,9 @@ class SubscriptionDetailsSerializer(serializers.Serializer): payment_source = serializers.ChoiceField(choices=[None, CHARGEBEE], allow_null=True) chargebee_email = serializers.EmailField() + + +class OrganisationAPIUsageNotificationSerializer(serializers.Serializer): + organisation_id = serializers.IntegerField() + percent_usage = serializers.IntegerField() + notified_at = serializers.DateTimeField() diff --git a/api/organisations/urls.py b/api/organisations/urls.py index 1640493483b6..d83e07c3e5dc 100644 --- a/api/organisations/urls.py +++ b/api/organisations/urls.py @@ -10,7 +10,10 @@ from api_keys.views import MasterAPIKeyViewSet from audit.views import OrganisationAuditLogViewSet from metadata.views import MetaDataModelFieldViewSet -from organisations.views import OrganisationWebhookViewSet +from organisations.views import ( + OrganisationAPIUsageNotificationView, + OrganisationWebhookViewSet, +) from users.views import ( FFAdminUserViewSet, UserPermissionGroupViewSet, @@ -93,6 +96,11 @@ remove_user_as_group_admin, name="remove-user-group-admin", ), + path( + "/api-usage-notification/", + OrganisationAPIUsageNotificationView.as_view(), + name="organisation-api-usage-notification", + ), ] if settings.IS_RBAC_INSTALLED: diff --git a/api/organisations/views.py b/api/organisations/views.py index b5db5a7d96b9..bd3faf9e2efe 100644 --- a/api/organisations/views.py +++ b/api/organisations/views.py @@ -8,11 +8,14 @@ get_multiple_event_list_for_organisation, ) from core.helpers import get_current_site_url +from dateutil.relativedelta import relativedelta +from django.utils import timezone from drf_yasg.utils import swagger_auto_schema from rest_framework import status, viewsets from rest_framework.authentication import BasicAuthentication from rest_framework.decorators import action, api_view, authentication_classes from rest_framework.exceptions import ValidationError +from rest_framework.generics import ListAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response @@ -21,6 +24,7 @@ from organisations.chargebee import webhook_event_types, webhook_handlers from organisations.exceptions import OrganisationHasNoPaidSubscription from organisations.models import ( + OranisationAPIUsageNotification, Organisation, OrganisationRole, OrganisationWebhook, @@ -28,6 +32,7 @@ from organisations.permissions.models import OrganisationPermissionModel from organisations.permissions.permissions import ( NestedOrganisationEntityPermission, + OrganisationAPIUsageNotificationPermission, OrganisationPermission, ) from organisations.serializers import ( @@ -51,6 +56,8 @@ from webhooks.mixins import TriggerSampleWebhookMixin from webhooks.webhooks import WebhookType +from .serializers import OrganisationAPIUsageNotificationSerializer + logger = logging.getLogger(__name__) @@ -303,3 +310,26 @@ def perform_update(self, serializer): def perform_create(self, serializer): organisation_id = self.kwargs["organisation_pk"] serializer.save(organisation_id=organisation_id) + + +class OrganisationAPIUsageNotificationView(ListAPIView): + serializer_class = OrganisationAPIUsageNotificationSerializer + permission_classes = [OrganisationAPIUsageNotificationPermission] + + def get_queryset(self): + organisation = Organisation.objects.get(id=self.kwargs["organisation_pk"]) + if not hasattr(organisation, "subscription_information_cache"): + return OranisationAPIUsageNotification.objects.none() + subscription_cache = organisation.subscription_information_cache + billing_starts_at = subscription_cache.current_billing_term_starts_at + now = timezone.now() + + month_delta = relativedelta(now, billing_starts_at).months + period_starts_at = relativedelta(months=month_delta) + billing_starts_at + + queryset = OranisationAPIUsageNotification.objects.filter( + organisation_id=organisation.id, + notified_at__gt=period_starts_at, + ) + + return queryset.order_by("-percent_usage")[:1] diff --git a/api/tests/unit/organisations/test_unit_organisations_models.py b/api/tests/unit/organisations/test_unit_organisations_models.py index 04ce31694726..a19b4ad85fac 100644 --- a/api/tests/unit/organisations/test_unit_organisations_models.py +++ b/api/tests/unit/organisations/test_unit_organisations_models.py @@ -1,14 +1,16 @@ -from datetime import datetime +from datetime import datetime, timedelta from unittest import mock import pytest from django.conf import settings +from django.utils import timezone from pytest_mock import MockerFixture from rest_framework.test import override_settings from environments.models import Environment from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.models import ( + OranisationAPIUsageNotification, Organisation, OrganisationSubscriptionInformationCache, Subscription, @@ -533,3 +535,40 @@ def test_organisation_subscription_get_api_call_overage( # Then assert overage == expected_overage + + +def test_reset_of_api_notifications(organisation: Organisation) -> None: + # Given + now = timezone.now() + osic = OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + allowed_seats=10, + allowed_projects=3, + allowed_30d_api_calls=100, + chargebee_email="test@example.com", + current_billing_term_starts_at=now - timedelta(days=45), + current_billing_term_ends_at=now + timedelta(days=320), + ) + + # Create a notification which should be deleted shortly. + OranisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=90, + notified_at=now, + ) + + # Keep a notification which should not be deleted. + organisation2 = Organisation.objects.create(name="Test org2") + oapiun = OranisationAPIUsageNotification.objects.create( + organisation=organisation2, + percent_usage=90, + notified_at=now, + ) + + # When + osic.allowed_30d_api_calls *= 2 + osic.save() + + # Then + assert OranisationAPIUsageNotification.objects.count() == 1 + assert OranisationAPIUsageNotification.objects.first() == oapiun diff --git a/api/tests/unit/organisations/test_unit_organisations_views.py b/api/tests/unit/organisations/test_unit_organisations_views.py index 43390940bd6a..87c9a27729cb 100644 --- a/api/tests/unit/organisations/test_unit_organisations_views.py +++ b/api/tests/unit/organisations/test_unit_organisations_views.py @@ -25,6 +25,7 @@ from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.invites.models import Invite from organisations.models import ( + OranisationAPIUsageNotification, Organisation, OrganisationRole, OrganisationSubscriptionInformationCache, @@ -1679,3 +1680,120 @@ def test_user_from_another_organisation_cannot_list_group_summaries( # Then assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_defaults_to_empty_api_notifications_when_no_subscription_information_cache( + staff_client: APIClient, + organisation: Organisation, +) -> None: + # Given + url = reverse( + "api-v1:organisations:organisation-api-usage-notification", + args=[organisation.id], + ) + + now = timezone.now() + OranisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=90, + notified_at=now, + ) + + assert hasattr(organisation, "subscription_information_cache") is False + + # When + response = staff_client.get(url) + + # Then + # There are no results even if there is a notification because + # the information cache can't provide an estimate as to API usage. + assert response.status_code == status.HTTP_200_OK + assert response.data["results"] == [] + + +@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00") +def test_retrieves_api_usage_notifications( + staff_client: APIClient, + organisation: Organisation, +) -> None: + # Given + url = reverse( + "api-v1:organisations:organisation-api-usage-notification", + args=[organisation.id], + ) + + now = timezone.now() + OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + allowed_seats=10, + allowed_projects=3, + allowed_30d_api_calls=100, + chargebee_email="test@example.com", + current_billing_term_starts_at=now - timedelta(days=45), + current_billing_term_ends_at=now + timedelta(days=320), + ) + + # Add three notifications, but we only get the 100% one. + OranisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=90, + notified_at=now, + ) + OranisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=75, + notified_at=now, + ) + OranisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=100, + notified_at=now, + ) + + # When + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + + assert len(response.data["results"]) == 1 + assert response.data["results"][0]["notified_at"] == "2023-01-19T09:09:47.325132Z" + assert response.data["results"][0]["organisation_id"] == organisation.id + assert response.data["results"][0]["percent_usage"] == 100 + + +@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00") +def test_doesnt_retrieve_stale_api_usage_notifications( + staff_client: APIClient, + organisation: Organisation, +) -> None: + # Given + url = reverse( + "api-v1:organisations:organisation-api-usage-notification", + args=[organisation.id], + ) + + now = timezone.now() + OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + allowed_seats=10, + allowed_projects=3, + allowed_30d_api_calls=100, + chargebee_email="test@example.com", + current_billing_term_starts_at=now - timedelta(days=45), + current_billing_term_ends_at=now + timedelta(days=320), + ) + + # Create a notification in the past which should not be shown. + OranisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=90, + notified_at=now - timedelta(20), + ) + + # When + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 0