Skip to content

Commit 08a0f95

Browse files
talissoncostaclaude
andcommitted
fix(usage): count whole years when resolving the current billing period
The current period was derived from the months component of the delta between now and the billing term start, so whole years were dropped. A term that began more than twelve months ago resolved to a period a year early, and the usage page reported a window over a year long instead of the current month. The same arithmetic existed in three places and was correct in only one of them, so it now lives in one function used by all three: the usage data query, the API usage notifications view and the notification task. The view had the same bug, which meant a stale notification could be treated as current. Closes #6099 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 93a6d02 commit 08a0f95

7 files changed

Lines changed: 157 additions & 17 deletions

File tree

api/app_analytics/analytics_db_service.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from environments.models import Environment
3030
from features.models import Feature
3131
from organisations.models import Organisation, OrganisationSubscriptionInformationCache
32+
from organisations.services import get_current_billing_period_start_date
3233

3334
logger = structlog.get_logger("app_analytics")
3435

@@ -341,8 +342,7 @@ def _get_start_date_and_stop_date_for_subscribed_organisation(
341342
else:
342343
raise NotFound("No billing periods found for this organisation.")
343344

344-
month_delta = relativedelta(now, starts_at).months
345-
date_start = relativedelta(months=month_delta) + starts_at
345+
date_start = get_current_billing_period_start_date(starts_at, now)
346346
return date_start, now
347347

348348
case constants.PREVIOUS_BILLING_PERIOD:
@@ -351,10 +351,8 @@ def _get_start_date_and_stop_date_for_subscribed_organisation(
351351
else:
352352
raise NotFound("No billing periods found for this organisation.")
353353

354-
month_delta = relativedelta(now, starts_at).months - 1
355-
month_delta += relativedelta(now, starts_at).years * 12
356-
date_start = relativedelta(months=month_delta) + starts_at
357-
date_stop = relativedelta(months=month_delta + 1) + starts_at
354+
date_stop = get_current_billing_period_start_date(starts_at, now)
355+
date_start = date_stop - relativedelta(months=1)
358356
return date_start, date_stop
359357

360358
case constants.NINETY_DAY_PERIOD:

api/organisations/services.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from datetime import datetime
2+
3+
from dateutil.relativedelta import relativedelta
4+
5+
6+
def get_current_billing_period_start_date(
7+
billing_term_starts_at: datetime,
8+
now: datetime,
9+
) -> datetime:
10+
"""
11+
Return the start of the monthly period an organisation is currently in.
12+
13+
A billing term can be longer than a month, an annual plan being the common
14+
case, but API usage is allowed per month. The current period therefore
15+
starts at the most recent monthly anniversary of the term start, which for
16+
a term that began more than a year ago means counting the years as well as
17+
the months.
18+
"""
19+
elapsed = relativedelta(now, billing_term_starts_at)
20+
months_elapsed = elapsed.years * 12 + elapsed.months
21+
return billing_term_starts_at + relativedelta(months=months_elapsed)

api/organisations/task_helpers.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from datetime import timedelta
22

33
import structlog
4-
from dateutil.relativedelta import relativedelta
54
from django.conf import settings
65
from django.core.mail import send_mail
76
from django.template.loader import render_to_string
@@ -16,6 +15,7 @@
1615
OrganisationAPIUsageNotification,
1716
OrganisationRole,
1817
)
18+
from organisations.services import get_current_billing_period_start_date
1919
from organisations.subscriptions.constants import MAX_API_CALLS_IN_FREE_PLAN
2020
from users.models import FFAdminUser
2121

@@ -123,9 +123,7 @@ def handle_api_usage_notification_for_organisation(organisation: Organisation) -
123123
)
124124
return
125125

126-
# Truncate to the closest active month to get start of current period.
127-
month_delta = _get_total_months(relativedelta(now, billing_starts_at))
128-
period_starts_at = relativedelta(months=month_delta) + billing_starts_at
126+
period_starts_at = get_current_billing_period_start_date(billing_starts_at, now)
129127

130128
allowed_api_calls = subscription_cache.allowed_30d_api_calls
131129

@@ -182,7 +180,3 @@ def handle_api_usage_notification_for_organisation(organisation: Organisation) -
182180
)
183181

184182
_send_api_usage_notification(organisation, matched_threshold)
185-
186-
187-
def _get_total_months(rd: relativedelta) -> int:
188-
return rd.months + rd.years * 12

api/organisations/views.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import logging
55
from datetime import timedelta
66

7-
from dateutil.relativedelta import relativedelta
87
from django.utils import timezone
98
from drf_spectacular.utils import extend_schema, extend_schema_view
109
from rest_framework import status, viewsets
@@ -48,6 +47,7 @@
4847
SubscriptionDetailsSerializer,
4948
UpdateSubscriptionSerializer,
5049
)
50+
from organisations.services import get_current_billing_period_start_date
5151
from permissions.permissions_calculator import get_organisation_permission_data
5252
from permissions.serializers import (
5353
PermissionModelSerializer,
@@ -393,8 +393,7 @@ def get_queryset(self): # type: ignore[no-untyped-def]
393393
# by defaulting to something as a reasonable default.
394394
billing_starts_at = billing_starts_at or now - timedelta(days=30)
395395

396-
month_delta = relativedelta(now, billing_starts_at).months
397-
period_starts_at = relativedelta(months=month_delta) + billing_starts_at
396+
period_starts_at = get_current_billing_period_start_date(billing_starts_at, now)
398397

399398
queryset = OrganisationAPIUsageNotification.objects.filter(
400399
organisation_id=organisation.id,

api/tests/unit/app_analytics/test_analytics_db_service.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,39 @@ def test_get_usage_data__current_billing_period__passes_correct_date_range(
707707
)
708708

709709

710+
@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
711+
def test_get_usage_data__current_billing_period_annual_term__passes_correct_date_range(
712+
mocker: MockerFixture,
713+
settings: SettingsWrapper,
714+
organisation: Organisation,
715+
cache: OrganisationSubscriptionInformationCache,
716+
) -> None:
717+
# Given
718+
# A term that started more than twelve months ago, as an annual plan does.
719+
settings.USE_POSTGRES_FOR_ANALYTICS = True
720+
cache.current_billing_term_starts_at = datetime(
721+
2021, 12, 30, 9, 9, 47, 325132, tzinfo=UTC
722+
)
723+
cache.save()
724+
mocked_get_usage_data_from_local_db = mocker.patch(
725+
"app_analytics.analytics_db_service.get_usage_data_from_local_db", autospec=True
726+
)
727+
728+
# When
729+
get_usage_data(organisation, period=CURRENT_BILLING_PERIOD)
730+
731+
# Then
732+
# The current month of the term, not the same month a year earlier.
733+
mocked_get_usage_data_from_local_db.assert_called_once_with(
734+
organisation=organisation,
735+
environment_id=None,
736+
project_id=None,
737+
date_start=datetime(2022, 12, 30, 9, 9, 47, 325132, tzinfo=UTC),
738+
date_stop=datetime(2023, 1, 19, 9, 9, 47, 325132, tzinfo=UTC),
739+
labels_filter=None,
740+
)
741+
742+
710743
@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
711744
def test_get_usage_data__previous_billing_period__passes_correct_date_range(
712745
mocker: MockerFixture,
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
from datetime import UTC, datetime
2+
3+
import pytest
4+
5+
from organisations.services import get_current_billing_period_start_date
6+
7+
8+
@pytest.mark.parametrize(
9+
"billing_term_starts_at, now, expected",
10+
[
11+
pytest.param(
12+
datetime(2026, 1, 10, 9, 0, tzinfo=UTC),
13+
datetime(2026, 1, 20, 9, 0, tzinfo=UTC),
14+
datetime(2026, 1, 10, 9, 0, tzinfo=UTC),
15+
id="first_month_of_the_term",
16+
),
17+
pytest.param(
18+
datetime(2026, 1, 10, 9, 0, tzinfo=UTC),
19+
datetime(2026, 5, 3, 9, 0, tzinfo=UTC),
20+
datetime(2026, 4, 10, 9, 0, tzinfo=UTC),
21+
id="part_way_through_a_monthly_term",
22+
),
23+
pytest.param(
24+
# An annual term, well over a year old: the months-only delta used
25+
# to drop the years and land a period a year early.
26+
datetime(2024, 9, 15, 9, 0, tzinfo=UTC),
27+
datetime(2026, 7, 2, 9, 0, tzinfo=UTC),
28+
datetime(2026, 6, 15, 9, 0, tzinfo=UTC),
29+
id="term_older_than_a_year",
30+
),
31+
pytest.param(
32+
datetime(2024, 9, 15, 9, 0, tzinfo=UTC),
33+
datetime(2026, 9, 15, 9, 0, tzinfo=UTC),
34+
datetime(2026, 9, 15, 9, 0, tzinfo=UTC),
35+
id="exactly_on_an_anniversary",
36+
),
37+
],
38+
)
39+
def test_get_current_billing_period_start_date__returns_latest_monthly_anniversary(
40+
billing_term_starts_at: datetime,
41+
now: datetime,
42+
expected: datetime,
43+
) -> None:
44+
# When
45+
result = get_current_billing_period_start_date(billing_term_starts_at, now)
46+
47+
# Then
48+
assert result == expected
49+
assert result <= now

api/tests/unit/organisations/test_unit_organisations_views.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,6 +2055,52 @@ def test_get_api_usage_notifications__with_cache__returns_highest_notification(
20552055
assert response.data["results"][0]["percent_usage"] == 100
20562056

20572057

2058+
@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
2059+
def test_get_api_usage_notifications__annual_term__returns_current_period_notification(
2060+
staff_client: APIClient,
2061+
organisation: Organisation,
2062+
) -> None:
2063+
# Given
2064+
# A term that started more than twelve months ago, as an annual plan does.
2065+
url = reverse(
2066+
"api-v1:organisations:organisation-api-usage-notification",
2067+
args=[organisation.id],
2068+
)
2069+
2070+
now = timezone.now()
2071+
OrganisationSubscriptionInformationCache.objects.create(
2072+
organisation=organisation,
2073+
allowed_seats=10,
2074+
allowed_projects=3,
2075+
allowed_30d_api_calls=100,
2076+
chargebee_email="test@example.com",
2077+
current_billing_term_starts_at=now - timedelta(days=400),
2078+
current_billing_term_ends_at=now + timedelta(days=330),
2079+
)
2080+
2081+
# Notified in a month of the term that has already passed, so it should not
2082+
# be returned. Counting only the months of the delta would have included it.
2083+
OrganisationAPIUsageNotification.objects.create(
2084+
organisation=organisation,
2085+
percent_usage=90,
2086+
notified_at=now - timedelta(days=60),
2087+
)
2088+
OrganisationAPIUsageNotification.objects.create(
2089+
organisation=organisation,
2090+
percent_usage=75,
2091+
notified_at=now,
2092+
)
2093+
2094+
# When
2095+
response = staff_client.get(url)
2096+
2097+
# Then
2098+
assert response.status_code == status.HTTP_200_OK
2099+
2100+
assert len(response.data["results"]) == 1
2101+
assert response.data["results"][0]["percent_usage"] == 75
2102+
2103+
20582104
@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
20592105
def test_get_api_usage_notifications__stale_notification__returns_empty(
20602106
staff_client: APIClient,

0 commit comments

Comments
 (0)