fix: Include free plans for api use notifications (#4204)

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()

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")

api/organisations/templates/organisations/api_usage_notification.html

@@ -11,7 +11,7 @@
                <td>
                  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.
                </td>
 
 

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!
 

api/organisations/templates/organisations/api_usage_notification_limit.html

@@ -11,7 +11,7 @@
                <td>
                  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.
                </td>
 
 

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!
 

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(
         "<tr>\n\n               <td>\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"
         "               </td>\n\n\n        </tr>\n\n        "
         "<tr>\n\n               <td>Thank you!</td>\n\n      "
         "  </tr>\n\n        <tr>\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] == (
+        "<table>\n\n        <tr>\n\n               <td>Hi "
+        "there,</td>\n\n        </tr>\n\n        <tr>\n\n    "
+        "           <td>\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               </td>\n\n\n      "
+        "  </tr>\n\n        <tr>\n\n               <td>"
+        "Thank you!</td>\n\n        </tr>\n\n        <tr>\n\n"
+        "               <td>The Flagsmith Team</td>\n\n        "
+        "</tr>\n\n</table>\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(
         "           <td>\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               </td>\n\n\n      "
         "  </tr>\n\n        <tr>\n\n               <td>"
         "Thank you!</td>\n\n        </tr>\n\n        <tr>\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,