diff --git a/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_handles_stripe_error.yaml b/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_handles_stripe_error.yaml new file mode 100644 index 0000000000..254ef9a55d --- /dev/null +++ b/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_handles_stripe_error.yaml @@ -0,0 +1,82 @@ +interactions: +- request: + body: null + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Stripe-Version: + - 2024-12-18.acacia + User-Agent: + - Stripe/v1 PythonBindings/11.4.1 + X-Stripe-Client-User-Agent: + - '{"bindings_version": "11.4.1", "lang": "python", "publisher": "stripe", "httplib": + "requests", "lang_version": "3.12.8", "platform": "Linux-6.10.14-linuxkit-aarch64-with-glibc2.36", + "uname": "Linux f69fe8d5c257 6.10.14-linuxkit #1 SMP Fri Nov 29 17:22:03 UTC + 2024 aarch64 "}' + method: GET + uri: https://api.stripe.com/v1/subscriptions/djfos?expand%5B0%5D=latest_invoice&expand%5B1%5D=customer&expand%5B2%5D=customer.invoice_settings.default_payment_method&expand%5B3%5D=customer.tax_ids + response: + body: + string: "{\n \"error\": {\n \"code\": \"resource_missing\",\n \"doc_url\": + \"https://stripe.com/docs/error-codes/resource-missing\",\n \"message\": + \"No such subscription: 'djfos'\",\n \"param\": \"id\",\n \"request_log_url\": + \"https://dashboard.stripe.com/test/logs/req_fCLmExiHliLLAy?t=1738274612\",\n + \ \"type\": \"invalid_request_error\"\n }\n}\n" + headers: + Access-Control-Allow-Credentials: + - 'true' + Access-Control-Allow-Methods: + - GET, HEAD, PUT, PATCH, POST, DELETE + Access-Control-Allow-Origin: + - '*' + Access-Control-Expose-Headers: + - Request-Id, Stripe-Manage-Version, Stripe-Should-Retry, X-Stripe-External-Auth-Required, + X-Stripe-Privileged-Session-Required + Access-Control-Max-Age: + - '300' + Cache-Control: + - no-cache, no-store + Connection: + - keep-alive + Content-Length: + - '324' + Content-Security-Policy: + - base-uri 'none'; default-src 'none'; form-action 'none'; frame-ancestors 'none'; + img-src 'self'; script-src 'self' 'report-sample'; style-src 'self'; upgrade-insecure-requests; + report-uri https://q.stripe.com/csp-violation?q=JU_aZLssk7a3_VZEeiDM3UWQN0mgWJiEG8zz5aFpDfoiI4Itt-XeW-vHYyCYd8ZJIklaArUO0YdslYml + Content-Type: + - application/json + Cross-Origin-Opener-Policy-Report-Only: + - same-origin; report-to="coop" + Date: + - Thu, 30 Jan 2025 22:03:32 GMT + Report-To: + - '{"group":"coop","max_age":8640,"endpoints":[{"url":"https://q.stripe.com/coop-report"}],"include_subdomains":true}' + Reporting-Endpoints: + - coop="https://q.stripe.com/coop-report" + Request-Id: + - req_fCLmExiHliLLAy + Server: + - nginx + Strict-Transport-Security: + - max-age=63072000; includeSubDomains; preload + Stripe-Version: + - 2024-12-18.acacia + Vary: + - Origin + X-Content-Type-Options: + - nosniff + X-Stripe-Priority-Routing-Enabled: + - 'true' + X-Stripe-Routing-Context-Priority-Tier: + - api-testmode + X-Wc: + - AB + status: + code: 404 + message: Not Found +version: 1 diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index f9d608e7a7..342c236110 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -26,21 +26,42 @@ class MockSubscription(object): - def __init__(self, subscription_params): + def __init__(self, subscription_params: dict): self.items = {"data": [{"id": "abc"}]} self.cancel_at_period_end = False self.current_period_end = 1633512445 - self.latest_invoice = subscription_params["latest_invoice"] + self.latest_invoice = subscription_params.get( + "latest_invoice", + { + "id": "in_123", + "status": "complete", + }, + ) + + default_payment_method = { + "id": "pm_123", + "card": { + "brand": "visa", + "exp_month": 12, + "exp_year": 2024, + "last4": "abcd", + }, + } self.customer = { "invoice_settings": { - "default_payment_method": subscription_params["default_payment_method"] + "default_payment_method": subscription_params.get( + "default_payment_method", default_payment_method + ) }, "id": "cus_LK&*Hli8YLIO", "discount": None, "email": None, } - self.schedule = subscription_params["schedule_id"] - self.collection_method = subscription_params["collection_method"] + self.schedule = subscription_params.get("schedule_id") + self.status = subscription_params.get("status", "active") + self.collection_method = subscription_params.get( + "collection_method", "charge_automatically" + ) self.trial_end = subscription_params.get("trial_end") customer_coupon = subscription_params.get("customer_coupon") @@ -1104,6 +1125,7 @@ def test_update_payment_method_without_body(self): response = self.client.patch(url, format="json") assert response.status_code == status.HTTP_400_BAD_REQUEST + @patch("services.billing.StripeService._is_unverified_payment_method") @patch("services.billing.stripe.Subscription.retrieve") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") @@ -1114,12 +1136,15 @@ def test_update_payment_method( modify_customer_mock, attach_payment_mock, retrieve_subscription_mock, + is_unverified_payment_method_mock, ): self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" self.current_owner.save() f = open("./services/tests/samples/stripe_invoice.json") + is_unverified_payment_method_mock.return_value = False + default_payment_method = { "card": { "brand": "visa", @@ -1435,14 +1460,15 @@ def test_update_can_change_name_and_email(self): assert self.current_owner.name == expected_name assert self.current_owner.email == expected_email + @patch("services.billing.stripe.Subscription.retrieve") @patch("services.billing.StripeService.modify_subscription") - def test_update_handles_stripe_error(self, modify_sub_mock): + def test_update_handles_stripe_error(self, retrieve_sub_mock, modify_sub_mock): code, message = 402, "Not right, wrong in fact" desired_plan = {"value": PlanName.CODECOV_PRO_MONTHLY.value, "quantity": 12} self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" self.current_owner.save() - + retrieve_sub_mock.return_value = MockSubscription({}) modify_sub_mock.side_effect = StripeError(message=message, http_status=code) response = self._update( @@ -1456,9 +1482,12 @@ def test_update_handles_stripe_error(self, modify_sub_mock): assert response.status_code == code assert response.data["detail"] == message + @patch("services.billing.stripe.Subscription.retrieve") @patch("api.internal.owner.serializers.send_sentry_webhook") @patch("services.billing.StripeService.modify_subscription") - def test_update_sentry_plan_monthly(self, modify_sub_mock, send_sentry_webhook): + def test_update_sentry_plan_monthly( + self, modify_sub_mock, send_sentry_webhook, retrieve_sub_mock + ): desired_plan = {"value": PlanName.SENTRY_MONTHLY.value, "quantity": 12} self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" @@ -1499,9 +1528,15 @@ def test_update_sentry_plan_monthly_with_users_org( ) send_sentry_webhook.assert_called_once_with(self.current_owner, org) + @patch("services.billing.stripe.Subscription.retrieve") @patch("api.internal.owner.serializers.send_sentry_webhook") @patch("services.billing.StripeService.modify_subscription") - def test_update_sentry_plan_annual(self, modify_sub_mock, send_sentry_webhook): + def test_update_sentry_plan_annual( + self, + modify_sub_mock, + send_sentry_webhook, + retrieve_sub_mock, + ): desired_plan = {"value": PlanName.SENTRY_YEARLY.value, "quantity": 12} self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" diff --git a/billing/constants.py b/billing/constants.py index 50edf67183..bc2f5b9116 100644 --- a/billing/constants.py +++ b/billing/constants.py @@ -17,6 +17,8 @@ class StripeWebhookEvents: "customer.updated", "invoice.payment_failed", "invoice.payment_succeeded", + "payment_intent.succeeded", + "setup_intent.succeeded", "subscription_schedule.created", "subscription_schedule.released", "subscription_schedule.updated", diff --git a/billing/tests/test_views.py b/billing/tests/test_views.py index ce05c6e911..36259e926f 100644 --- a/billing/tests/test_views.py +++ b/billing/tests/test_views.py @@ -1,6 +1,6 @@ import time from datetime import datetime -from unittest.mock import call, patch +from unittest.mock import Mock, call, patch import stripe from django.conf import settings @@ -12,6 +12,7 @@ from shared.plan.constants import PlanName from billing.helpers import mock_all_plans_and_tiers +from billing.views import StripeWebhookHandler from ..constants import StripeHTTPHeaders @@ -49,6 +50,9 @@ def __init__(self): def __getitem__(self, key): return getattr(self, key) + def get(self, key, default=None): + return getattr(self, key, default) + class MockPaymentMethod(object): def __init__(self, noCard=False): @@ -61,14 +65,21 @@ def __init__(self, noCard=False): def __getitem__(self, key): return getattr(self, key) + def get(self, key, default=None): + return getattr(self, key, default) + class MockPaymentIntent(object): def __init__(self, noCard=False): self.payment_method = MockPaymentMethod(noCard) + self.status = "succeeded" def __getitem__(self, key): return getattr(self, key) + def get(self, key, default=None): + return getattr(self, key, default) + class StripeWebhookHandlerTests(APITestCase): @classmethod @@ -266,6 +277,42 @@ def test_invoice_payment_succeeded_emails_delinquents(self, mocked_send_email): mocked_send_email.assert_has_calls(expected_calls) + @patch("services.billing.stripe.PaymentIntent.retrieve") + def test_invoice_payment_failed_skips_delinquency_if_payment_intent_requires_action( + self, retrieve_paymentintent_mock + ): + self.owner.delinquent = False + self.owner.save() + + retrieve_paymentintent_mock.return_value = stripe.PaymentIntent.construct_from( + { + "status": "requires_action", + "next_action": {"type": "verify_with_microdeposits"}, + }, + "payment_intent_asdf", + ) + + response = self._send_event( + payload={ + "type": "invoice.payment_failed", + "data": { + "object": { + "customer": self.owner.stripe_customer_id, + "subscription": self.owner.stripe_subscription_id, + "total": 24000, + "hosted_invoice_url": "https://stripe.com", + "payment_intent": "payment_intent_asdf", + "default_payment_method": None, + } + }, + } + ) + + self.owner.refresh_from_db() + assert response.status_code == status.HTTP_204_NO_CONTENT + assert self.owner.delinquent is False + retrieve_paymentintent_mock.assert_called_once_with("payment_intent_asdf") + @patch("services.billing.stripe.PaymentIntent.retrieve") def test_invoice_payment_failed_sets_owner_delinquent_true( self, retrieve_paymentintent_mock @@ -273,7 +320,13 @@ def test_invoice_payment_failed_sets_owner_delinquent_true( self.owner.delinquent = False self.owner.save() - retrieve_paymentintent_mock.return_value = MockPaymentIntent() + retrieve_paymentintent_mock.return_value = stripe.PaymentIntent.construct_from( + { + "status": "requires_action", + "next_action": {"type": "verify_with_microdeposits"}, + }, + "payment_intent_asdf", + ) response = self._send_event( payload={ @@ -285,6 +338,7 @@ def test_invoice_payment_failed_sets_owner_delinquent_true( "total": 24000, "hosted_invoice_url": "https://stripe.com", "payment_intent": "payment_intent_asdf", + "default_payment_method": {}, } }, } @@ -316,6 +370,7 @@ def test_invoice_payment_failed_sets_multiple_owners_delinquent_true( "total": 24000, "hosted_invoice_url": "https://stripe.com", "payment_intent": "payment_intent_asdf", + "default_payment_method": {}, } }, } @@ -354,6 +409,7 @@ def test_invoice_payment_failed_sends_email_to_admins( "total": 24000, "hosted_invoice_url": "https://stripe.com", "payment_intent": "payment_intent_asdf", + "default_payment_method": {}, } }, } @@ -428,7 +484,10 @@ def test_invoice_payment_failed_sends_email_to_admins_no_card( "default_payment_method": None, "total": 24000, "hosted_invoice_url": "https://stripe.com", - "payment_intent": "payment_intent_asdf", + "payment_intent": { + "id": "payment_intent_asdf", + "status": "succeeded", + }, } }, } @@ -488,6 +547,7 @@ def test_customer_subscription_deleted_sets_plan_to_free(self): "id": self.owner.stripe_subscription_id, "customer": self.owner.stripe_customer_id, "plan": {"name": self.owner.plan}, + "status": "active", } }, } @@ -516,6 +576,7 @@ def test_customer_subscription_deleted_sets_plan_to_free_mutliple_owner(self): "id": self.owner.stripe_subscription_id, "customer": self.owner.stripe_customer_id, "plan": {"name": self.owner.plan}, + "status": "active", } }, } @@ -550,6 +611,7 @@ def test_customer_subscription_deleted_deactivates_all_repos(self): "id": self.owner.stripe_subscription_id, "customer": self.owner.stripe_customer_id, "plan": {"name": PlanName.CODECOV_PRO_MONTHLY.value}, + "status": "active", } }, } @@ -587,6 +649,7 @@ def test_customer_subscription_deleted_deactivates_all_repos_multiple_owner(self "id": self.owner.stripe_subscription_id, "customer": self.owner.stripe_customer_id, "plan": {"name": PlanName.CODECOV_PRO_MONTHLY.value}, + "status": "active", } }, } @@ -619,6 +682,7 @@ def test_customer_subscription_deleted_no_customer(self, log_info_mock): "id": "HUH", "customer": "nah", "plan": {"name": self.owner.plan}, + "status": "active", } }, } @@ -640,6 +704,40 @@ def test_customer_created_logs_and_doesnt_crash(self): } ) + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") + def test_customer_subscription_created_early_returns_if_unverified_payment( + self, mock_has_unverified + ): + mock_has_unverified.return_value = True + self.owner.stripe_subscription_id = None + self.owner.stripe_customer_id = None + self.owner.plan = "users-basic" + self.owner.save() + + response = self._send_event( + payload={ + "type": "customer.subscription.created", + "data": { + "object": { + "id": "sub_123", + "customer": "cus_123", + "plan": {"id": "plan_H6P16wij3lUuxg"}, + "metadata": {"obo_organization": self.owner.ownerid}, + "quantity": 20, + } + }, + } + ) + + self.owner.refresh_from_db() + assert response.status_code == status.HTTP_204_NO_CONTENT + # Subscription and customer IDs should be set + assert self.owner.stripe_subscription_id == "sub_123" + assert self.owner.stripe_customer_id == "cus_123" + # But plan should not be updated since payment is unverified + assert self.owner.plan == "users-basic" + mock_has_unverified.assert_called_once() + def test_customer_subscription_created_does_nothing_if_no_plan_id(self): self.owner.stripe_subscription_id = None self.owner.stripe_customer_id = None @@ -690,7 +788,11 @@ def test_customer_subscription_created_does_nothing_if_plan_not_paid_user_plan( assert self.owner.stripe_subscription_id is None assert self.owner.stripe_customer_id is None - def test_customer_subscription_created_sets_plan_info(self): + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") + def test_customer_subscription_created_sets_plan_info( + self, has_unverified_initial_payment_method_mock + ): + has_unverified_initial_payment_method_mock.return_value = False self.owner.stripe_subscription_id = None self.owner.stripe_customer_id = None self.owner.save() @@ -724,12 +826,18 @@ def test_customer_subscription_created_sets_plan_info(self): assert self.owner.plan == plan_name @freeze_time("2023-06-19") + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("shared.plan.service.PlanService.expire_trial_when_upgrading") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_created_can_trigger_trial_expiration( - self, c_mock, pm_mock, expire_trial_when_upgrading_mock + self, + c_mock, + pm_mock, + expire_trial_when_upgrading_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False stripe_subscription_id = "FOEKDCDEQ" stripe_customer_id = "sdo050493" quantity = 20 @@ -752,11 +860,16 @@ def test_customer_subscription_created_can_trigger_trial_expiration( expire_trial_when_upgrading_mock.assert_called_once() + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_does_not_change_subscription_if_not_paid_user_plan( - self, c_mock, pm_mock + self, + c_mock, + pm_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False self.owner.plan = PlanName.BASIC_PLAN_NAME.value self.owner.plan_user_count = 0 self.owner.plan_auto_activate = False @@ -792,12 +905,18 @@ def test_customer_subscription_updated_does_not_change_subscription_if_not_paid_ invoice_settings={"default_payment_method": "pm_1LhiRsGlVGuVgOrkQguJXdeV"}, ) + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("logging.Logger.info") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_does_not_change_subscription_if_there_is_a_schedule( - self, c_mock, pm_mock, log_info_mock + self, + c_mock, + pm_mock, + log_info_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False self.owner.plan = "users-pr-inappy" self.owner.plan_user_count = 10 self.owner.plan_auto_activate = False @@ -838,11 +957,16 @@ def test_customer_subscription_updated_does_not_change_subscription_if_there_is_ extra={"stripe_webhook_event": "customer.subscription.updated"}, ) + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_sets_free_and_deactivates_all_repos_if_incomplete_expired( - self, c_mock, pm_mock + self, + c_mock, + pm_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False self.owner.plan = "users-pr-inappy" self.owner.plan_user_count = 10 self.owner.plan_auto_activate = False @@ -889,7 +1013,11 @@ def test_customer_subscription_updated_sets_free_and_deactivates_all_repos_if_in invoice_settings={"default_payment_method": "pm_1LhiRsGlVGuVgOrkQguJXdeV"}, ) - def test_customer_subscription_updated_payment_failed(self): + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") + def test_customer_subscription_updated_payment_failed( + self, has_unverified_initial_payment_method_mock + ): + has_unverified_initial_payment_method_mock.return_value = False self.owner.delinquent = False self.owner.save() @@ -923,11 +1051,16 @@ def test_customer_subscription_updated_payment_failed(self): self.owner.refresh_from_db() assert self.owner.delinquent == True + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_sets_free_and_deactivates_all_repos_if_incomplete_expired_multiple_owner( - self, c_mock, pm_mock + self, + c_mock, + pm_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False self.add_second_owner() self.owner.plan = "users-pr-inappy" self.owner.plan_user_count = 10 @@ -992,11 +1125,16 @@ def test_customer_subscription_updated_sets_free_and_deactivates_all_repos_if_in invoice_settings={"default_payment_method": "pm_1LhiRsGlVGuVgOrkQguJXdeV"}, ) + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_sets_fields_on_success( - self, c_mock, pm_mock + self, + c_mock, + pm_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False self.owner.plan = "users-free" self.owner.plan_user_count = 5 self.owner.plan_auto_activate = False @@ -1034,11 +1172,16 @@ def test_customer_subscription_updated_sets_fields_on_success( invoice_settings={"default_payment_method": "pm_1LhiRsGlVGuVgOrkQguJXdeV"}, ) + @patch("billing.views.StripeWebhookHandler._has_unverified_initial_payment_method") @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_sets_fields_on_success_multiple_owner( - self, c_mock, pm_mock + self, + c_mock, + pm_mock, + has_unverified_initial_payment_method_mock, ): + has_unverified_initial_payment_method_mock.return_value = False self.add_second_owner() self.owner.plan = "users-free" self.owner.plan_user_count = 5 @@ -1391,3 +1534,165 @@ def test_customer_update_payment_method(self, subscription_modify_mock): subscription_modify_mock.assert_called_once_with( "sub_123", default_payment_method=payment_method ) + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Invoice.retrieve") + def test_has_unverified_initial_payment_method( + self, invoice_retrieve_mock, payment_intent_retrieve_mock + ): + subscription = Mock() + subscription.latest_invoice = "inv_123" + + class MockPaymentIntent: + status = "requires_action" + + invoice_retrieve_mock.return_value = Mock(payment_intent="pi_123") + payment_intent_retrieve_mock.return_value = stripe.PaymentIntent.construct_from( + { + "status": "requires_action", + "next_action": {"type": "verify_with_microdeposits"}, + }, + "payment_intent_asdf", + ) + + handler = StripeWebhookHandler() + result = handler._has_unverified_initial_payment_method(subscription) + + assert result is True + invoice_retrieve_mock.assert_called_once_with("inv_123") + payment_intent_retrieve_mock.assert_called_once_with("pi_123") + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Invoice.retrieve") + def test_has_unverified_initial_payment_method_no_payment_intent( + self, invoice_retrieve_mock, payment_intent_retrieve_mock + ): + subscription = Mock() + subscription.latest_invoice = "inv_123" + + invoice_retrieve_mock.return_value = Mock(payment_intent=None) + + handler = StripeWebhookHandler() + result = handler._has_unverified_initial_payment_method(subscription) + + assert result is False + invoice_retrieve_mock.assert_called_once_with("inv_123") + payment_intent_retrieve_mock.assert_not_called() + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Invoice.retrieve") + def test_has_unverified_initial_payment_method_payment_intent_succeeded( + self, invoice_retrieve_mock, payment_intent_retrieve_mock + ): + subscription = stripe.Subscription.construct_from( + {"latest_invoice": "inv_123"}, "sub_123" + ) + + invoice_retrieve_mock.return_value = stripe.Invoice.construct_from( + {"payment_intent": "pi_123"}, "inv_123" + ) + payment_intent_retrieve_mock.return_value = stripe.PaymentIntent.construct_from( + {"status": "succeeded"}, "payment_intent_asdf" + ) + + handler = StripeWebhookHandler() + result = handler._has_unverified_initial_payment_method(subscription) + + assert result is False + invoice_retrieve_mock.assert_called_once_with("inv_123") + payment_intent_retrieve_mock.assert_called_once_with("pi_123") + + @patch("services.billing.stripe.PaymentMethod.attach") + @patch("services.billing.stripe.Customer.modify") + @patch("services.billing.stripe.Subscription.modify") + @patch("services.billing.stripe.PaymentMethod.retrieve") + def test_check_and_handle_delayed_notification_payment_methods( + self, + payment_method_retrieve_mock, + subscription_modify_mock, + customer_modify_mock, + payment_method_attach_mock, + ): + class MockPaymentMethod: + type = "us_bank_account" + us_bank_account = {} + id = "pm_123" + + payment_method_retrieve_mock.return_value = MockPaymentMethod() + + self.owner.stripe_subscription_id = "sub_123" + self.owner.stripe_customer_id = "cus_123" + self.owner.save() + + handler = StripeWebhookHandler() + handler._check_and_handle_delayed_notification_payment_methods( + "cus_123", "pm_123" + ) + + payment_method_retrieve_mock.assert_called_once_with("pm_123") + payment_method_attach_mock.assert_called_once_with( + payment_method_retrieve_mock.return_value, customer="cus_123" + ) + customer_modify_mock.assert_called_once_with( + "cus_123", + invoice_settings={ + "default_payment_method": payment_method_retrieve_mock.return_value + }, + ) + subscription_modify_mock.assert_called_once_with( + "sub_123", default_payment_method=payment_method_retrieve_mock.return_value + ) + + @patch( + "billing.views.StripeWebhookHandler._check_and_handle_delayed_notification_payment_methods" + ) + @patch("logging.Logger.info") + def test_payment_intent_succeeded( + self, log_info_mock, check_and_handle_delayed_notification_mock + ): + class MockPaymentIntent: + id = "pi_123" + customer = "cus_123" + payment_method = "pm_123" + + handler = StripeWebhookHandler() + handler.payment_intent_succeeded(MockPaymentIntent()) + + check_and_handle_delayed_notification_mock.assert_called_once_with( + "cus_123", "pm_123" + ) + log_info_mock.assert_called_once_with( + "Payment intent succeeded", + extra=dict( + stripe_customer_id="cus_123", + payment_intent_id="pi_123", + payment_method_type="pm_123", + ), + ) + + @patch( + "billing.views.StripeWebhookHandler._check_and_handle_delayed_notification_payment_methods" + ) + @patch("logging.Logger.info") + def test_setup_intent_succeeded( + self, log_info_mock, check_and_handle_delayed_notification_mock + ): + class MockSetupIntent: + id = "seti_123" + customer = "cus_123" + payment_method = "pm_123" + + handler = StripeWebhookHandler() + handler.setup_intent_succeeded(MockSetupIntent()) + + check_and_handle_delayed_notification_mock.assert_called_once_with( + "cus_123", "pm_123" + ) + log_info_mock.assert_called_once_with( + "Setup intent succeeded", + extra=dict( + stripe_customer_id="cus_123", + setup_intent_id="seti_123", + payment_method_type="pm_123", + ), + ) diff --git a/billing/views.py b/billing/views.py index 8060ee4b4c..56427ff6a4 100644 --- a/billing/views.py +++ b/billing/views.py @@ -83,6 +83,31 @@ def invoice_payment_succeeded(self, invoice: stripe.Invoice) -> None: ) def invoice_payment_failed(self, invoice: stripe.Invoice) -> None: + """ + Stripe invoice.payment_failed webhook event is emitted when an invoice payment fails + (initial or recurring). Note that delayed payment methods (including ACH with + microdeposits) may have a failed initial invoice until the account is verified. + """ + if invoice.default_payment_method is None: + if invoice.payment_intent: + payment_intent = stripe.PaymentIntent.retrieve(invoice.payment_intent) + if ( + payment_intent + and payment_intent.get("status") == "requires_action" + and payment_intent.get("next_action", {}).get("type") + == "verify_with_microdeposits" + ): + log.info( + "Invoice payment failed but still awaiting known customer action, skipping Delinquency actions", + extra=dict( + stripe_customer_id=invoice.customer, + stripe_subscription_id=invoice.subscription, + payment_intent_id=invoice.payment_intent, + payment_intent_status=payment_intent.status, + ), + ) + return + log.info( "Invoice Payment Failed - Setting Delinquency status True", extra=dict( @@ -104,12 +129,12 @@ def invoice_payment_failed(self, invoice: stripe.Invoice) -> None: payment_intent = stripe.PaymentIntent.retrieve( invoice["payment_intent"], expand=["payment_method"] ) - card = ( - payment_intent.payment_method.card - if payment_intent.payment_method - and not isinstance(payment_intent.payment_method, str) - else None - ) + + try: + card = payment_intent.payment_method.card + except AttributeError: + card = None + template_vars = { "amount": invoice.total / 100, "card_type": card.brand if card else None, @@ -138,11 +163,18 @@ def invoice_payment_failed(self, invoice: stripe.Invoice) -> None: ) def customer_subscription_deleted(self, subscription: stripe.Subscription) -> None: + """ + Stripe customer.subscription.deleted webhook event is emitted when a subscription is deleted. + This happens when an org goes from paid to free (see payment_service.delete_subscription) + or when cleaning up an incomplete subscription that never activated (e.g., abandoned async + ACH microdeposits verification). + """ log.info( "Customer Subscription Deleted - Setting free plan and deactivating repos for stripe customer", extra=dict( stripe_subscription_id=subscription.id, stripe_customer_id=subscription.customer, + previous_subscription_status=subscription.status, ), ) owners: QuerySet[Owner] = Owner.objects.filter( @@ -253,6 +285,10 @@ def customer_created(self, customer: stripe.Customer) -> None: log.info("Customer created", extra=dict(stripe_customer_id=customer.id)) def customer_subscription_created(self, subscription: stripe.Subscription) -> None: + """ + Stripe customer.subscription.created webhook event is emitted when a subscription is created. + This happens when an owner completes a CheckoutSession for a new subscription. + """ sub_item_plan_id = subscription.plan.id if not sub_item_plan_id: @@ -289,11 +325,24 @@ def customer_subscription_created(self, subscription: stripe.Subscription) -> No quantity=subscription.quantity, ), ) + # add the subscription_id and customer_id to the owner owner = Owner.objects.get(ownerid=subscription.metadata.get("obo_organization")) owner.stripe_subscription_id = subscription.id owner.stripe_customer_id = subscription.customer owner.save() + # We may reach here if the subscription was created with a payment method + # that is awaiting verification (e.g. ACH microdeposits) + if self._has_unverified_initial_payment_method(subscription): + log.info( + "Subscription has pending initial payment verification - will upgrade plan after initial invoice payment", + extra=dict( + subscription_id=subscription.id, + customer_id=subscription.customer, + ), + ) + return + plan_service = PlanService(current_org=owner) plan_service.expire_trial_when_upgrading() @@ -311,7 +360,34 @@ def customer_subscription_created(self, subscription: stripe.Subscription) -> No self._log_updated([owner]) + def _has_unverified_initial_payment_method( + self, subscription: stripe.Subscription + ) -> bool: + """ + Helper method to check if a subscription's latest invoice has a payment intent + that requires verification (e.g. ACH microdeposits). This indicates that + there is an unverified payment method from the initial CheckoutSession. + """ + latest_invoice = stripe.Invoice.retrieve(subscription.latest_invoice) + if latest_invoice and latest_invoice.payment_intent: + payment_intent = stripe.PaymentIntent.retrieve( + latest_invoice.payment_intent + ) + return ( + payment_intent + and payment_intent.get("status") == "requires_action" + and payment_intent.get("next_action") + and payment_intent.get("next_action", {}).get("type") + == "verify_with_microdeposits" + ) + return False + def customer_subscription_updated(self, subscription: stripe.Subscription) -> None: + """ + Stripe customer.subscription.updated webhook event is emitted when a subscription is updated. + This can happen when an owner updates the subscription's default payment method using our + update_payment_method api + """ owners: QuerySet[Owner] = Owner.objects.filter( stripe_subscription_id=subscription.id, stripe_customer_id=subscription.customer, @@ -327,6 +403,16 @@ def customer_subscription_updated(self, subscription: stripe.Subscription) -> No ) return + if self._has_unverified_initial_payment_method(subscription): + log.info( + "Subscription has pending initial payment verification - will upgrade plan after initial invoice payment", + extra=dict( + subscription_id=subscription.id, + customer_id=subscription.customer, + ), + ) + return + indication_of_payment_failure = getattr(subscription, "pending_update", None) if indication_of_payment_failure: # payment failed, raise this to user by setting as delinquent @@ -445,6 +531,74 @@ def checkout_session_completed( self._log_updated([owner]) + def _check_and_handle_delayed_notification_payment_methods( + self, customer_id: str, payment_method_id: str + ): + """ + Helper method to handle payment methods that require delayed verification (like ACH). + When verification succeeds, this attaches the payment method to the customer and sets + it as the default payment method for both the customer and subscription. + """ + owner = Owner.objects.get(stripe_customer_id=customer_id) + payment_method = stripe.PaymentMethod.retrieve(payment_method_id) + + is_us_bank_account = payment_method.type == "us_bank_account" and hasattr( + payment_method, "us_bank_account" + ) + + should_set_as_default = is_us_bank_account + + if should_set_as_default: + # attach the payment method + set as default on the invoice and subscription + stripe.PaymentMethod.attach( + payment_method, customer=owner.stripe_customer_id + ) + stripe.Customer.modify( + owner.stripe_customer_id, + invoice_settings={"default_payment_method": payment_method}, + ) + stripe.Subscription.modify( + owner.stripe_subscription_id, default_payment_method=payment_method + ) + + def payment_intent_succeeded(self, payment_intent: stripe.PaymentIntent) -> None: + """ + Stripe payment_intent.succeeded webhook event is emitted when a + payment intent goes to a success state. + We create a Stripe PaymentIntent for the initial checkout session. + """ + log.info( + "Payment intent succeeded", + extra=dict( + stripe_customer_id=payment_intent.customer, + payment_intent_id=payment_intent.id, + payment_method_type=payment_intent.payment_method, + ), + ) + + self._check_and_handle_delayed_notification_payment_methods( + payment_intent.customer, payment_intent.payment_method + ) + + def setup_intent_succeeded(self, setup_intent: stripe.SetupIntent) -> None: + """ + Stripe setup_intent.succeeded webhook event is emitted when a setup intent + goes to a success state. We create a Stripe SetupIntent for the gazebo UI + PaymentElement to modify payment methods. + """ + log.info( + "Setup intent succeeded", + extra=dict( + stripe_customer_id=setup_intent.customer, + setup_intent_id=setup_intent.id, + payment_method_type=setup_intent.payment_method, + ), + ) + + self._check_and_handle_delayed_notification_payment_methods( + setup_intent.customer, setup_intent.payment_method + ) + def post(self, request: HttpRequest, *args: Any, **kwargs: Any) -> Response: if settings.STRIPE_ENDPOINT_SECRET is None: log.critical( diff --git a/services/billing.py b/services/billing.py index 2d35f40b5a..184ba7acb7 100644 --- a/services/billing.py +++ b/services/billing.py @@ -546,8 +546,37 @@ def create_checkout_session(self, owner: Owner, desired_plan): ) return session["id"] + def _is_unverified_payment_method(self, payment_method_id: str) -> bool: + payment_method = stripe.PaymentMethod.retrieve(payment_method_id) + + is_us_bank_account = payment_method.type == "us_bank_account" and hasattr( + payment_method, "us_bank_account" + ) + if is_us_bank_account: + setup_intents = stripe.SetupIntent.list( + payment_method=payment_method_id, limit=1 + ) + + try: + latest_intent = setup_intents.data[0] + if ( + latest_intent.status == "requires_action" + and latest_intent.next_action + and latest_intent.next_action.type == "verify_with_microdeposits" + ): + return True + except Exception as e: + log.error( + "Error retrieving latest setup intent", + payment_method_id=payment_method_id, + extra=dict(error=e), + ) + return False + + return False + @_log_stripe_error - def update_payment_method(self, owner: Owner, payment_method): + def update_payment_method(self, owner: Owner, payment_method: str) -> None: log.info( "Stripe update payment method for owner", extra=dict( @@ -567,15 +596,21 @@ def update_payment_method(self, owner: Owner, payment_method): ), ) return None - # attach the payment method + set as default on the invoice and subscription - stripe.PaymentMethod.attach(payment_method, customer=owner.stripe_customer_id) - stripe.Customer.modify( - owner.stripe_customer_id, - invoice_settings={"default_payment_method": payment_method}, - ) - stripe.Subscription.modify( - owner.stripe_subscription_id, default_payment_method=payment_method - ) + + # do not set as default if the new payment method is unverified (e.g., awaiting microdeposits) + should_set_as_default = not self._is_unverified_payment_method(payment_method) + + if should_set_as_default: + stripe.PaymentMethod.attach( + payment_method, customer=owner.stripe_customer_id + ) + stripe.Customer.modify( + owner.stripe_customer_id, + invoice_settings={"default_payment_method": payment_method}, + ) + stripe.Subscription.modify( + owner.stripe_subscription_id, default_payment_method=payment_method + ) log.info( f"Successfully updated payment method for owner {owner.ownerid} by user #{self.requesting_user.ownerid}", extra=dict( @@ -895,8 +930,19 @@ def update_plan(self, owner, desired_plan): plan_service.set_default_plan_data() else: if owner.stripe_subscription_id is not None: + # if the existing subscription is incomplete, clean it up and create a new checkout session + subscription = self.payment_service.get_subscription(owner) + + if subscription and subscription.status == "incomplete": + self._cleanup_incomplete_subscription(subscription, owner) + return self.payment_service.create_checkout_session( + owner, desired_plan + ) + + # if the existing subscription is complete, modify the plan self.payment_service.modify_subscription(owner, desired_plan) else: + # if the owner has no subscription, create a new checkout session return self.payment_service.create_checkout_session(owner, desired_plan) def update_payment_method(self, owner, payment_method): @@ -940,3 +986,46 @@ def create_setup_intent(self, owner: Owner): See https://docs.stripe.com/api/setup_intents/create """ return self.payment_service.create_setup_intent(owner) + + def _cleanup_incomplete_subscription( + self, subscription: stripe.Subscription, owner: Owner + ): + try: + payment_intent_id = subscription.latest_invoice.payment_intent + except Exception as e: + log.error( + "Latest invoice is missing payment intent id", + extra=dict(error=e), + ) + return None + + payment_intent = stripe.PaymentIntent.retrieve(payment_intent_id) + if payment_intent.status == "requires_action": + log.info( + "Subscription has pending payment verification", + extra=dict( + subscription_id=subscription.get("id"), + payment_intent_id=payment_intent.get("id"), + payment_intent_status=payment_intent.get("status"), + ), + ) + try: + # Delete the subscription, which also removes the + # pending payment method and unverified payment intent + stripe.Subscription.delete(subscription) + log.info( + "Deleted incomplete subscription", + extra=dict( + subscription_id=subscription.get("id"), + payment_intent_id=payment_intent.get("id"), + ), + ) + except Exception as e: + log.error( + "Failed to delete subscription", + extra=dict( + subscription_id=subscription.get("id"), + payment_intent_id=payment_intent.get("id"), + error=str(e), + ), + ) diff --git a/services/tests/samples/stripe_invoice.json b/services/tests/samples/stripe_invoice.json index 2947637136..289f59d5e3 100644 --- a/services/tests/samples/stripe_invoice.json +++ b/services/tests/samples/stripe_invoice.json @@ -97,7 +97,10 @@ "next_payment_attempt": null, "number": "EF0A41E-0001", "paid": true, - "payment_intent": null, + "payment_intent": { + "id": "pi_3P4567890123456789012345", + "status": "completed" + }, "period_end": 1489789420, "period_start": 1487370220, "post_payment_credit_notes_amount": 0, @@ -214,7 +217,10 @@ "next_payment_attempt": null, "number": "EF0A41E-0001", "paid": true, - "payment_intent": null, + "payment_intent": { + "id": "pi_3P4567890123456789012345", + "status": "completed" + }, "period_end": 1489789420, "period_start": 1487370220, "post_payment_credit_notes_amount": 0, @@ -351,7 +357,10 @@ "on_behalf_of": null, "paid": false, "paid_out_of_band": false, - "payment_intent": null, + "payment_intent": { + "id": "pi_3P4567890123456789012345", + "status": "completed" + }, "payment_settings": { "default_mandate": null, "payment_method_options": null, diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 2055e18deb..e856a8d0e4 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, call, patch import requests +import stripe from django.conf import settings from django.test import TestCase from freezegun import freeze_time @@ -106,7 +107,7 @@ "next_payment_attempt": None, "number": "EF0A41E-0001", "paid": True, - "payment_intent": None, + "payment_intent": {"id": "pi_3P4567890123456789012345", "status": "completed"}, "period_end": 1489789420, "period_start": 1487370220, "post_payment_credit_notes_amount": 0, @@ -1622,8 +1623,13 @@ def test_update_payment_method_when_no_subscription(self): @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") @patch("services.billing.stripe.Subscription.modify") + @patch("services.billing.StripeService._is_unverified_payment_method") def test_update_payment_method( - self, modify_sub_mock, modify_customer_mock, attach_payment_mock + self, + is_unverified_mock, + modify_sub_mock, + modify_customer_mock, + attach_payment_mock, ): payment_method_id = "pm_1234567" subscription_id = "sub_abc" @@ -1631,6 +1637,7 @@ def test_update_payment_method( owner = OwnerFactory( stripe_subscription_id=subscription_id, stripe_customer_id=customer_id ) + is_unverified_mock.return_value = False self.stripe.update_payment_method(owner, payment_method_id) attach_payment_mock.assert_called_once_with( payment_method_id, customer=customer_id @@ -1643,6 +1650,65 @@ def test_update_payment_method( subscription_id, default_payment_method=payment_method_id ) + @patch("services.billing.stripe.PaymentMethod.attach") + @patch("services.billing.stripe.Customer.modify") + @patch("services.billing.stripe.Subscription.modify") + @patch("services.billing.stripe.PaymentMethod.retrieve") + @patch("services.billing.stripe.SetupIntent.list") + def test_update_payment_method_with_unverified_payment_method( + self, + setup_intent_list_mock, + payment_method_retrieve_mock, + modify_sub_mock, + modify_customer_mock, + attach_payment_mock, + ): + # Define the mock return values + setup_intent_list_mock.return_value = MagicMock( + data=[ + MagicMock( + status="requires_action", + next_action=MagicMock( + type="verify_with_microdeposits", + verify_with_microdeposits=MagicMock( + hosted_verification_url="https://verify.stripe.com/1" + ), + ), + ) + ] + ) + payment_method_retrieve_mock.return_value = MagicMock( + type="us_bank_account", + us_bank_account=MagicMock( + status="requires_action", + next_action=MagicMock( + type="verify_with_microdeposits", + verify_with_microdeposits=MagicMock( + hosted_verification_url="https://verify.stripe.com/1" + ), + ), + ), + ) + modify_sub_mock.return_value = MagicMock() + modify_customer_mock.return_value = MagicMock() + attach_payment_mock.return_value = MagicMock() + + # Create a mock owner object + subscription_id = "sub_abc" + customer_id = "cus_abc" + owner = OwnerFactory( + stripe_subscription_id=subscription_id, stripe_customer_id=customer_id + ) + + result = self.stripe.update_payment_method(owner, "abc") + + assert result is None + assert payment_method_retrieve_mock.called + assert setup_intent_list_mock.called + assert not attach_payment_mock.called + assert not modify_customer_mock.called + assert not modify_sub_mock.called + def test_update_email_address_with_invalid_email(self): owner = OwnerFactory(stripe_subscription_id=None) assert self.stripe.update_email_address(owner, "not-an-email") is None @@ -2107,8 +2173,10 @@ def test_update_plan_to_users_basic_sets_plan_if_no_subscription_id( @patch("services.tests.test_billing.MockPaymentService.create_checkout_session") @patch("services.tests.test_billing.MockPaymentService.modify_subscription") @patch("services.tests.test_billing.MockPaymentService.delete_subscription") + @patch("services.tests.test_billing.MockPaymentService.get_subscription") def test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists( self, + get_subscription_mock, delete_subscription_mock, modify_subscription_mock, create_checkout_session_mock, @@ -2116,8 +2184,20 @@ def test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists( ): owner = OwnerFactory(stripe_subscription_id=10) desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10} - self.billing_service.update_plan(owner, desired_plan) + get_subscription_mock.return_value = stripe.util.convert_to_stripe_object( + { + "schedule": None, + "current_period_start": 1489799420, + "current_period_end": 1492477820, + "quantity": 10, + "name": PlanName.CODECOV_PRO_YEARLY.value, + "id": 215, + "status": "active", + } + ) + + self.billing_service.update_plan(owner, desired_plan) modify_subscription_mock.assert_called_once_with(owner, desired_plan) set_default_plan_data.assert_not_called() @@ -2145,6 +2225,122 @@ def test_update_plan_creates_checkout_session_if_user_plan_and_no_subscription( delete_subscription_mock.assert_not_called() modify_subscription_mock.assert_not_called() + @patch("services.tests.test_billing.MockPaymentService.get_subscription") + @patch("services.tests.test_billing.MockPaymentService.create_checkout_session") + @patch("services.tests.test_billing.MockPaymentService.modify_subscription") + @patch("services.billing.BillingService._cleanup_incomplete_subscription") + def test_update_plan_cleans_up_incomplete_subscription_and_creates_new_checkout( + self, + cleanup_incomplete_mock, + modify_subscription_mock, + create_checkout_session_mock, + get_subscription_mock, + ): + owner = OwnerFactory(stripe_subscription_id="sub_123") + desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10} + + subscription = stripe.Subscription.construct_from( + {"status": "incomplete"}, "fake_api_key" + ) + get_subscription_mock.return_value = subscription + + self.billing_service.update_plan(owner, desired_plan) + + cleanup_incomplete_mock.assert_called_once_with(subscription, owner) + create_checkout_session_mock.assert_called_once_with(owner, desired_plan) + modify_subscription_mock.assert_not_called() + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Subscription.delete") + def test_cleanup_incomplete_subscription(self, delete_mock, retrieve_mock): + owner = OwnerFactory(stripe_subscription_id="sub_123") + + payment_intent = stripe.PaymentIntent.construct_from( + {"id": "pi_123", "status": "requires_action"}, "fake_api_key" + ) + subscription = stripe.Subscription.construct_from( + {"id": "abcd", "latest_invoice": {"payment_intent": "pi_123"}}, + "fake_api_key", + ) + retrieve_mock.return_value = payment_intent + + self.billing_service._cleanup_incomplete_subscription(subscription, owner) + + retrieve_mock.assert_called_once_with("pi_123") + delete_mock.assert_called_once_with(subscription) + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Subscription.delete") + def test_cleanup_incomplete_subscription_no_latest_invoice( + self, delete_mock, retrieve_mock + ): + owner = OwnerFactory(stripe_subscription_id="sub_123") + + subscription = stripe.Subscription.construct_from( + {"id": "sub_123"}, "fake_api_key" + ) + + result = self.billing_service._cleanup_incomplete_subscription( + subscription, owner + ) + + assert result is None + delete_mock.assert_not_called() + retrieve_mock.assert_not_called() + assert owner.stripe_subscription_id == "sub_123" + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Subscription.delete") + def test_cleanup_incomplete_subscription_no_payment_intent( + self, delete_mock, retrieve_mock + ): + owner = OwnerFactory(stripe_subscription_id="sub_123") + + class MockSubscription: + id = "sub_123" + + def get(self, key): + if key == "latest_invoice": + return {"payment_intent": None} + return None + + subscription = MockSubscription() + + result = self.billing_service._cleanup_incomplete_subscription( + subscription, owner + ) + + assert result is None + delete_mock.assert_not_called() + retrieve_mock.assert_not_called() + assert owner.stripe_subscription_id == "sub_123" + + @patch("services.billing.stripe.PaymentIntent.retrieve") + @patch("services.billing.stripe.Subscription.delete") + def test_cleanup_incomplete_subscription_delete_fails( + self, delete_mock, retrieve_mock + ): + owner = OwnerFactory(stripe_subscription_id="sub_123") + + payment_intent = stripe.PaymentIntent.construct_from( + {"id": "pi_123", "status": "requires_action"}, "fake_api_key" + ) + subscription = stripe.Subscription.construct_from( + {"id": "abcd", "latest_invoice": {"payment_intent": "pi_123"}}, + "fake_api_key", + ) + retrieve_mock.return_value = payment_intent + delete_mock.side_effect = Exception("Delete failed") + + result = self.billing_service._cleanup_incomplete_subscription( + subscription, owner + ) + + assert result is None + retrieve_mock.assert_called_once_with("pi_123") + delete_mock.assert_called_once_with(subscription) + assert owner.stripe_subscription_id == "sub_123" + @patch("shared.plan.service.PlanService.set_default_plan_data") @patch("services.tests.test_billing.MockPaymentService.create_checkout_session") @patch("services.tests.test_billing.MockPaymentService.modify_subscription")