diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index 5fb5ccbdfc..30983263ad 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -45,6 +45,10 @@ def __init__(self, subscription_params): if customer_coupon: self.customer["discount"] = {"coupon": customer_coupon} + pending_update = subscription_params.get("pending_update") + if pending_update: + self.pending_update = pending_update + def __getitem__(self, key): return getattr(self, key) @@ -755,6 +759,7 @@ def test_update_can_upgrade_to_paid_plan_for_existing_customer_and_set_plan_info } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + modify_subscription_mock.return_value = MockSubscription(subscription_params) response = self._update( kwargs={ @@ -774,6 +779,69 @@ def test_update_can_upgrade_to_paid_plan_for_existing_customer_and_set_plan_info assert self.current_owner.plan == desired_plan["value"] assert self.current_owner.plan_user_count == desired_plan["quantity"] + @patch("services.billing.stripe.Subscription.retrieve") + @patch("services.billing.stripe.Subscription.modify") + def test_upgrade_payment_failure( + self, modify_subscription_mock, retrieve_subscription_mock + ): + 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.plan = PlanName.CODECOV_PRO_MONTHLY.value + self.current_owner.plan_user_count = 8 + self.current_owner.delinquent = False + self.current_owner.save() + + f = open("./services/tests/samples/stripe_invoice.json") + + default_payment_method = { + "card": { + "brand": "visa", + "exp_month": 12, + "exp_year": 2024, + "last4": "abcd", + "should be": "removed", + } + } + subscription_params = { + "default_payment_method": default_payment_method, + "latest_invoice": json.load(f)["data"][0], + "schedule_id": None, + "collection_method": "charge_automatically", + "tax_ids": None, + "pending_update": { + "expires_at": 1571194285, + "subscription_items": [ + { + "id": "si_09IkI4u3ZypJUk5onGUZpe8O", + "price": "price_CBb6IXqvTLXp3f", + } + ], + }, + } + + retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + modify_subscription_mock.return_value = MockSubscription(subscription_params) + + response = self._update( + kwargs={ + "service": self.current_owner.service, + "owner_username": self.current_owner.username, + }, + data={"plan": desired_plan}, + ) + + modify_subscription_mock.assert_called_once() + + assert response.status_code == status.HTTP_200_OK + assert response.data["plan"]["value"] == desired_plan["value"] + assert response.data["plan"]["quantity"] == 8 + + self.current_owner.refresh_from_db() + assert self.current_owner.plan == desired_plan["value"] + assert self.current_owner.plan_user_count == 8 + assert self.current_owner.delinquent == True + def test_update_requires_quantity_if_updating_to_paid_plan(self): desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value} response = self._update( diff --git a/billing/tests/test_views.py b/billing/tests/test_views.py index 46bdcf175f..ec3c1b04b5 100644 --- a/billing/tests/test_views.py +++ b/billing/tests/test_views.py @@ -443,6 +443,40 @@ 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): + self.owner.delinquent = False + self.owner.save() + + self._send_event( + payload={ + "type": "customer.subscription.updated", + "data": { + "object": { + "id": self.owner.stripe_subscription_id, + "customer": self.owner.stripe_customer_id, + "plan": {"id": "?"}, + "metadata": {"obo_organization": self.owner.ownerid}, + "quantity": 20, + "status": "active", + "schedule": None, + "default_payment_method": "pm_1LhiRsGlVGuVgOrkQguJXdeV", + "pending_update": { + "expires_at": 1571194285, + "subscription_items": [ + { + "id": "si_09IkI4u3ZypJUk5onGUZpe8O", + "price": "price_CBb6IXqvTLXp3f", + } + ], + }, + } + }, + } + ) + + self.owner.refresh_from_db() + assert self.owner.delinquent == True + @patch("services.billing.stripe.PaymentMethod.attach") @patch("services.billing.stripe.Customer.modify") def test_customer_subscription_updated_sets_fields_on_success( diff --git a/billing/views.py b/billing/views.py index 38a97ddf50..a3faeae8af 100644 --- a/billing/views.py +++ b/billing/views.py @@ -222,9 +222,20 @@ def customer_subscription_updated(self, subscription: stripe.Subscription) -> No stripe_customer_id=subscription.customer, ) + indication_of_payment_failure = getattr(subscription, "pending_update", None) + if indication_of_payment_failure: + # payment failed, raise this to user by setting as delinquent + owner.delinquent = True + owner.save() + log.info( + f"Stripe subscription upgrade failed for owner {owner.ownerid}", + extra=dict(pending_update=indication_of_payment_failure), + ) + return + # Properly attach the payment method on the customer - # This hook will be called after a checkout session completes, updating the subscription created - # with it + # This hook will be called after a checkout session completes, + # updating the subscription created with it default_payment_method = subscription.default_payment_method if default_payment_method and owner.stripe_customer_id is not None: stripe.PaymentMethod.attach( @@ -235,11 +246,9 @@ def customer_subscription_updated(self, subscription: stripe.Subscription) -> No invoice_settings={"default_payment_method": default_payment_method}, ) - subscription_schedule_id = subscription.schedule - plan_service = PlanService(current_org=owner) - # Only update if there isn't a scheduled subscription - if not subscription_schedule_id: + if not subscription.schedule: + plan_service = PlanService(current_org=owner) if subscription.status == "incomplete_expired": log.info( "Subscription status updated to incomplete_expired, cancelling to free", @@ -267,9 +276,7 @@ def customer_subscription_updated(self, subscription: stripe.Subscription) -> No return plan_name = settings.STRIPE_PLAN_VALS[sub_item_plan_id] - plan_service.update_plan(name=plan_name, user_count=subscription.quantity) - log.info( "Successfully updated customer subscription", extra=dict( diff --git a/services/billing.py b/services/billing.py index 29402423c8..7352af132e 100644 --- a/services/billing.py +++ b/services/billing.py @@ -225,7 +225,7 @@ def modify_subscription(self, owner, desired_plan): log.info( f"Updating Stripe subscription for owner {owner.ownerid} to {desired_plan['value']} by user #{self.requesting_user.ownerid}" ) - stripe.Subscription.modify( + subscription = stripe.Subscription.modify( owner.stripe_subscription_id, cancel_at_period_end=False, items=[ @@ -237,16 +237,31 @@ def modify_subscription(self, owner, desired_plan): ], metadata=self._get_checkout_session_and_subscription_metadata(owner), proration_behavior=proration_behavior, + payment_behavior="pending_if_incomplete", ) - - plan_service = PlanService(current_org=owner) - plan_service.update_plan( - name=desired_plan["value"], user_count=desired_plan["quantity"] - ) - log.info( - f"Stripe subscription modified successfully for owner {owner.ownerid} by user #{self.requesting_user.ownerid}" + f"Stripe subscription upgrade attempted for owner {owner.ownerid} by user #{self.requesting_user.ownerid}" ) + indication_of_payment_failure = getattr( + subscription, "pending_update", None + ) + if indication_of_payment_failure: + # payment failed, raise this to user by setting as delinquent + owner.delinquent = True + owner.save() + log.info( + f"Stripe subscription upgrade failed for owner {owner.ownerid} by user #{self.requesting_user.ownerid}", + extra=dict(pending_update=indication_of_payment_failure), + ) + else: + # payment successful + plan_service = PlanService(current_org=owner) + plan_service.update_plan( + name=desired_plan["value"], user_count=desired_plan["quantity"] + ) + log.info( + f"Stripe subscription upgraded successfully for owner {owner.ownerid} by user #{self.requesting_user.ownerid}" + ) else: if not subscription_schedule_id: schedule = stripe.SubscriptionSchedule.create( @@ -431,7 +446,7 @@ def update_payment_method(self, owner: Owner, payment_method): owner.stripe_subscription_id, default_payment_method=payment_method ) log.info( - "Successfully updated payment method for owner {owner.ownerid} by user #{self.requesting_user.ownerid}", + f"Successfully updated payment method for owner {owner.ownerid} by user #{self.requesting_user.ownerid}", extra=dict( owner_id=owner.ownerid, user_id=self.requesting_user.ownerid, diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 0f44d22d17..204eb05d21 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -147,6 +147,16 @@ def __getitem__(self, key): return getattr(self, key) +class MockFailedSubscriptionUpgrade(object): + def __init__(self, subscription_params): + self.id = subscription_params["id"] + self.object = subscription_params["object"] + self.pending_update = subscription_params["pending_update"] + + def __getitem__(self, key): + return getattr(self, key) + + class StripeServiceTests(TestCase): def setUp(self): self.user = OwnerFactory() @@ -178,6 +188,7 @@ def _assert_subscription_modify( "obo": self.user.ownerid, }, proration_behavior="always_invoice", + payment_behavior="pending_if_incomplete", ) def _assert_schedule_modify( @@ -360,6 +371,7 @@ def test_modify_subscription_without_schedule_increases_user_count_immediately( } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value desired_user_count = 20 @@ -402,6 +414,7 @@ def test_modify_subscription_without_schedule_upgrades_plan_immediately( } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value desired_user_count = 10 @@ -416,6 +429,120 @@ def test_modify_subscription_without_schedule_upgrades_plan_immediately( assert owner.plan == desired_plan_name assert owner.plan_user_count == desired_user_count + @patch("services.billing.stripe.Subscription.modify") + @patch("services.billing.stripe.Subscription.retrieve") + def test_modify_subscription_payment_failure( + self, + retrieve_subscription_mock, + subscription_modify_mock, + ): + original_user_count = 10 + original_plan = PlanName.CODECOV_PRO_YEARLY.value + owner = OwnerFactory( + plan=original_plan, + plan_user_count=original_user_count, + stripe_subscription_id="33043sdf", + delinquent=False, + ) + schedule_id = None + current_subscription_start_date = 1639628096 + current_subscription_end_date = 1644107871 + subscription_params = { + "schedule_id": schedule_id, + "start_date": current_subscription_start_date, + "end_date": current_subscription_end_date, + "quantity": original_user_count, + "name": original_plan, + "id": 105, + } + retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + + subscription_response = { + "id": 105, + "object": "subscription", + "application_fee_percent": None, + "pending_update": { + "expires_at": 1571194285, + "subscription_items": [ + { + "id": "si_09IkI4u3ZypJUk5onGUZpe8O", + "price": "price_CBb6IXqvTLXp3f", + } + ], + }, + } + subscription_modify_mock.return_value = MockFailedSubscriptionUpgrade( + subscription_response + ) + + desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value + desired_user_count = 20 + desired_plan = {"value": desired_plan_name, "quantity": desired_user_count} + self.stripe.modify_subscription(owner, desired_plan) + + self._assert_subscription_modify( + subscription_modify_mock, owner, subscription_params, desired_plan + ) + + # changes to plan are rejected, owner becomes delinquent + owner.refresh_from_db() + assert owner.plan == desired_plan_name + assert owner.plan_user_count == original_user_count + assert owner.delinquent == True + + @patch("services.billing.stripe.Subscription.modify") + @patch("services.billing.stripe.Subscription.retrieve") + def test_modify_subscription_payment_no_false_positives( + self, + retrieve_subscription_mock, + subscription_modify_mock, + ): + original_user_count = 10 + original_plan = PlanName.CODECOV_PRO_YEARLY.value + owner = OwnerFactory( + plan=original_plan, + plan_user_count=original_user_count, + stripe_subscription_id="33043sdf", + delinquent=False, + ) + schedule_id = None + current_subscription_start_date = 1639628096 + current_subscription_end_date = 1644107871 + subscription_params = { + "schedule_id": schedule_id, + "start_date": current_subscription_start_date, + "end_date": current_subscription_end_date, + "quantity": original_user_count, + "name": original_plan, + "id": 105, + } + retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + + subscription_response = { + "id": 105, + "object": "subscription", + "application_fee_percent": None, + "pending_update": {}, + } + subscription_modify_mock.return_value = MockFailedSubscriptionUpgrade( + subscription_response + ) + + desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value + desired_user_count = 20 + desired_plan = {"value": desired_plan_name, "quantity": desired_user_count} + self.stripe.modify_subscription(owner, desired_plan) + + self._assert_subscription_modify( + subscription_modify_mock, owner, subscription_params, desired_plan + ) + + # plan is updated, owner is not delinquent + owner.refresh_from_db() + assert owner.plan == desired_plan_name + assert owner.plan_user_count == desired_user_count + assert owner.delinquent == False + @patch("services.billing.stripe.Subscription.modify") @patch("services.billing.stripe.Subscription.retrieve") def test_modify_subscription_without_schedule_upgrades_plan_and_users_immediately( @@ -444,6 +571,7 @@ def test_modify_subscription_without_schedule_upgrades_plan_and_users_immediatel } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value desired_user_count = 15 @@ -673,6 +801,7 @@ def test_modify_subscription_with_schedule_modifies_schedule_when_user_count_inc } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_MONTHLY.value desired_user_count = 26 @@ -760,6 +889,7 @@ def test_modify_subscription_with_schedule_releases_schedule_when_plan_upgrades( } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value desired_user_count = 15 @@ -806,6 +936,7 @@ def test_modify_subscription_with_schedule_releases_schedule_when_plan_upgrades_ } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_YEARLY.value desired_user_count = 10 @@ -851,6 +982,7 @@ def test_modify_subscription_with_schedule_releases_schedule_when_plan_downgrade } retrieve_subscription_mock.return_value = MockSubscription(subscription_params) + subscription_modify_mock.return_value = MockSubscription(subscription_params) desired_plan_name = PlanName.CODECOV_PRO_MONTHLY.value desired_user_count = 20