diff --git a/api/conftest.py b/api/conftest.py index a799dfbf43d0..7a6f1726564a 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -1,3 +1,4 @@ +import logging import os import typing from unittest.mock import MagicMock @@ -1072,3 +1073,20 @@ def superuser(): def superuser_client(superuser: FFAdminUser, client: APIClient): client.force_login(superuser, backend="django.contrib.auth.backends.ModelBackend") return client + + +@pytest.fixture +def inspecting_handler() -> logging.Handler: + """ + Fixture used to test the output of logger related output. + """ + + class InspectingHandler(logging.Handler): + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.messages = [] + + def handle(self, record: logging.LogRecord) -> None: + self.messages.append(self.format(record)) + + return InspectingHandler() diff --git a/api/organisations/tasks.py b/api/organisations/tasks.py index 3199311383b7..f6e4b7bc579f 100644 --- a/api/organisations/tasks.py +++ b/api/organisations/tasks.py @@ -52,7 +52,7 @@ @register_task_handler() -def send_org_over_limit_alert(organisation_id) -> None: +def send_org_over_limit_alert(organisation_id: int) -> None: organisation = Organisation.objects.get(id=organisation_id) subscription_metadata = get_subscription_metadata(organisation) @@ -150,18 +150,34 @@ def send_admin_api_usage_notification( def _handle_api_usage_notifications(organisation: Organisation) -> None: - subscription_cache = organisation.subscription_information_cache - billing_starts_at = subscription_cache.current_billing_term_starts_at now = timezone.now() - # Truncate to the closest active month to get start of current period. - month_delta = relativedelta(now, billing_starts_at).months - period_starts_at = relativedelta(months=month_delta) + billing_starts_at + if organisation.subscription.is_free_plan: + allowed_api_calls = organisation.subscription.max_api_calls + # Default to a rolling month for free accounts + days = 30 + period_starts_at = now - timedelta(days) + elif not organisation.has_subscription_information_cache(): + # Since the calling code is a list of many organisations + # log the error and return without raising an exception. + logger.error( + f"Paid organisation {organisation.id} is missing subscription information cache" + ) + return + else: + subscription_cache = organisation.subscription_information_cache + billing_starts_at = subscription_cache.current_billing_term_starts_at + + # Truncate to the closest active month to get start of current period. + month_delta = relativedelta(now, billing_starts_at).months + period_starts_at = relativedelta(months=month_delta) + billing_starts_at + + days = relativedelta(now, period_starts_at).days + allowed_api_calls = subscription_cache.allowed_30d_api_calls - days = relativedelta(now, period_starts_at).days api_usage = get_current_api_usage(organisation.id, f"-{days}d") - api_usage_percent = int(100 * api_usage / subscription_cache.allowed_30d_api_calls) + api_usage_percent = int(100 * api_usage / allowed_api_calls) matched_threshold = None for threshold in API_USAGE_ALERT_THRESHOLDS: @@ -176,7 +192,7 @@ def _handle_api_usage_notifications(organisation: Organisation) -> None: if OrganisationAPIUsageNotification.objects.filter( notified_at__gt=period_starts_at, - percent_usage=matched_threshold, + percent_usage__gte=matched_threshold, ).exists(): # Already sent the max notification level so don't resend. return @@ -187,10 +203,7 @@ def _handle_api_usage_notifications(organisation: Organisation) -> None: def handle_api_usage_notifications() -> None: flagsmith_client = get_client("local", local_eval=True) - for organisation in Organisation.objects.filter( - subscription_information_cache__current_billing_term_starts_at__isnull=False, - subscription_information_cache__current_billing_term_ends_at__isnull=False, - ).select_related( + for organisation in Organisation.objects.all().select_related( "subscription_information_cache", ): feature_enabled = flagsmith_client.get_identity_flags( @@ -234,21 +247,27 @@ def charge_for_api_call_count_overages(): ).values_list("organisation_id", flat=True) ) - for organisation in Organisation.objects.filter( - id__in=organisation_ids, - subscription_information_cache__current_billing_term_ends_at__lte=closing_billing_term, - subscription_information_cache__current_billing_term_ends_at__gte=now, - subscription_information_cache__current_billing_term_starts_at__lte=F( - "subscription_information_cache__current_billing_term_ends_at" + for organisation in ( + Organisation.objects.filter( + id__in=organisation_ids, + subscription_information_cache__current_billing_term_ends_at__lte=closing_billing_term, + subscription_information_cache__current_billing_term_ends_at__gte=now, + subscription_information_cache__current_billing_term_starts_at__lte=F( + "subscription_information_cache__current_billing_term_ends_at" + ) + - month_window_start, + subscription_information_cache__current_billing_term_starts_at__gte=F( + "subscription_information_cache__current_billing_term_ends_at" + ) + - month_window_end, ) - - month_window_start, - subscription_information_cache__current_billing_term_starts_at__gte=F( - "subscription_information_cache__current_billing_term_ends_at" + .exclude( + subscription__plan=FREE_PLAN_ID, + ) + .select_related( + "subscription_information_cache", + "subscription", ) - - month_window_end, - ).select_related( - "subscription_information_cache", - "subscription", ): subscription_cache = organisation.subscription_information_cache api_usage = get_current_api_usage(organisation.id, "30d") diff --git a/api/organisations/templates/organisations/api_usage_notification.html b/api/organisations/templates/organisations/api_usage_notification.html index e23d4d5f037e..d82eaaea38c9 100644 --- a/api/organisations/templates/organisations/api_usage_notification.html +++ b/api/organisations/templates/organisations/api_usage_notification.html @@ -11,7 +11,7 @@ The API usage for {{ organisation.name }} has reached {{ matched_threshold }}% within the current subscription period. - Please consider upgrading your organisations account limits. + Please consider upgrading your organisation's account limits. diff --git a/api/organisations/templates/organisations/api_usage_notification.txt b/api/organisations/templates/organisations/api_usage_notification.txt index b13042132ea6..cce88fcce54e 100644 --- a/api/organisations/templates/organisations/api_usage_notification.txt +++ b/api/organisations/templates/organisations/api_usage_notification.txt @@ -1,6 +1,6 @@ Hi there, -The API usage for {{ organisation.name }} has reached {{ matched_threshold }}% within the current subscription period. Please consider upgrading your organisations account limits. +The API usage for {{ organisation.name }} has reached {{ matched_threshold }}% within the current subscription period. Please consider upgrading your organisation's account limits. Thank you! diff --git a/api/organisations/templates/organisations/api_usage_notification_limit.html b/api/organisations/templates/organisations/api_usage_notification_limit.html index 3e9c1ae577cf..b5f25ebb1f94 100644 --- a/api/organisations/templates/organisations/api_usage_notification_limit.html +++ b/api/organisations/templates/organisations/api_usage_notification_limit.html @@ -11,7 +11,7 @@ The API usage for {{ organisation.name }} has breached {{ matched_threshold }}% within the current subscription period. - Please upgrade your organisations account to ensure continued service. + Please upgrade your organisation's account to ensure continued service. diff --git a/api/organisations/templates/organisations/api_usage_notification_limit.txt b/api/organisations/templates/organisations/api_usage_notification_limit.txt index 4e39e7adc54a..11a0816bca4c 100644 --- a/api/organisations/templates/organisations/api_usage_notification_limit.txt +++ b/api/organisations/templates/organisations/api_usage_notification_limit.txt @@ -1,6 +1,6 @@ Hi there, -The API usage for {{ organisation.name }} has breached {{ matched_threshold }}% within the current subscription period. Please upgrade your organisations account to ensure continued service. +The API usage for {{ organisation.name }} has breached {{ matched_threshold }}% within the current subscription period. Please upgrade your organisation's account to ensure continued service. Thank you! diff --git a/api/tests/unit/organisations/test_unit_organisations_tasks.py b/api/tests/unit/organisations/test_unit_organisations_tasks.py index 7d729684ead5..f50009037126 100644 --- a/api/tests/unit/organisations/test_unit_organisations_tasks.py +++ b/api/tests/unit/organisations/test_unit_organisations_tasks.py @@ -1,3 +1,4 @@ +import logging import uuid from datetime import timedelta from unittest.mock import MagicMock, call @@ -25,6 +26,7 @@ FREE_PLAN_ID, MAX_API_CALLS_IN_FREE_PLAN, MAX_SEATS_IN_FREE_PLAN, + SCALE_UP, ) from organisations.subscriptions.xero.metadata import XeroSubscriptionMetadata from organisations.tasks import ( @@ -290,6 +292,8 @@ def test_handle_api_usage_notifications_below_100( ) -> None: # Given now = timezone.now() + organisation.subscription.plan = SCALE_UP + organisation.subscription.save() OrganisationSubscriptionInformationCache.objects.create( organisation=organisation, allowed_seats=10, @@ -324,7 +328,7 @@ def test_handle_api_usage_notifications_below_100( assert email.body == ( "Hi there,\n\nThe API usage for Test Org has reached " "90% within the current subscription period. Please " - "consider upgrading your organisations account limits.\n\n" + "consider upgrading your organisation's account limits.\n\n" "Thank you!\n\nThe Flagsmith Team\n" ) @@ -338,7 +342,7 @@ def test_handle_api_usage_notifications_below_100( "\n\n \n " "The API usage for Test Org has reached\n " "90% within the current subscription period.\n " - "Please consider upgrading your organisations account limits.\n" + "Please consider upgrading your organisation's account limits.\n" " \n\n\n \n\n " "\n\n Thank you!\n\n " " \n\n \n\n " @@ -382,6 +386,8 @@ def test_handle_api_usage_notifications_above_100( ) -> None: # Given now = timezone.now() + organisation.subscription.plan = SCALE_UP + organisation.subscription.save() OrganisationSubscriptionInformationCache.objects.create( organisation=organisation, allowed_seats=10, @@ -417,7 +423,95 @@ def test_handle_api_usage_notifications_above_100( assert email.body == ( "Hi there,\n\nThe API usage for Test Org has breached " "100% within the current subscription period. Please " - "upgrade your organisations account to ensure " + "upgrade your organisation's account to ensure " + "continued service.\n\nThank you!\n\n" + "The Flagsmith Team\n" + ) + + assert len(email.alternatives) == 1 + assert len(email.alternatives[0]) == 2 + assert email.alternatives[0][1] == "text/html" + + assert email.alternatives[0][0] == ( + "\n\n \n\n \n\n \n\n \n\n " + " \n\n\n " + " \n\n \n\n \n\n \n\n \n\n" + " \n\n " + "\n\n
Hi " + "there,
\n The API usage for Test Org " + "has breached\n 100% within the " + "current subscription period.\n " + "Please upgrade your organisation's account to ensure " + "continued service.\n
" + "Thank you!
The Flagsmith Team
\n" + ) + + assert email.from_email == "noreply@flagsmith.com" + # Extra staff included because threshold is over 100. + assert email.to == ["admin@example.com", "staff@example.com"] + + assert ( + OrganisationAPIUsageNotification.objects.filter( + organisation=organisation, + ).count() + == 1 + ) + api_usage_notification = OrganisationAPIUsageNotification.objects.filter( + organisation=organisation, + ).first() + + assert api_usage_notification.percent_usage == 100 + + # Now re-run the usage to make sure the notification isn't resent. + handle_api_usage_notifications() + + assert ( + OrganisationAPIUsageNotification.objects.filter( + organisation=organisation, + ).count() + == 1 + ) + + assert OrganisationAPIUsageNotification.objects.first() == api_usage_notification + + +@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00") +def test_handle_api_usage_notifications_for_free_accounts( + mocker: MockerFixture, + organisation: Organisation, + mailoutbox: list[EmailMultiAlternatives], +) -> None: + # Given + assert organisation.subscription.is_free_plan + assert organisation.subscription.max_api_calls == MAX_API_CALLS_IN_FREE_PLAN + + mock_api_usage = mocker.patch( + "organisations.tasks.get_current_api_usage", + ) + mock_api_usage.return_value = MAX_API_CALLS_IN_FREE_PLAN + 5_000 + + get_client_mock = mocker.patch("organisations.tasks.get_client") + client_mock = MagicMock() + get_client_mock.return_value = client_mock + client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + + assert not OrganisationAPIUsageNotification.objects.filter( + organisation=organisation, + ).exists() + + # When + handle_api_usage_notifications() + + # Then + mock_api_usage.assert_called_once_with(organisation.id, "-30d") + + assert len(mailoutbox) == 1 + email = mailoutbox[0] + assert email.subject == "Flagsmith API use has reached 100%" + assert email.body == ( + "Hi there,\n\nThe API usage for Test Org has breached " + "100% within the current subscription period. Please " + "upgrade your organisation's account to ensure " "continued service.\n\nThank you!\n\n" "The Flagsmith Team\n" ) @@ -432,7 +526,7 @@ def test_handle_api_usage_notifications_above_100( " \n The API usage for Test Org " "has breached\n 100% within the " "current subscription period.\n " - "Please upgrade your organisations account to ensure " + "Please upgrade your organisation's account to ensure " "continued service.\n \n\n\n " " \n\n \n\n " "Thank you!\n\n \n\n \n\n" @@ -469,6 +563,51 @@ def test_handle_api_usage_notifications_above_100( assert OrganisationAPIUsageNotification.objects.first() == api_usage_notification +def test_handle_api_usage_notifications_missing_info_cache( + mocker: MockerFixture, + organisation: Organisation, + mailoutbox: list[EmailMultiAlternatives], + inspecting_handler: logging.Handler, +) -> None: + # Given + organisation.subscription.plan = SCALE_UP + organisation.subscription.save() + + from organisations.tasks import logger + + logger.addHandler(inspecting_handler) + + assert organisation.has_subscription_information_cache() is False + + mock_api_usage = mocker.patch( + "organisations.tasks.get_current_api_usage", + ) + + get_client_mock = mocker.patch("organisations.tasks.get_client") + client_mock = MagicMock() + get_client_mock.return_value = client_mock + client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + + assert not OrganisationAPIUsageNotification.objects.filter( + organisation=organisation, + ).exists() + + # When + handle_api_usage_notifications() + + # Then + mock_api_usage.assert_not_called() + + assert len(mailoutbox) == 0 + assert not OrganisationAPIUsageNotification.objects.filter( + organisation=organisation, + ).exists() + + assert inspecting_handler.messages == [ + f"Paid organisation {organisation.id} is missing subscription information cache" + ] + + @pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00") def test_charge_for_api_call_count_overages_scale_up( organisation: Organisation, diff --git a/api/tests/unit/util/test_logging.py b/api/tests/unit/util/test_logging.py index d8c31a19f9f3..33f34b277a23 100644 --- a/api/tests/unit/util/test_logging.py +++ b/api/tests/unit/util/test_logging.py @@ -7,19 +7,6 @@ from util.logging import JsonFormatter -@pytest.fixture -def inspecting_handler() -> logging.Handler: - class InspectingHandler(logging.Handler): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.messages = [] - - def handle(self, record): - self.messages.append(self.format(record)) - - return InspectingHandler() - - @pytest.mark.freeze_time("2023-12-08T06:05:47.320000+00:00") def test_json_formatter__outputs_expected( inspecting_handler: logging.Handler, @@ -38,7 +25,7 @@ def test_json_formatter__outputs_expected( expected_tb_string = ( "Traceback (most recent call last):\n" f' File "{expected_module_path}",' - " line 47, in _log_traceback\n" + " line 34, in _log_traceback\n" " raise Exception()\nException" )