From 2b75e96d50755b040ebeb8841f1bc04e88f775f8 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 26 Feb 2026 15:31:10 +0000 Subject: [PATCH 1/5] chore: Add type hints to Chargebee functions --- api/organisations/chargebee/chargebee.py | 103 +++++++++++++---------- 1 file changed, 60 insertions(+), 43 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index 0643220ef688..18f8ebd3df44 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -1,7 +1,7 @@ import logging -import typing from contextlib import suppress from datetime import datetime +from typing import Any from chargebee.api_error import ( # type: ignore[import-untyped] APIError as ChargebeeAPIError, @@ -9,6 +9,12 @@ from chargebee.models.hosted_page.operations import ( # type: ignore[import-untyped] HostedPage as HostedPageOps, ) +from chargebee.models.hosted_page.responses import ( # type: ignore[import-untyped] + HostedPageResponse, +) +from chargebee.models.plan.responses import ( # type: ignore[import-untyped] + RetrieveResponse as PlanRetrieveResponse, +) from chargebee.models.portal_session.operations import ( # type: ignore[import-untyped] PortalSession as PortalSessionOps, ) @@ -49,64 +55,70 @@ ] -def get_subscription_data_from_hosted_page(hosted_page_id): # type: ignore[no-untyped-def] - hosted_page = get_hosted_page(hosted_page_id) # type: ignore[no-untyped-call] - subscription = get_subscription_from_hosted_page(hosted_page) # type: ignore[no-untyped-call] - plan_metadata = get_plan_meta_data(subscription["plan_id"]) # type: ignore[no-untyped-call] - if subscription: - return { - "subscription_id": subscription["id"], - "plan": subscription["plan_id"], - "subscription_date": datetime.fromtimestamp( - subscription["created_at"], tz=UTC - ), - "max_seats": get_max_seats_for_plan(plan_metadata), - "max_api_calls": get_max_api_calls_for_plan(plan_metadata), - "customer_id": get_customer_id_from_hosted_page(hosted_page), # type: ignore[no-untyped-call] - "payment_method": CHARGEBEE, - } - else: +def get_subscription_data_from_hosted_page( + hosted_page_id: str, +) -> dict[str, Any]: + hosted_page = get_hosted_page(hosted_page_id) + subscription = get_subscription_from_hosted_page(hosted_page) + if not subscription: return {} - - -def get_hosted_page(hosted_page_id): # type: ignore[no-untyped-def] + plan_metadata = get_plan_meta_data(subscription["plan_id"]) + return { + "subscription_id": subscription["id"], + "plan": subscription["plan_id"], + "subscription_date": datetime.fromtimestamp(subscription["created_at"], tz=UTC), + "max_seats": get_max_seats_for_plan(plan_metadata), + "max_api_calls": get_max_api_calls_for_plan(plan_metadata), + "customer_id": get_customer_id_from_hosted_page(hosted_page), + "payment_method": CHARGEBEE, + } + + +def get_hosted_page(hosted_page_id: str) -> HostedPageResponse: response = chargebee_client.HostedPage.retrieve(hosted_page_id) return response.hosted_page -def get_subscription_from_hosted_page(hosted_page): # type: ignore[no-untyped-def] +def get_subscription_from_hosted_page( + hosted_page: HostedPageResponse, +) -> dict[str, Any] | None: content = hosted_page.content if content and "subscription" in content: - return content["subscription"] + return content["subscription"] # type: ignore[no-any-return] + return None -def get_customer_id_from_hosted_page(hosted_page): # type: ignore[no-untyped-def] +def get_customer_id_from_hosted_page( + hosted_page: HostedPageResponse, +) -> str | None: content = hosted_page.content if content and "customer" in content: - return content["customer"]["id"] + return content["customer"]["id"] # type: ignore[no-any-return] + return None -def get_max_seats_for_plan(meta_data: dict) -> int: # type: ignore[type-arg] - return meta_data.get("seats", 1) # type: ignore[no-any-return] +def get_max_seats_for_plan(meta_data: dict[str, Any]) -> int: + return int(meta_data.get("seats", 1)) -def get_max_api_calls_for_plan(meta_data: dict) -> int: # type: ignore[type-arg] - return meta_data.get("api_calls", 50000) # type: ignore[no-any-return] +def get_max_api_calls_for_plan(meta_data: dict[str, Any]) -> int: + return int(meta_data.get("api_calls", 50000)) -def get_plan_meta_data(plan_id): # type: ignore[no-untyped-def] - plan_details = get_plan_details(plan_id) # type: ignore[no-untyped-call] +def get_plan_meta_data(plan_id: str) -> dict[str, Any]: + plan_details = get_plan_details(plan_id) if plan_details and hasattr(plan_details.plan, "meta_data"): return plan_details.plan.meta_data or {} return {} -def get_plan_details(plan_id): # type: ignore[no-untyped-def] +def get_plan_details(plan_id: str) -> PlanRetrieveResponse | None: if plan_id: return chargebee_client.Plan.retrieve(plan_id) + return None -def get_portal_url(customer_id, redirect_url): # type: ignore[no-untyped-def] +def get_portal_url(customer_id: str, redirect_url: str) -> str | None: result = chargebee_client.PortalSession.create( PortalSessionOps.CreateParams( redirect_url=redirect_url, @@ -116,13 +128,15 @@ def get_portal_url(customer_id, redirect_url): # type: ignore[no-untyped-def] ) ) if result and hasattr(result, "portal_session"): - return result.portal_session.access_url + return result.portal_session.access_url # type: ignore[no-any-return] + return None -def get_customer_id_from_subscription_id(subscription_id): # type: ignore[no-untyped-def] +def get_customer_id_from_subscription_id(subscription_id: str) -> str | None: subscription_response = chargebee_client.Subscription.retrieve(subscription_id) if hasattr(subscription_response, "customer"): - return subscription_response.customer.id + return subscription_response.customer.id # type: ignore[no-any-return] + return None def get_hosted_page_url_for_subscription_upgrade( @@ -140,7 +154,7 @@ def get_hosted_page_url_for_subscription_upgrade( def extract_subscription_metadata( - chargebee_subscription: dict, # type: ignore[type-arg] + chargebee_subscription: dict[str, Any], customer_email: str, ) -> ChargebeeObjMetadata: chargebee_addons = chargebee_subscription.get("addons", []) @@ -160,9 +174,9 @@ def extract_subscription_metadata( return subscription_metadata -def get_subscription_metadata_from_id( # type: ignore[return] +def get_subscription_metadata_from_id( subscription_id: str, -) -> typing.Optional[ChargebeeObjMetadata]: +) -> ChargebeeObjMetadata | None: if not (subscription_id and subscription_id.strip() != ""): logger.warning("Subscription id is empty or None") return None @@ -174,11 +188,14 @@ def get_subscription_metadata_from_id( # type: ignore[return] ) return extract_subscription_metadata( - chargebee_subscription, chargebee_result.customer.email + chargebee_subscription, + chargebee_result.customer.email, ) + return None + -def cancel_subscription(subscription_id: str): # type: ignore[no-untyped-def] +def cancel_subscription(subscription_id: str) -> None: try: chargebee_client.Subscription.cancel( subscription_id, @@ -190,7 +207,7 @@ def cancel_subscription(subscription_id: str): # type: ignore[no-untyped-def] raise CannotCancelChargebeeSubscription(msg) from e -def add_single_seat(subscription_id: str): # type: ignore[no-untyped-def] +def add_single_seat(subscription_id: str) -> None: try: subscription = chargebee_client.Subscription.retrieve( subscription_id @@ -292,7 +309,7 @@ def add_100k_api_calls( def _convert_chargebee_subscription_to_dictionary( chargebee_subscription: SubscriptionResponse, -) -> dict: # type: ignore[type-arg] +) -> dict[str, Any]: chargebee_subscription_dict = dict(chargebee_subscription.raw_data) addons = chargebee_subscription.addons or [] chargebee_subscription_dict["addons"] = [dict(addon.raw_data) for addon in addons] From 66a14ef20d995c1d35bbc7f18b886c481e116f81 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 26 Feb 2026 15:43:54 +0000 Subject: [PATCH 2/5] remove stale type ignores --- api/organisations/models.py | 8 +++++--- api/organisations/serializers.py | 2 +- .../chargebee/test_unit_chargebee_chargebee.py | 8 ++++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/api/organisations/models.py b/api/organisations/models.py index 77f6909f2e0e..10219fe8ff6d 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -261,7 +261,7 @@ class Subscription(LifecycleModelMixin, SoftDeleteExportableModel): # type: ign history = HistoricalRecords() def update_plan(self, plan_id): # type: ignore[no-untyped-def] - plan_metadata = get_plan_meta_data(plan_id) # type: ignore[no-untyped-call] + plan_metadata = get_plan_meta_data(plan_id) self.cancellation_date = None self.plan = plan_id self.max_seats = get_max_seats_for_plan(plan_metadata) @@ -375,11 +375,13 @@ def get_portal_url(self, redirect_url): # type: ignore[no-untyped-def] return None if not self.customer_id: - self.customer_id = get_customer_id_from_subscription_id( # type: ignore[no-untyped-call] + self.customer_id = get_customer_id_from_subscription_id( self.subscription_id ) self.save() - return get_portal_url(self.customer_id, redirect_url) # type: ignore[no-untyped-call] + if not self.customer_id: + return None + return get_portal_url(self.customer_id, redirect_url) def get_subscription_metadata(self) -> BaseSubscriptionMetadata: if self.is_free_plan: diff --git a/api/organisations/serializers.py b/api/organisations/serializers.py index 519718961a3d..baf70e32ebc7 100644 --- a/api/organisations/serializers.py +++ b/api/organisations/serializers.py @@ -200,7 +200,7 @@ def create(self, validated_data): # type: ignore[no-untyped-def] organisation = self._get_organisation() # type: ignore[no-untyped-call] if settings.ENABLE_CHARGEBEE: - subscription_data = get_subscription_data_from_hosted_page( # type: ignore[no-untyped-call] + subscription_data = get_subscription_data_from_hosted_page( hosted_page_id=validated_data["hosted_page_id"] ) 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 2f1c543f5b21..a393129cda15 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -198,7 +198,7 @@ def test_chargebee_get_plan_meta_data_returns_correct_metadata( ) # When - plan_meta_data = get_plan_meta_data(plan_id) # type: ignore[no-untyped-call] + plan_meta_data = get_plan_meta_data(plan_id) # Then assert plan_meta_data == { @@ -230,7 +230,7 @@ def test_chargebee_get_subscription_data_from_hosted_page_returns_expected_value mock_cb.Plan.retrieve.return_value = MockChargeBeePlanResponse(expected_max_seats) # type: ignore[no-untyped-call] # noqa: E501 # When - subscription_data = get_subscription_data_from_hosted_page("hosted_page_id") # type: ignore[no-untyped-call] + subscription_data = get_subscription_data_from_hosted_page("hosted_page_id") # Then assert subscription_data["subscription_id"] == subscription_id @@ -252,7 +252,7 @@ def test_get_chargebee_portal_url(mocker: MockerFixture) -> None: ) # When - portal_url = get_portal_url("some-customer-id", "https://redirect.url.com") # type: ignore[no-untyped-call] + portal_url = get_portal_url("some-customer-id", "https://redirect.url.com") # Then assert portal_url == access_url @@ -271,7 +271,7 @@ def test_chargebee_get_customer_id_from_subscription( ) # When - customer_id = get_customer_id_from_subscription_id("subscription-id") # type: ignore[no-untyped-call] + customer_id = get_customer_id_from_subscription_id("subscription-id") # Then assert customer_id == expected_customer_id From 6dc19c13537f48770ec41057b76c9b7da4524d75 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 26 Feb 2026 15:45:34 +0000 Subject: [PATCH 3/5] nicer branching --- api/organisations/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/organisations/models.py b/api/organisations/models.py index 10219fe8ff6d..647e2b4d86f2 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -379,9 +379,9 @@ def get_portal_url(self, redirect_url): # type: ignore[no-untyped-def] self.subscription_id ) self.save() - if not self.customer_id: - return None - return get_portal_url(self.customer_id, redirect_url) + + if self.customer_id: + return get_portal_url(self.customer_id, redirect_url) def get_subscription_metadata(self) -> BaseSubscriptionMetadata: if self.is_free_plan: From 93700a5fd63bea358a98ace297eb5a0b91ad59a1 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 26 Feb 2026 16:30:50 +0000 Subject: [PATCH 4/5] fix coverage --- api/organisations/models.py | 3 +- .../test_unit_chargebee_chargebee.py | 80 ++++++++++++++++++- .../test_unit_organisations_models.py | 70 ++++++++++++++++ .../test_unit_organisations_serializers.py | 41 ++++++++++ 4 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 api/tests/unit/organisations/test_unit_organisations_serializers.py diff --git a/api/organisations/models.py b/api/organisations/models.py index 647e2b4d86f2..95168db04064 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -370,7 +370,7 @@ def prepare_for_cancel( # type: ignore[no-untyped-def] self.billing_status = None self.save() - def get_portal_url(self, redirect_url): # type: ignore[no-untyped-def] + def get_portal_url(self, redirect_url: str) -> str | None: if not self.subscription_id: return None @@ -382,6 +382,7 @@ def get_portal_url(self, redirect_url): # type: ignore[no-untyped-def] if self.customer_id: return get_portal_url(self.customer_id, redirect_url) + return None def get_subscription_metadata(self) -> BaseSubscriptionMetadata: if self.is_free_plan: 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 a393129cda15..93c378d94c83 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -27,7 +27,12 @@ get_subscription_data_from_hosted_page, get_subscription_metadata_from_id, ) -from organisations.chargebee.chargebee import cancel_subscription +from organisations.chargebee.chargebee import ( + cancel_subscription, + get_customer_id_from_hosted_page, + get_plan_details, + get_subscription_from_hosted_page, +) from organisations.chargebee.constants import ( ADDITIONAL_API_SCALE_UP_ADDON_ID, ADDITIONAL_SEAT_ADDON_ID, @@ -713,3 +718,76 @@ def test_add_100k_api_calls_when_chargebee_api_error_has_no_error_code( count=1, invoice_immediately=True, ) + + +def test_get_subscription_from_hosted_page__no_subscription__returns_none( + mocker: MockerFixture, +) -> None: + # Given + hosted_page = mocker.MagicMock(content={"customer": {"id": "cust-1"}}) + + # When + result = get_subscription_from_hosted_page(hosted_page) + + # Then + assert result is None + + +def test_get_customer_id_from_hosted_page__no_customer__returns_none( + mocker: MockerFixture, +) -> None: + # Given + hosted_page = mocker.MagicMock( + content={"subscription": {"id": "sub-1", "plan_id": "plan-1"}} + ) + + # When + result = get_customer_id_from_hosted_page(hosted_page) + + # Then + assert result is None + + +def test_get_plan_details__empty_plan_id__returns_none() -> None: + # Given + plan_id = "" + + # When + result = get_plan_details(plan_id) + + # Then + assert result is None + + +def test_get_portal_url__no_portal_session__returns_none( + mocker: MockerFixture, +) -> None: + # Given + mock_cb = mocker.patch( + "organisations.chargebee.chargebee.chargebee_client", autospec=True + ) + mock_result = mocker.MagicMock(spec=[]) # no attributes at all + mock_cb.PortalSession.create.return_value = mock_result + + # When + result = get_portal_url("customer-id", "https://redirect.url.com") + + # Then + assert result is None + + +def test_get_customer_id_from_subscription_id__no_customer__returns_none( + mocker: MockerFixture, +) -> None: + # Given + mock_cb = mocker.patch( + "organisations.chargebee.chargebee.chargebee_client", autospec=True + ) + mock_response = mocker.MagicMock(spec=[]) # no customer attribute + mock_cb.Subscription.retrieve.return_value = mock_response + + # When + result = get_customer_id_from_subscription_id("sub-123") + + # Then + assert result is None diff --git a/api/tests/unit/organisations/test_unit_organisations_models.py b/api/tests/unit/organisations/test_unit_organisations_models.py index 4c78652fba1f..3e07b6b6b36c 100644 --- a/api/tests/unit/organisations/test_unit_organisations_models.py +++ b/api/tests/unit/organisations/test_unit_organisations_models.py @@ -672,3 +672,73 @@ def test_user_organisation_create_calls_hubspot_lead_tracking( args=(user.id, organisation.id), delay_until=timezone.now() + timedelta(minutes=3), ) + + +def test_subscription_get_portal_url__customer_id_not_found__returns_none( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + subscription = organisation.subscription + subscription.subscription_id = "sub-123" + subscription.customer_id = "" + subscription.save() + + mocker.patch( + "organisations.models.get_customer_id_from_subscription_id", + return_value=None, + ) + + # When + result = subscription.get_portal_url("https://example.com") + + # Then + assert result is None + + +def test_subscription_get_portal_url__customer_id_exists__returns_url( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + subscription = organisation.subscription + subscription.subscription_id = "sub-123" + subscription.customer_id = "cust-123" + subscription.save() + + expected_url = "https://portal.chargebee.com/session" + mocker.patch( + "organisations.models.get_portal_url", + return_value=expected_url, + ) + + # When + result = subscription.get_portal_url("https://example.com") + + # Then + assert result == expected_url + + +def test_subscription_update_plan__updates_fields_from_chargebee( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + subscription = organisation.subscription + plan_id = "startup-v2" + expected_metadata = {"seats": 5, "api_calls": 500000} + + mocker.patch( + "organisations.models.get_plan_meta_data", + return_value=expected_metadata, + ) + + # When + subscription.update_plan(plan_id) # type: ignore[no-untyped-call] + + # Then + subscription.refresh_from_db() + assert subscription.plan == plan_id + assert subscription.max_seats == 5 + assert subscription.max_api_calls == 500000 + assert subscription.cancellation_date is None diff --git a/api/tests/unit/organisations/test_unit_organisations_serializers.py b/api/tests/unit/organisations/test_unit_organisations_serializers.py new file mode 100644 index 000000000000..d9033659f800 --- /dev/null +++ b/api/tests/unit/organisations/test_unit_organisations_serializers.py @@ -0,0 +1,41 @@ +from pytest_django.fixtures import SettingsWrapper +from pytest_mock import MockerFixture + +from organisations.models import Organisation +from organisations.serializers import UpdateSubscriptionSerializer + + +def test_update_subscription_serializer__create__updates_subscription( + organisation: Organisation, + mocker: MockerFixture, + settings: SettingsWrapper, +) -> None: + # Given + settings.ENABLE_CHARGEBEE = True + subscription_data = { + "subscription_id": "new-sub-id", + "plan": "startup-v2", + "max_seats": 10, + "max_api_calls": 1000000, + "customer_id": "cust-123", + "payment_method": "CHARGEBEE", + } + mocker.patch( + "organisations.serializers.get_subscription_data_from_hosted_page", + return_value=subscription_data, + ) + + serializer = UpdateSubscriptionSerializer( + data={"hosted_page_id": "hp-123"}, + context={"organisation": organisation.id}, + ) + serializer.is_valid(raise_exception=True) + + # When + result = serializer.save() + + # Then + assert result == organisation + organisation.subscription.refresh_from_db() + assert organisation.subscription.subscription_id == "new-sub-id" + assert organisation.subscription.plan == "startup-v2" From e3ff98286c27664bf244df948198616549133f59 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 26 Feb 2026 16:32:22 +0000 Subject: [PATCH 5/5] standardise aliasing --- api/organisations/chargebee/chargebee.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index 18f8ebd3df44..b97db651d69e 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -7,13 +7,13 @@ APIError as ChargebeeAPIError, ) from chargebee.models.hosted_page.operations import ( # type: ignore[import-untyped] - HostedPage as HostedPageOps, + HostedPage as ChargebeeHostedPageOps, ) from chargebee.models.hosted_page.responses import ( # type: ignore[import-untyped] HostedPageResponse, ) from chargebee.models.plan.responses import ( # type: ignore[import-untyped] - RetrieveResponse as PlanRetrieveResponse, + RetrieveResponse as ChargebeePlanRetrieveResponse, ) from chargebee.models.portal_session.operations import ( # type: ignore[import-untyped] PortalSession as PortalSessionOps, @@ -112,7 +112,7 @@ def get_plan_meta_data(plan_id: str) -> dict[str, Any]: return {} -def get_plan_details(plan_id: str) -> PlanRetrieveResponse | None: +def get_plan_details(plan_id: str) -> ChargebeePlanRetrieveResponse | None: if plan_id: return chargebee_client.Plan.retrieve(plan_id) return None @@ -143,8 +143,8 @@ def get_hosted_page_url_for_subscription_upgrade( subscription_id: str, plan_id: str ) -> str: checkout_existing_response = chargebee_client.HostedPage.checkout_existing( - HostedPageOps.CheckoutExistingParams( - subscription=HostedPageOps.CheckoutExistingSubscriptionParams( + ChargebeeHostedPageOps.CheckoutExistingParams( + subscription=ChargebeeHostedPageOps.CheckoutExistingSubscriptionParams( id=subscription_id, plan_id=plan_id, ),