From b3f48d2782ceb43a329ce672f7be986ac822007c Mon Sep 17 00:00:00 2001 From: Calvin Yau Date: Mon, 17 Nov 2025 14:53:39 -0800 Subject: [PATCH 1/5] feat: Add billing rate field to account details payload --- .../api/internal/owner/serializers.py | 15 ++++++++++--- .../tests/views/test_account_viewset.py | 22 ++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/apps/codecov-api/api/internal/owner/serializers.py b/apps/codecov-api/api/internal/owner/serializers.py index 8a36284b2..90e292270 100644 --- a/apps/codecov-api/api/internal/owner/serializers.py +++ b/apps/codecov-api/api/internal/owner/serializers.py @@ -212,15 +212,24 @@ class StripeScheduledPhaseSerializer(serializers.Serializer): start_date = serializers.IntegerField() plan = serializers.SerializerMethodField() quantity = serializers.SerializerMethodField() + billing_rate = serializers.SerializerMethodField() + + def _get_plan_object(self, phase: dict[str, Any]) -> Plan: + """Helper method to get and cache the Plan object.""" + if not hasattr(self, "_cached_plan"): + plan_id = phase["items"][0]["plan"] + self._cached_plan = Plan.objects.get(stripe_id=plan_id) + return self._cached_plan def get_plan(self, phase: dict[str, Any]) -> str: - plan_id = phase["items"][0]["plan"] - marketing_plan_name = Plan.objects.get(stripe_id=plan_id).marketing_name - return marketing_plan_name + return self._get_plan_object(phase).marketing_name def get_quantity(self, phase: dict[str, Any]) -> int: return phase["items"][0]["quantity"] + def get_billing_rate(self, phase: dict[str, Any]) -> str | None: + return self._get_plan_object(phase).billing_rate + class ScheduleDetailSerializer(serializers.Serializer): id = serializers.CharField() diff --git a/apps/codecov-api/api/internal/tests/views/test_account_viewset.py b/apps/codecov-api/api/internal/tests/views/test_account_viewset.py index a4669c93c..daa713e82 100644 --- a/apps/codecov-api/api/internal/tests/views/test_account_viewset.py +++ b/apps/codecov-api/api/internal/tests/views/test_account_viewset.py @@ -19,7 +19,11 @@ OwnerFactory, UserFactory, ) -from shared.plan.constants import DEFAULT_FREE_PLAN, PlanName, TrialStatus +from shared.plan.constants import ( + DEFAULT_FREE_PLAN, + PlanName, + TrialStatus, +) from utils.test_utils import APIClient curr_path = os.path.dirname(__file__) @@ -295,6 +299,7 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( "start_date": schedule_params["start_date"], "plan": "Pro", "quantity": schedule_params["quantity"], + "billing_rate": "annually", }, }, "student_count": 0, @@ -354,11 +359,21 @@ def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases phases = [ { "start_date": 123689126536, - "items": [{"plan": "test_plan_123", "quantity": 4}], + "items": [ + { + "plan": "test_plan_123", + "quantity": 4, + } + ], }, { "start_date": 123689126636, - "items": [{"plan": "test_plan_456", "quantity": 5}], + "items": [ + { + "plan": "test_plan_456", + "quantity": 5, + } + ], }, { "start_date": schedule_params["start_date"], @@ -430,6 +445,7 @@ def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases "plan": "Pro", "quantity": schedule_params["quantity"], "start_date": schedule_params["start_date"], + "billing_rate": "annually", }, }, "uses_invoice": False, From 2cfcfa70fbd03765d4a1eb2c5ee47a87c375cd93 Mon Sep 17 00:00:00 2001 From: Calvin Yau Date: Mon, 17 Nov 2025 16:33:15 -0800 Subject: [PATCH 2/5] fix: Add exception handling to plan lookup --- apps/codecov-api/api/internal/owner/serializers.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/codecov-api/api/internal/owner/serializers.py b/apps/codecov-api/api/internal/owner/serializers.py index 90e292270..311c07aab 100644 --- a/apps/codecov-api/api/internal/owner/serializers.py +++ b/apps/codecov-api/api/internal/owner/serializers.py @@ -218,7 +218,16 @@ def _get_plan_object(self, phase: dict[str, Any]) -> Plan: """Helper method to get and cache the Plan object.""" if not hasattr(self, "_cached_plan"): plan_id = phase["items"][0]["plan"] - self._cached_plan = Plan.objects.get(stripe_id=plan_id) + try: + self._cached_plan = Plan.objects.get(stripe_id=plan_id) + except Plan.DoesNotExist: + log.warning( + "Plan not found for scheduled phase", + extra={ + "plan_id": plan_id, + }, + ) + return None return self._cached_plan def get_plan(self, phase: dict[str, Any]) -> str: From 0ef5691886e751016d50564dda5d8248272f8650 Mon Sep 17 00:00:00 2001 From: Calvin Yau Date: Tue, 18 Nov 2025 13:48:38 -0800 Subject: [PATCH 3/5] fix: Prevent AttributeError --- apps/codecov-api/api/internal/owner/serializers.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/codecov-api/api/internal/owner/serializers.py b/apps/codecov-api/api/internal/owner/serializers.py index 311c07aab..cd7302933 100644 --- a/apps/codecov-api/api/internal/owner/serializers.py +++ b/apps/codecov-api/api/internal/owner/serializers.py @@ -214,7 +214,7 @@ class StripeScheduledPhaseSerializer(serializers.Serializer): quantity = serializers.SerializerMethodField() billing_rate = serializers.SerializerMethodField() - def _get_plan_object(self, phase: dict[str, Any]) -> Plan: + def _get_plan_object(self, phase: dict[str, Any]) -> Plan | None: """Helper method to get and cache the Plan object.""" if not hasattr(self, "_cached_plan"): plan_id = phase["items"][0]["plan"] @@ -227,17 +227,19 @@ def _get_plan_object(self, phase: dict[str, Any]) -> Plan: "plan_id": plan_id, }, ) - return None + self._cached_plan = None return self._cached_plan - def get_plan(self, phase: dict[str, Any]) -> str: - return self._get_plan_object(phase).marketing_name + def get_plan(self, phase: dict[str, Any]) -> str | None: + plan = self._get_plan_object(phase) + return plan.marketing_name if plan else None def get_quantity(self, phase: dict[str, Any]) -> int: return phase["items"][0]["quantity"] def get_billing_rate(self, phase: dict[str, Any]) -> str | None: - return self._get_plan_object(phase).billing_rate + plan = self._get_plan_object(phase) + return plan.billing_rate if plan else None class ScheduleDetailSerializer(serializers.Serializer): From 522d998997418fe4ccf5e1462cafda33d7844446 Mon Sep 17 00:00:00 2001 From: Calvin Yau Date: Wed, 19 Nov 2025 17:14:12 -0800 Subject: [PATCH 4/5] test: Add test for missing plan --- .../tests/views/test_account_viewset.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/apps/codecov-api/api/internal/tests/views/test_account_viewset.py b/apps/codecov-api/api/internal/tests/views/test_account_viewset.py index daa713e82..0308a6e83 100644 --- a/apps/codecov-api/api/internal/tests/views/test_account_viewset.py +++ b/apps/codecov-api/api/internal/tests/views/test_account_viewset.py @@ -530,6 +530,68 @@ def test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexi "delinquent": None, } + @patch("api.internal.owner.serializers.log") + @patch("services.billing.stripe.SubscriptionSchedule.retrieve") + @patch("services.billing.stripe.Subscription.retrieve") + def test_retrieve_account_handles_missing_plan_in_scheduled_phase( + self, mock_retrieve_subscription, mock_retrieve_schedule, log_mock + ): + owner = OwnerFactory( + admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123" + ) + self.current_owner.organizations = [owner.ownerid] + self.current_owner.save() + + subscription_params = { + "default_payment_method": None, + "cancel_at_period_end": False, + "current_period_end": 1633512445, + "latest_invoice": None, + "schedule_id": "sub_sched_456", + "collection_method": "charge_automatically", + "tax_ids": None, + } + + mock_retrieve_subscription.return_value = MockSubscription(subscription_params) + schedule_params = { + "id": 123, + "start_date": 123689126736, + "stripe_plan_id": "nonexistent_plan_id", + "quantity": 6, + } + phases = [ + {}, + { + "start_date": schedule_params["start_date"], + "items": [ + { + "plan": schedule_params["stripe_plan_id"], + "quantity": schedule_params["quantity"], + } + ], + }, + ] + + mock_retrieve_schedule.return_value = MockSchedule(schedule_params, phases) + + response = self._retrieve( + kwargs={"service": owner.service, "owner_username": owner.username} + ) + assert response.status_code == status.HTTP_200_OK + assert response.data["schedule_detail"] == { + "id": "123", + "scheduled_phase": { + "start_date": schedule_params["start_date"], + "plan": None, + "quantity": schedule_params["quantity"], + "billing_rate": None, + }, + } + log_mock.warning.assert_called_once() + call_args = log_mock.warning.call_args + assert call_args[0][0] == "Plan not found for scheduled phase" + assert call_args[1]["extra"]["plan_id"] == "nonexistent_plan_id" + def test_retrieve_account_gets_account_students(self): owner = OwnerFactory( admins=[self.current_owner.ownerid], From 34dfbd2d9ca78c632c980adbd2d9c66927363e60 Mon Sep 17 00:00:00 2001 From: Calvin Yau Date: Wed, 19 Nov 2025 18:37:20 -0800 Subject: [PATCH 5/5] feat: Add base unit price to scheduled phase payload --- apps/codecov-api/api/internal/owner/serializers.py | 5 +++++ .../api/internal/tests/views/test_account_viewset.py | 3 +++ 2 files changed, 8 insertions(+) diff --git a/apps/codecov-api/api/internal/owner/serializers.py b/apps/codecov-api/api/internal/owner/serializers.py index cd7302933..39499b158 100644 --- a/apps/codecov-api/api/internal/owner/serializers.py +++ b/apps/codecov-api/api/internal/owner/serializers.py @@ -213,6 +213,7 @@ class StripeScheduledPhaseSerializer(serializers.Serializer): plan = serializers.SerializerMethodField() quantity = serializers.SerializerMethodField() billing_rate = serializers.SerializerMethodField() + base_unit_price = serializers.SerializerMethodField() def _get_plan_object(self, phase: dict[str, Any]) -> Plan | None: """Helper method to get and cache the Plan object.""" @@ -241,6 +242,10 @@ def get_billing_rate(self, phase: dict[str, Any]) -> str | None: plan = self._get_plan_object(phase) return plan.billing_rate if plan else None + def get_base_unit_price(self, phase: dict[str, Any]) -> int | None: + plan = self._get_plan_object(phase) + return plan.base_unit_price if plan else None + class ScheduleDetailSerializer(serializers.Serializer): id = serializers.CharField() diff --git a/apps/codecov-api/api/internal/tests/views/test_account_viewset.py b/apps/codecov-api/api/internal/tests/views/test_account_viewset.py index 0308a6e83..ee6f52999 100644 --- a/apps/codecov-api/api/internal/tests/views/test_account_viewset.py +++ b/apps/codecov-api/api/internal/tests/views/test_account_viewset.py @@ -300,6 +300,7 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( "plan": "Pro", "quantity": schedule_params["quantity"], "billing_rate": "annually", + "base_unit_price": 10, }, }, "student_count": 0, @@ -446,6 +447,7 @@ def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases "quantity": schedule_params["quantity"], "start_date": schedule_params["start_date"], "billing_rate": "annually", + "base_unit_price": 10, }, }, "uses_invoice": False, @@ -585,6 +587,7 @@ def test_retrieve_account_handles_missing_plan_in_scheduled_phase( "plan": None, "quantity": schedule_params["quantity"], "billing_rate": None, + "base_unit_price": None, }, } log_mock.warning.assert_called_once()