From fbf7294909af5a294482c9ed40c387af5af56e2a Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 11 Sep 2026 17:32:42 -0300 Subject: [PATCH 1/4] fix(usage): count whole years when resolving the billing period relativedelta(...).months returns the month component alone, so a term that began over a year ago resolved to the same month a year early. A term starting September 2024 viewed today opened the current period in September 2025, which is the date range reported in #6099. The arithmetic was repeated at four call sites and wrong at two. It now lives in one place. Closes #6099 Co-Authored-By: Claude Opus 5 (1M context) --- api/app_analytics/analytics_db_service.py | 17 ++++--- api/organisations/billing_periods.py | 20 ++++++++ api/organisations/models.py | 4 +- api/organisations/task_helpers.py | 10 +--- api/organisations/views.py | 5 +- .../test_analytics_db_service.py | 38 ++++++++++++++ ...test_unit_organisations_billing_periods.py | 51 +++++++++++++++++++ .../test_unit_organisations_views.py | 44 ++++++++++++++++ 8 files changed, 168 insertions(+), 21 deletions(-) create mode 100644 api/organisations/billing_periods.py create mode 100644 api/tests/unit/organisations/test_unit_organisations_billing_periods.py diff --git a/api/app_analytics/analytics_db_service.py b/api/app_analytics/analytics_db_service.py index dd5f336daf1a..8032ee3a303e 100644 --- a/api/app_analytics/analytics_db_service.py +++ b/api/app_analytics/analytics_db_service.py @@ -28,6 +28,7 @@ from app_analytics.types import Labels, PeriodType from environments.models import Environment from features.models import Feature +from organisations.billing_periods import months_elapsed, period_start from organisations.models import Organisation, OrganisationSubscriptionInformationCache logger = structlog.get_logger("app_analytics") @@ -341,9 +342,7 @@ def _get_start_date_and_stop_date_for_subscribed_organisation( else: raise NotFound("No billing periods found for this organisation.") - month_delta = relativedelta(now, starts_at).months - date_start = relativedelta(months=month_delta) + starts_at - return date_start, now + return period_start(starts_at, now), now case constants.PREVIOUS_BILLING_PERIOD: if sub_cache and sub_cache.current_billing_term_starts_at: @@ -351,11 +350,13 @@ def _get_start_date_and_stop_date_for_subscribed_organisation( else: raise NotFound("No billing periods found for this organisation.") - month_delta = relativedelta(now, starts_at).months - 1 - month_delta += relativedelta(now, starts_at).years * 12 - date_start = relativedelta(months=month_delta) + starts_at - date_stop = relativedelta(months=month_delta + 1) + starts_at - return date_start, date_stop + # Both ends count from the term start, so a month too short + # for its day does not shift the window. + months = months_elapsed(starts_at, now) + return ( + starts_at + relativedelta(months=months - 1), + starts_at + relativedelta(months=months), + ) case constants.NINETY_DAY_PERIOD: date_start = now - relativedelta(days=90) diff --git a/api/organisations/billing_periods.py b/api/organisations/billing_periods.py new file mode 100644 index 000000000000..2370e3d2da3c --- /dev/null +++ b/api/organisations/billing_periods.py @@ -0,0 +1,20 @@ +from datetime import datetime + +from dateutil.relativedelta import relativedelta + + +def months_elapsed(since: datetime, now: datetime) -> int: + """Whole months between two datetimes, years included.""" + elapsed = relativedelta(now, since) + return elapsed.years * 12 + elapsed.months + + +def period_start(billing_term_starts_at: datetime, now: datetime) -> datetime: + """ + Start of the monthly allowance window a term is currently in. A term can + run longer than a month, so this is the most recent monthly anniversary of + its start. + """ + return billing_term_starts_at + relativedelta( + months=months_elapsed(billing_term_starts_at, now) + ) diff --git a/api/organisations/models.py b/api/organisations/models.py index c58f6907702c..3a1b67ec50ae 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -25,6 +25,7 @@ from integrations.lead_tracking.hubspot.tasks import ( track_hubspot_lead_v2, ) +from organisations.billing_periods import months_elapsed from organisations.chargebee import ( # type: ignore[attr-defined] get_customer_id_from_subscription_id, get_max_api_calls_for_plan, @@ -619,8 +620,7 @@ def get_current_billing_period(self) -> BillingPeriod | None: ) return None - elapsed = relativedelta(now, starts_at) - months = elapsed.years * 12 + elapsed.months + months = months_elapsed(starts_at, now) # Both ends count from the term start; counting the end from the start # of the window loses the original day when a month is too short for it. return BillingPeriod( diff --git a/api/organisations/task_helpers.py b/api/organisations/task_helpers.py index ff918b14d89b..89dba84725d1 100644 --- a/api/organisations/task_helpers.py +++ b/api/organisations/task_helpers.py @@ -1,7 +1,6 @@ from datetime import timedelta import structlog -from dateutil.relativedelta import relativedelta from django.conf import settings from django.core.mail import EmailMultiAlternatives from django.template.loader import render_to_string @@ -11,6 +10,7 @@ from app_analytics.influxdb_wrapper import get_current_api_usage from core.helpers import get_current_site_url from integrations.flagsmith.client import get_openfeature_client +from organisations.billing_periods import period_start from organisations.models import ( Organisation, OrganisationAPIUsageNotification, @@ -125,9 +125,7 @@ def handle_api_usage_notification_for_organisation(organisation: Organisation) - ) return - # Truncate to the closest active month to get start of current period. - month_delta = _get_total_months(relativedelta(now, billing_starts_at)) - period_starts_at = relativedelta(months=month_delta) + billing_starts_at + period_starts_at = period_start(billing_starts_at, now) allowed_api_calls = subscription_cache.allowed_30d_api_calls @@ -184,7 +182,3 @@ def handle_api_usage_notification_for_organisation(organisation: Organisation) - ) _send_api_usage_notification(organisation, matched_threshold) - - -def _get_total_months(rd: relativedelta) -> int: - return rd.months + rd.years * 12 diff --git a/api/organisations/views.py b/api/organisations/views.py index ee6068272a8d..7049dfd49827 100644 --- a/api/organisations/views.py +++ b/api/organisations/views.py @@ -4,7 +4,6 @@ import logging from datetime import timedelta -from dateutil.relativedelta import relativedelta from django.utils import timezone from drf_spectacular.utils import extend_schema, extend_schema_view from rest_framework import status, viewsets @@ -23,6 +22,7 @@ ) from app_analytics.throttles import InfluxQueryThrottle from core.helpers import get_current_site_url +from organisations.billing_periods import period_start from organisations.chargebee import webhook_event_types, webhook_handlers from organisations.exceptions import OrganisationHasNoPaidSubscription from organisations.models import ( @@ -393,8 +393,7 @@ def get_queryset(self): # type: ignore[no-untyped-def] # by defaulting to something as a reasonable default. billing_starts_at = billing_starts_at or now - timedelta(days=30) - month_delta = relativedelta(now, billing_starts_at).months - period_starts_at = relativedelta(months=month_delta) + billing_starts_at + period_starts_at = period_start(billing_starts_at, now) queryset = OrganisationAPIUsageNotification.objects.filter( organisation_id=organisation.id, diff --git a/api/tests/unit/app_analytics/test_analytics_db_service.py b/api/tests/unit/app_analytics/test_analytics_db_service.py index 50321bdcc47a..887a5d6963af 100644 --- a/api/tests/unit/app_analytics/test_analytics_db_service.py +++ b/api/tests/unit/app_analytics/test_analytics_db_service.py @@ -7,6 +7,7 @@ from rest_framework.exceptions import NotFound from app_analytics.analytics_db_service import ( + _get_start_date_and_stop_date_for_subscribed_organisation, get_feature_evaluation_data, get_feature_evaluation_data_from_local_db, get_top_organisations_from_local_db, @@ -966,3 +967,40 @@ def test_get_usage_data_for_window__no_analytics_configured__returns_empty( # Then assert result == [] + + +# A term over a year old resolved to the wrong year before #6099, so the current +# period opened twelve months early and the usage shown was not the period's. +@pytest.mark.freeze_time("2026-09-11T00:00:00+00:00") +@pytest.mark.parametrize( + "period, expected_start", + [ + (CURRENT_BILLING_PERIOD, "2026-09-07T00:00:00+00:00"), + (PREVIOUS_BILLING_PERIOD, "2026-08-07T00:00:00+00:00"), + ], +) +def test_get_start_date_and_stop_date__term_over_a_year_old__counts_the_years( + db: None, + organisation: Organisation, + period: PeriodType, + expected_start: str, +) -> None: + # Given + sub_cache = OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + current_billing_term_starts_at=datetime.fromisoformat( + "2025-08-07T00:00:00+00:00" + ), + current_billing_term_ends_at=datetime.fromisoformat( + "2027-08-07T00:00:00+00:00" + ), + ) + + # When + date_start, _ = _get_start_date_and_stop_date_for_subscribed_organisation( + sub_cache=sub_cache, + period=period, + ) + + # Then + assert date_start == datetime.fromisoformat(expected_start) diff --git a/api/tests/unit/organisations/test_unit_organisations_billing_periods.py b/api/tests/unit/organisations/test_unit_organisations_billing_periods.py new file mode 100644 index 000000000000..e2b61edff993 --- /dev/null +++ b/api/tests/unit/organisations/test_unit_organisations_billing_periods.py @@ -0,0 +1,51 @@ +from datetime import datetime + +import pytest + +from organisations.billing_periods import months_elapsed, period_start + + +@pytest.mark.parametrize( + "since, now, expected", + [ + ("2026-09-01T00:00:00+00:00", "2026-09-10T00:00:00+00:00", 0), + ("2026-01-05T00:00:00+00:00", "2026-09-10T00:00:00+00:00", 8), + # The year is the part #6099 dropped. + ("2024-09-03T00:00:00+00:00", "2026-09-10T00:00:00+00:00", 24), + ("2025-08-07T00:00:00+00:00", "2026-09-11T00:00:00+00:00", 13), + ], +) +def test_months_elapsed__spans_years__counts_them( + since: str, now: str, expected: int +) -> None: + # Given / When / Then + assert ( + months_elapsed(datetime.fromisoformat(since), datetime.fromisoformat(now)) + == expected + ) + + +@pytest.mark.parametrize( + "term_starts_at, now, expected", + [ + # Annual term over a year old. Dropping the year lands in 2025. + ( + "2024-09-03T00:00:00+00:00", + "2026-09-10T00:00:00+00:00", + "2026-09-03T00:00:00+00:00", + ), + # February is too short for a 31st, so the window opens on the 28th. + ( + "2026-01-31T00:00:00+00:00", + "2026-03-01T00:00:00+00:00", + "2026-02-28T00:00:00+00:00", + ), + ], +) +def test_period_start__long_term__opens_on_the_latest_anniversary( + term_starts_at: str, now: str, expected: str +) -> None: + # Given / When / Then + assert period_start( + datetime.fromisoformat(term_starts_at), datetime.fromisoformat(now) + ) == datetime.fromisoformat(expected) diff --git a/api/tests/unit/organisations/test_unit_organisations_views.py b/api/tests/unit/organisations/test_unit_organisations_views.py index 062ea3bba09c..062bf2b9e3fd 100644 --- a/api/tests/unit/organisations/test_unit_organisations_views.py +++ b/api/tests/unit/organisations/test_unit_organisations_views.py @@ -2234,3 +2234,47 @@ def test_get_detailed_permissions__other_user_as_admin__returns_permissions( "derived_from": {"groups": [], "roles": []}, } ] + + +# A term over a year old resolved to the wrong year before #6099, so the window +# opened twelve months early and swept up notifications from previous periods. +@pytest.mark.freeze_time("2026-09-11T00:00:00+00:00") +def test_get_api_usage_notifications__term_over_a_year_old__excludes_earlier_periods( + staff_client: APIClient, + organisation: Organisation, +) -> None: + # Given + now = timezone.now() + OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + current_billing_term_starts_at=datetime.fromisoformat( + "2025-08-07T00:00:00+00:00" + ), + current_billing_term_ends_at=datetime.fromisoformat( + "2027-08-07T00:00:00+00:00" + ), + ) + # Inside the current window, which opens on 7 September 2026. + OrganisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=90, + notified_at=now, + ) + # A year earlier, only reachable if the year is dropped. + OrganisationAPIUsageNotification.objects.create( + organisation=organisation, + percent_usage=100, + notified_at=datetime.fromisoformat("2025-09-20T00:00:00+00:00"), + ) + + url = reverse( + "api-v1:organisations:organisation-api-usage-notification", + args=[organisation.id], + ) + + # When + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert [r["percent_usage"] for r in response.data["results"]] == [90] From 1a33ab26456862601ecfad8cb90e07c3e3414094 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 11 Sep 2026 17:37:45 -0300 Subject: [PATCH 2/4] test(usage): split the Given/When/Then comments the linter wants Co-Authored-By: Claude Opus 5 (1M context) --- ...test_unit_organisations_billing_periods.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/api/tests/unit/organisations/test_unit_organisations_billing_periods.py b/api/tests/unit/organisations/test_unit_organisations_billing_periods.py index e2b61edff993..68f24b1c6562 100644 --- a/api/tests/unit/organisations/test_unit_organisations_billing_periods.py +++ b/api/tests/unit/organisations/test_unit_organisations_billing_periods.py @@ -18,11 +18,11 @@ def test_months_elapsed__spans_years__counts_them( since: str, now: str, expected: int ) -> None: - # Given / When / Then - assert ( - months_elapsed(datetime.fromisoformat(since), datetime.fromisoformat(now)) - == expected - ) + # Given / When + elapsed = months_elapsed(datetime.fromisoformat(since), datetime.fromisoformat(now)) + + # Then + assert elapsed == expected @pytest.mark.parametrize( @@ -45,7 +45,10 @@ def test_months_elapsed__spans_years__counts_them( def test_period_start__long_term__opens_on_the_latest_anniversary( term_starts_at: str, now: str, expected: str ) -> None: - # Given / When / Then - assert period_start( + # Given / When + start = period_start( datetime.fromisoformat(term_starts_at), datetime.fromisoformat(now) - ) == datetime.fromisoformat(expected) + ) + + # Then + assert start == datetime.fromisoformat(expected) From b3cca301d16686f34daaaf8e0fd25ffadd3859de Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Thu, 24 Sep 2026 14:51:32 -0300 Subject: [PATCH 3/4] chore: Update documentation artefacts --- .../observability/_events-catalogue.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 9be9f43dd113..a6001e9c2ba4 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -2,7 +2,7 @@ ### `api_usage.notification.evaluated` Logged at `info` from: - - `api/organisations/task_helpers.py:157` + - `api/organisations/task_helpers.py:155` Attributes: - `allowed_api_calls` @@ -24,7 +24,7 @@ Attributes: ### `api_usage.notification.sent` Logged at `info` from: - - `api/organisations/task_helpers.py:180` + - `api/organisations/task_helpers.py:178` Attributes: - `matched_threshold` @@ -33,9 +33,9 @@ Attributes: ### `app_analytics.no_analytics_database_configured` Logged at `warning` from: - - `api/app_analytics/analytics_db_service.py:74` - - `api/app_analytics/analytics_db_service.py:187` - - `api/app_analytics/analytics_db_service.py:278` + - `api/app_analytics/analytics_db_service.py:75` + - `api/app_analytics/analytics_db_service.py:188` + - `api/app_analytics/analytics_db_service.py:279` Attributes: - `details` @@ -604,7 +604,7 @@ Attributes: ### `organisations.billing_term.stale` Logged at `warning` from: - - `api/organisations/models.py:614` + - `api/organisations/models.py:615` Attributes: - `billing_term.ends_at` From 9025cc902676afd4156db5316bca30e41518f407 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Thu, 24 Sep 2026 17:57:53 -0300 Subject: [PATCH 4/4] test(usage): assert the previous period's end in February The regression test discarded date_stop, so nothing covered the claim that both ends count from the term start. A 31 January term viewed on 1 March closes the previous window on 28 February, not 28 January. Co-Authored-By: Claude Opus 5 (1M context) --- .../test_analytics_db_service.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/api/tests/unit/app_analytics/test_analytics_db_service.py b/api/tests/unit/app_analytics/test_analytics_db_service.py index 887a5d6963af..3543c44173ca 100644 --- a/api/tests/unit/app_analytics/test_analytics_db_service.py +++ b/api/tests/unit/app_analytics/test_analytics_db_service.py @@ -1004,3 +1004,32 @@ def test_get_start_date_and_stop_date__term_over_a_year_old__counts_the_years( # Then assert date_start == datetime.fromisoformat(expected_start) + + +# February is too short for a 31st, so deriving the previous period's end from +# its own start would close the window on 28 January rather than 28 February. +@pytest.mark.freeze_time("2026-03-01T00:00:00+00:00") +def test_get_start_date_and_stop_date__previous_period_in_february__ends_on_the_28th( + db: None, + organisation: Organisation, +) -> None: + # Given + sub_cache = OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + current_billing_term_starts_at=datetime.fromisoformat( + "2026-01-31T00:00:00+00:00" + ), + current_billing_term_ends_at=datetime.fromisoformat( + "2027-01-31T00:00:00+00:00" + ), + ) + + # When + date_start, date_stop = _get_start_date_and_stop_date_for_subscribed_organisation( + sub_cache=sub_cache, + period=PREVIOUS_BILLING_PERIOD, + ) + + # Then + assert date_start == datetime.fromisoformat("2026-01-31T00:00:00+00:00") + assert date_stop == datetime.fromisoformat("2026-02-28T00:00:00+00:00")