Fix stripe subscription upgrade failures #831

nora-codecov · 2024-09-19T23:11:46Z

Purpose/Motivation

We were eagerly updating the user's plan in the specific case where they already have a subscription and they are upgrading. We were updating without checking that the payment was successful, so in the case where we have updated the plan but the stripe payment fails, we were bumping the user all the way back to free tier.

Links to relevant tickets

codecov/engineering-team#2407
stripe's docs: https://docs.stripe.com/billing/subscriptions/pending-updates

What does this PR do?

Only update the plan if the payment was successful. If it's not successful, reject the changes and give they user the delinquent pop-up. The user in our db and in Stripe's system stays on the original plan until they payment is successful.

codecov-notifications · 2024-09-19T23:21:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

codecov · 2024-09-19T23:22:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.23%. Comparing base (6d7be25) to head (536f112).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff             @@
##               main       #831   +/-   ##
===========================================
  Coverage   96.23000   96.23000           
===========================================
  Files           812        812           
  Lines         18575      18587   +12     
===========================================
+ Hits          17876      17888   +12     
  Misses          699        699

Flag	Coverage Δ
unit	`92.47% <100.00%> (+<0.01%)`	⬆️
unit-latest-uploader	`92.47% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ajay-sentry · 2024-09-20T16:44:04Z

billing/views.py

        # Only update if there isn't a scheduled subscription
-        if not subscription_schedule_id:
+        if not subscription.schedule:


love this ngl

ajay-sentry · 2024-09-20T16:45:12Z

services/billing.py

+                    extra=dict(pending_update=indication_of_payment_failure),
+                )
+            else:
+                # payment successful


will delinquent ever be true at this point where we'd want to set to false?

love this suggestion, but "yes and"ed it with this change https://github.com/codecov/codecov-api/pull/831/files#diff-dda355f2f33801fe525e8ba51493017a8957e35710e9dc1d4affed3940e18685R56 (clearing delinquency in update_plan!)
From what I could tell, we are only using plan_service.update_plan in a situation where the owner should not be delinquent, so just make sure that field is set correctly while we're updating the owner obj.

ajay-sentry

looks good! Could we add the stripe docs you were looking at to the review description?

…pgrading

codecov-qa · 2024-09-23T21:59:14Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2297	1	2296	6

View the top 1 failed tests by shortest run time

billing.tests.test_views.StripeWebhookHandlerTests test_customer_subscription_deleted_deactivates_all_repos

Stack Traces | 0.025s run time

self = &lt;billing.tests.test_views.StripeWebhookHandlerTests testMethod=test_customer_subscription_deleted_deactivates_all_repos&gt;

    def test_customer_subscription_deleted_deactivates_all_repos(self):
        RepositoryFactory(author=self.owner, activated=True, active=True)
        RepositoryFactory(author=self.owner, activated=True, active=True)
        RepositoryFactory(author=self.owner, activated=True, active=True)
    
&gt;       assert (
            self.owner.repository_set.filter(activated=True, active=True).count() == 3
        )
E       assert 2 == 3
E        +  where 2 = &lt;bound method QuerySet.count of &lt;RepositoryQuerySet [&lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/lawyer&gt;&gt;, &lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/use&gt;&gt;]&gt;&gt;()
E        +    where &lt;bound method QuerySet.count of &lt;RepositoryQuerySet [&lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/lawyer&gt;&gt;, &lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/use&gt;&gt;]&gt;&gt; = &lt;RepositoryQuerySet [&lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/lawyer&gt;&gt;, &lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/use&gt;&gt;]&gt;.count
E        +      where &lt;RepositoryQuerySet [&lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/lawyer&gt;&gt;, &lt;Repository: Repo&lt;Owner&lt;github/wesley45&gt;/use&gt;&gt;]&gt; = &lt;bound method QuerySet.filter of &lt;django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.&lt;locals&gt;.RelatedManager object at 0x7f3aaa3dc590&gt;&gt;(activated=True, active=True)
E        +        where &lt;bound method QuerySet.filter of &lt;django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.&lt;locals&gt;.RelatedManager object at 0x7f3aaa3dc590&gt;&gt; = &lt;django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.&lt;locals&gt;.RelatedManager object at 0x7f3aaa3dc590&gt;.filter
E        +          where &lt;django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.&lt;locals&gt;.RelatedManager object at 0x7f3aaa3dc590&gt; = &lt;Owner: Owner&lt;github/wesley45&gt;&gt;.repository_set
E        +            where &lt;Owner: Owner&lt;github/wesley45&gt;&gt; = &lt;billing.tests.test_views.StripeWebhookHandlerTests testMethod=test_customer_subscription_deleted_deactivates_all_repos&gt;.owner

billing/tests/test_views.py:151: AssertionError

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

codecov-public-qa · 2024-09-23T21:59:15Z

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 2303 tests with 1 failed, 2296 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: billing.tests.test_views.StripeWebhookHandlerTests
Test name: test_customer_subscription_deleted_deactivates_all_repos

self = <billing.tests.test_views.StripeWebhookHandlerTests testMethod=test_customer_subscription_deleted_deactivates_all_repos>

    def test_customer_subscription_deleted_deactivates_all_repos(self):
        RepositoryFactory(author=self.owner, activated=True, active=True)
        RepositoryFactory(author=self.owner, activated=True, active=True)
        RepositoryFactory(author=self.owner, activated=True, active=True)
    
>       assert (
            self.owner.repository_set.filter(activated=True, active=True).count() == 3
        )
E       assert 2 == 3
E        +  where 2 = <bound method QuerySet.count of <RepositoryQuerySet [<Repository: Repo<Owner<github/wesley45>/lawyer>>, <Repository: Repo<Owner<github/wesley45>/use>>]>>()
E        +    where <bound method QuerySet.count of <RepositoryQuerySet [<Repository: Repo<Owner<github/wesley45>/lawyer>>, <Repository: Repo<Owner<github/wesley45>/use>>]>> = <RepositoryQuerySet [<Repository: Repo<Owner<github/wesley45>/lawyer>>, <Repository: Repo<Owner<github/wesley45>/use>>]>.count
E        +      where <RepositoryQuerySet [<Repository: Repo<Owner<github/wesley45>/lawyer>>, <Repository: Repo<Owner<github/wesley45>/use>>]> = <bound method QuerySet.filter of <django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.<locals>.RelatedManager object at 0x7f3aaa3dc590>>(activated=True, active=True)
E        +        where <bound method QuerySet.filter of <django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.<locals>.RelatedManager object at 0x7f3aaa3dc590>> = <django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.<locals>.RelatedManager object at 0x7f3aaa3dc590>.filter
E        +          where <django.db.models.fields.related_descriptors.create_reverse_many_to_one_manager.<locals>.RelatedManager object at 0x7f3aaa3dc590> = <Owner: Owner<github/wesley45>>.repository_set
E        +            where <Owner: Owner<github/wesley45>> = <billing.tests.test_views.StripeWebhookHandlerTests testMethod=test_customer_subscription_deleted_deactivates_all_repos>.owner

billing/tests/test_views.py:151: AssertionError

nora-codecov requested review from ajay-sentry and a team September 19, 2024 23:11

ajay-sentry reviewed Sep 20, 2024

View reviewed changes

ajay-sentry approved these changes Sep 20, 2024

View reviewed changes

nora-codecov force-pushed the nora/2407 branch from 081b72c to a6fdc8f Compare September 20, 2024 21:06

nora-codecov added 2 commits September 23, 2024 14:49

add payment_behavior to stripe subscription modification calls when u…

e079577

…pgrading

update delinquency of owner when updating plan

536f112

nora-codecov force-pushed the nora/2407 branch from a6fdc8f to 536f112 Compare September 23, 2024 21:49

nora-codecov enabled auto-merge September 23, 2024 21:49

nora-codecov added this pull request to the merge queue Sep 23, 2024

Merged via the queue into main with commit b70830f Sep 23, 2024
19 of 20 checks passed

nora-codecov deleted the nora/2407 branch September 23, 2024 22:40

suejung-sentry pushed a commit that referenced this pull request Sep 30, 2024

Fix stripe subscription upgrade failures (#831)

85740ee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix stripe subscription upgrade failures #831

Fix stripe subscription upgrade failures #831

nora-codecov commented Sep 19, 2024 •

edited

Loading

codecov-notifications bot commented Sep 19, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading

ajay-sentry Sep 20, 2024

ajay-sentry Sep 20, 2024

nora-codecov Sep 23, 2024

ajay-sentry left a comment

codecov-qa bot commented Sep 23, 2024

codecov-public-qa bot commented Sep 23, 2024

pytest

Fix stripe subscription upgrade failures #831

Fix stripe subscription upgrade failures #831

Conversation

nora-codecov commented Sep 19, 2024 • edited Loading

Purpose/Motivation

Links to relevant tickets

What does this PR do?

codecov-notifications bot commented Sep 19, 2024 • edited Loading

Codecov Report

codecov bot commented Sep 19, 2024 • edited Loading

Codecov Report

ajay-sentry Sep 20, 2024

Choose a reason for hiding this comment

ajay-sentry Sep 20, 2024

Choose a reason for hiding this comment

nora-codecov Sep 23, 2024

Choose a reason for hiding this comment

ajay-sentry left a comment

Choose a reason for hiding this comment

codecov-qa bot commented Sep 23, 2024

❌ 1 Tests Failed:

codecov-public-qa bot commented Sep 23, 2024

❌ Failed Test Results:

pytest

nora-codecov commented Sep 19, 2024 •

edited

Loading

codecov-notifications bot commented Sep 19, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading