From 95984f95f8f3c86c1fdddb61a62e8704f7672eff Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 2 Apr 2026 15:14:32 +0100 Subject: [PATCH 1/6] Fix billing period per add on logic --- api/organisations/chargebee/chargebee.py | 29 ++++++++++++++----- api/organisations/chargebee/constants.py | 2 -- .../test_unit_chargebee_chargebee.py | 26 ++++++++++++----- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index 02daa26cbbb1..de6f5aa89f4f 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -18,6 +18,9 @@ from chargebee.models.portal_session.operations import ( # type: ignore[import-untyped] PortalSession as PortalSessionOps, ) +from chargebee.models.subscription.operations import ( + Subscription, +) from chargebee.models.subscription.operations import ( # type: ignore[import-untyped] Subscription as SubscriptionOps, ) @@ -31,7 +34,6 @@ from organisations.chargebee.constants import ( ADDITIONAL_API_SCALE_UP_ADDON_ID, ADDITIONAL_API_START_UP_ADDON_ID, - ADDITIONAL_SEAT_ADDON_ID, ) from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.subscriptions.constants import CHARGEBEE @@ -212,13 +214,11 @@ def add_single_seat(subscription_id: str) -> None: subscription = chargebee_client.Subscription.retrieve( subscription_id ).subscription + addon_id = _get_additional_seat_addon_id(subscription) + addons = subscription.addons or [] current_seats = next( - ( - addon.quantity - for addon in addons - if addon.id == ADDITIONAL_SEAT_ADDON_ID - ), + (addon.quantity for addon in addons if addon.id == addon_id), 0, ) @@ -227,7 +227,7 @@ def add_single_seat(subscription_id: str) -> None: SubscriptionOps.UpdateParams( addons=[ SubscriptionOps.UpdateAddonParams( - id=ADDITIONAL_SEAT_ADDON_ID, + id=_get_additional_seat_addon_id(subscription), quantity=current_seats + 1, ) ], @@ -254,6 +254,21 @@ def add_single_seat(subscription_id: str) -> None: raise UpgradeSeatsError(msg) from e +def _get_additional_seat_addon_id(subscription: Subscription) -> str: + addon_id_prefix = "additional-team-members-scale-up-v2" + addon_suffixes_by_billing_period = {1: "monthly", 6: "semiannual", 12: "annual"} + suffix = addon_suffixes_by_billing_period.get( + subscription.billing_period, "monthly" + ) + if not suffix: + logger.warning( + "Unexpected billing period for subscription ID %s: %d", + subscription.id, + subscription.billing_period, + ) + return "-".join([addon_id_prefix, suffix]) + + def add_100k_api_calls_start_up( subscription_id: str, count: int = 1, invoice_immediately: bool = False ) -> None: diff --git a/api/organisations/chargebee/constants.py b/api/organisations/chargebee/constants.py index af3135c0e926..c063d49586fc 100644 --- a/api/organisations/chargebee/constants.py +++ b/api/organisations/chargebee/constants.py @@ -1,4 +1,2 @@ -ADDITIONAL_SEAT_ADDON_ID = "additional-team-members-scale-up-v2" - ADDITIONAL_API_START_UP_ADDON_ID = "additional-api-start-up-monthly" ADDITIONAL_API_SCALE_UP_ADDON_ID = "additional-api-scale-up-monthly" diff --git a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py index 5a2413a793e6..d7bb4cf7936c 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -35,7 +35,6 @@ ) from organisations.chargebee.constants import ( ADDITIONAL_API_SCALE_UP_ADDON_ID, - ADDITIONAL_SEAT_ADDON_ID, ) from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.subscriptions.exceptions import ( @@ -514,7 +513,7 @@ def test_get_subscription_metadata_from_id__addons_is_none__returns_plan_metadat def test_add_single_seat__existing_addon__increments_quantity(mocker) -> None: # type: ignore[no-untyped-def] # Given plan_id = "plan-id" - addon_id = ADDITIONAL_SEAT_ADDON_ID + addon_id = "additional-team-members-scale-up-v2-monthly" subscription_id = "subscription-id" addon_quantity = 1 @@ -523,6 +522,7 @@ def test_add_single_seat__existing_addon__increments_quantity(mocker) -> None: id=subscription_id, plan_id=plan_id, addons=[mocker.MagicMock(id=addon_id, quantity=addon_quantity)], + billing_period=1, ) mocked_chargebee = mocker.patch( "organisations.chargebee.chargebee.chargebee_client", autospec=True @@ -542,7 +542,7 @@ def test_add_single_seat__existing_addon__increments_quantity(mocker) -> None: SubscriptionOps.UpdateParams( addons=[ SubscriptionOps.UpdateAddonParams( - id=ADDITIONAL_SEAT_ADDON_ID, quantity=addon_quantity + 1 + id=addon_id, quantity=addon_quantity + 1 ) ], prorate=True, @@ -551,8 +551,19 @@ def test_add_single_seat__existing_addon__increments_quantity(mocker) -> None: ) +@pytest.mark.parametrize( + "billing_period,expected_add_on_id", + ( + (1, "additional-team-members-scale-up-v2-monthly"), + (6, "additional-team-members-scale-up-v2-semiannual"), + (12, "additional-team-members-scale-up-v2-annual"), + # unexpected or missing billing period should default to monthly + (None, "additional-team-members-scale-up-v2-monthly"), + (7, "additional-team-members-scale-up-v2-monthly"), + ), +) def test_add_single_seat__no_existing_addon__creates_addon_with_quantity_one( # type: ignore[no-untyped-def] - mocker, + mocker: MockerFixture, billing_period: int, expected_add_on_id: str ) -> None: # Given subscription_id = "subscription-id" @@ -562,6 +573,7 @@ def test_add_single_seat__no_existing_addon__creates_addon_with_quantity_one( # id=subscription_id, plan_id="plan_id", addons=[], + billing_period=billing_period, ) mocked_chargebee = mocker.patch( "organisations.chargebee.chargebee.chargebee_client", autospec=True @@ -580,9 +592,7 @@ def test_add_single_seat__no_existing_addon__creates_addon_with_quantity_one( # subscription_id, SubscriptionOps.UpdateParams( addons=[ - SubscriptionOps.UpdateAddonParams( - id=ADDITIONAL_SEAT_ADDON_ID, quantity=1 - ) + SubscriptionOps.UpdateAddonParams(id=expected_add_on_id, quantity=1) ], prorate=True, invoice_immediately=True, @@ -633,7 +643,7 @@ def test_add_single_seat__api_error__raises_upgrade_seats_error( # type: ignore SubscriptionOps.UpdateParams( addons=[ SubscriptionOps.UpdateAddonParams( - id=ADDITIONAL_SEAT_ADDON_ID, quantity=1 + id="additional-team-members-scale-up-v2-monthly", quantity=1 ) ], prorate=True, From df6a2d8b874ee818a37a537ec4addcf0330095c3 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 2 Apr 2026 15:15:28 +0100 Subject: [PATCH 2/6] Remove unused type ignore --- .../organisations/chargebee/test_unit_chargebee_chargebee.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py index d7bb4cf7936c..25012df71d57 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -562,7 +562,7 @@ def test_add_single_seat__existing_addon__increments_quantity(mocker) -> None: (7, "additional-team-members-scale-up-v2-monthly"), ), ) -def test_add_single_seat__no_existing_addon__creates_addon_with_quantity_one( # type: ignore[no-untyped-def] +def test_add_single_seat__no_existing_addon__creates_addon_with_quantity_one( mocker: MockerFixture, billing_period: int, expected_add_on_id: str ) -> None: # Given From e99fdf704c5e7197896abde11c11a735399c4350 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 2 Apr 2026 15:16:40 +0100 Subject: [PATCH 3/6] Fix remaining typing issues --- api/organisations/chargebee/chargebee.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index de6f5aa89f4f..3c3c1387a722 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -18,9 +18,6 @@ from chargebee.models.portal_session.operations import ( # type: ignore[import-untyped] PortalSession as PortalSessionOps, ) -from chargebee.models.subscription.operations import ( - Subscription, -) from chargebee.models.subscription.operations import ( # type: ignore[import-untyped] Subscription as SubscriptionOps, ) @@ -254,7 +251,7 @@ def add_single_seat(subscription_id: str) -> None: raise UpgradeSeatsError(msg) from e -def _get_additional_seat_addon_id(subscription: Subscription) -> str: +def _get_additional_seat_addon_id(subscription: SubscriptionOps) -> str: addon_id_prefix = "additional-team-members-scale-up-v2" addon_suffixes_by_billing_period = {1: "monthly", 6: "semiannual", 12: "annual"} suffix = addon_suffixes_by_billing_period.get( From 3b54a636c59861e1d97f9286c8fbd3594b8a12a0 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 2 Apr 2026 15:54:29 +0100 Subject: [PATCH 4/6] Fix fallback logic --- api/organisations/chargebee/chargebee.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index 3c3c1387a722..632c9343d5e6 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -254,15 +254,14 @@ def add_single_seat(subscription_id: str) -> None: def _get_additional_seat_addon_id(subscription: SubscriptionOps) -> str: addon_id_prefix = "additional-team-members-scale-up-v2" addon_suffixes_by_billing_period = {1: "monthly", 6: "semiannual", 12: "annual"} - suffix = addon_suffixes_by_billing_period.get( - subscription.billing_period, "monthly" - ) + suffix = addon_suffixes_by_billing_period.get(subscription.billing_period) if not suffix: logger.warning( "Unexpected billing period for subscription ID %s: %d", subscription.id, subscription.billing_period, ) + suffix = "monthly" return "-".join([addon_id_prefix, suffix]) From 172fd45be35681bd7a5c91dfda81a3f462c490b1 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 2 Apr 2026 16:06:55 +0100 Subject: [PATCH 5/6] Fix logging --- api/organisations/chargebee/chargebee.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index 632c9343d5e6..cbcef7dc402a 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -257,9 +257,8 @@ def _get_additional_seat_addon_id(subscription: SubscriptionOps) -> str: suffix = addon_suffixes_by_billing_period.get(subscription.billing_period) if not suffix: logger.warning( - "Unexpected billing period for subscription ID %s: %d", + "Unexpected billing period for subscription ID %s", subscription.id, - subscription.billing_period, ) suffix = "monthly" return "-".join([addon_id_prefix, suffix]) From 301c72e875d6a7390f2187a108cdf54183e5f8a8 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 2 Apr 2026 16:43:13 +0100 Subject: [PATCH 6/6] Fix broken test missing billing period --- .../organisations/chargebee/test_unit_chargebee_chargebee.py | 1 + 1 file changed, 1 insertion(+) diff --git a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py index 25012df71d57..bf1ac00c5e16 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -626,6 +626,7 @@ def test_add_single_seat__api_error__raises_upgrade_seats_error( # type: ignore id=subscription_id, plan_id="plan-id", addons=[], + billing_period=1, ) # tie that subscription object to the mocked chargebee object