dev: Upgrade stripe version to latest and add typings #569

ajay-sentry · 2024-05-16T19:06:22Z

Purpose/Motivation

This initiative stemmed from upgrading python to version 3.12, where in the process we bumped up our stripe version from 2.55.2 -> 8.9. This represented ~6.5 years of stripe versions and associated model updates in the process. Change log from stripe's website can be found here.

After the initial merge of that PR, @adrian-codecov and I noticed that there were some issues coming in from sentry; which we thought to have fixed in #563. However, this didn't quell the issues; and in fact surfaced other issues in sentry where certain webhooks around subscription schedules were no longer being processed. Upon doing some additional investigation in the stripe dashboard, we realized that Stripe itself was using the 2017 version of their API when generating webhooks for us to process.

This meant then that we now had a mismatch between our local version of stripe; the local implementation of stripe, and the dashboard version of stripe, each returning or expecting slightly different objects between them. This PR then hoped to tackle the differing versions once and for all; putting us on the latest version of stripe for API, with version updates for Gazebo and Worker soon to follow. Because API is where the "meat" of our stripe integration is, we believe that even with this single PR and using stripe's "native" dashboard API version update tool we should be good. But for sanity, we will be merging codecov/gazebo#2884 and codecov/worker#456 as well.

Things that were broken at some point or another:

Invoice payment succeeded
Updating a customer subscription
Creating a subscription schedule
Updating a subscription schedule
Deleting a subscription

Links to relevant tickets

Related ticket for tracking codecov/engineering-team#1754

What does this PR do?

Fixes:

Checkout Session input params to match latest stripe object
Subscription schedule object keys to pull off plan name correctly
Stripe errors are now on the stripe object itself from package

Updates:

Removes unnecessary segment stuff in checkout_session_completed
Removes interpolated logs in favor of dictionary params
Adds types for all webhook functions
Adds a new set of keys in the settings mapping stripe values to plan names (TBD if we should remove the original mapping)
Uses stripe version 9.6 (latest) from 8.9 -- This will match the dashboard version once we upgrade

Notes to Reviewer

I have been working heavily with @adrian-codecov to test this locally throughout. We feel confident in this PR having flipped the dashboard version to latest and doing quick tests and then flipping back. We will be monitoring this post merge to confirm successful webhooks in the console before we breathe a final sigh of relief.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

codecov · 2024-05-16T19:15:41Z

Codecov Report

Attention: Patch coverage is 96.36364% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 95.84%. Comparing base (d5ebabe) to head (ca9c30d).

✅ All tests successful. No failed tests found.

Files	Patch %	Lines
codecov/settings_prod.py	0.00%	1 Missing ⚠️
codecov/settings_staging.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #569     +/-   ##
=======================================
+ Coverage   95.83   95.84   +0.01     
=======================================
  Files        777     777             
  Lines      17286   17292      +6     
=======================================
+ Hits       16565   16573      +8     
+ Misses       721     719      -2

Flag	Coverage Δ
unit	`91.36% <96.36%> (+0.01%)`	⬆️
unit-latest-uploader	`91.36% <96.36%> (+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.

codecov-staging · 2024-05-16T19:22:41Z

Codecov Report

Attention: Patch coverage is 96.36364% with 2 lines in your changes are missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files	Patch %	Lines
codecov/settings_prod.py	0.00%	1 Missing ⚠️
codecov/settings_staging.py	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

codecov-qa · 2024-05-16T19:22:47Z

Codecov Report

Attention: Patch coverage is 96.36364% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 91.36%. Comparing base (d5ebabe) to head (ca9c30d).

✅ All tests successful. No failed tests found.

Files	Patch %	Lines
codecov/settings_prod.py	0.00%	1 Missing ⚠️
codecov/settings_staging.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #569      +/-   ##
==========================================
+ Coverage   91.34%   91.36%   +0.01%     
==========================================
  Files         599      599              
  Lines       15968    15974       +6     
==========================================
+ Hits        14586    14594       +8     
+ Misses       1382     1380       -2

Flag	Coverage Δ
unit	`91.36% <96.36%> (+0.01%)`	⬆️
unit-latest-uploader	`91.36% <96.36%> (+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.

codecov-public-qa · 2024-05-16T19:22:55Z

Codecov Report

Attention: Patch coverage is 96.36364% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 91.36%. Comparing base (d5ebabe) to head (ca9c30d).

✅ All tests successful. No failed tests found ☺️

@@            Coverage Diff             @@
##             main     #569      +/-   ##
==========================================
+ Coverage   91.34%   91.36%   +0.01%     
==========================================
  Files         599      599              
  Lines       15968    15974       +6     
==========================================
+ Hits        14586    14594       +8     
+ Misses       1382     1380       -2

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

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

Files	Coverage Δ
billing/views.py	`96.94% <100.00%> (+2.33%)`	⬆️
codecov/settings_dev.py	`100.00% <100.00%> (ø)`
plan/service.py	`97.05% <100.00%> (+0.79%)`	⬆️
services/billing.py	`92.48% <100.00%> (ø)`
services/decorators.py	`100.00% <100.00%> (ø)`
codecov/settings_prod.py	`0.00% <0.00%> (ø)`
codecov/settings_staging.py	`0.00% <0.00%> (ø)`

adrian-codecov

Left some comments but otherwise looks good!

adrian-codecov · 2024-05-17T16:48:25Z

billing/tests/test_views.py

@@ -18,7 +18,7 @@

 class MockSubscriptionPlan(object):
    def __init__(self, params):
-        self.name = params["new_plan"]
+        self.id = params["new_plan"] or "price_1OCM2cGlVGuVgOrkMWUFjPFz"


Do we need this price?

Nope we don't! Removed in ca9c30d

adrian-codecov · 2024-05-17T16:49:04Z

billing/tests/test_views.py

@@ -29,7 +29,7 @@ def __init__(self, owner):

 class MockSubscription(object):
    def __init__(self, owner, params):
-        self.metadata = MockOboOrg(owner)
+        self.metadata = {"obo_organization": owner.ownerid, "obo": 15}


Mmm, I'm curious what made this change, afaik the metadata object didn't change on our end. Can we confirm we're setting the metadata correctly in stripe? This would be under the customer object

Yep, we are! Here are a few screenshots

As a quick followup, does subscription.metadata.obo_organization work over subscription.metadata["obo_organization"]? Trying to see if we need to make this + test changes related to this in the first place, or if this was when we were unsure about the dot/bracket notation

adrian-codecov · 2024-05-17T16:50:01Z

billing/tests/test_views.py

@@ -301,7 +301,7 @@ def test_customer_subscription_updated_does_not_change_subscription_if_not_paid_
                    "object": {
                        "id": self.owner.stripe_subscription_id,
                        "customer": self.owner.stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": "users-free"},
+                        "plan": {"id": "fieown4"},


For consistency, shouldn't this be id: ?

Sure, changed all instances to '?' in ca9c30d

adrian-codecov · 2024-05-17T16:51:23Z

billing/views.py

            extra=dict(
                stripe_customer_id=subscription.customer,
                stripe_subscription_id=subscription.id,
-                ownerid=subscription.metadata.obo_organization,
+                ownerid=subscription.metadata["obo_organization"],


Oh do you think we changed the test because we changed this part? Did we have to change this part in the end after seeing object/class notation worked fine?

We may be able to swap to . notation, but that makes pylance upset; and we'd have to rerun all our local tests. I don't think the cost is worth it tbh.

I'll let you make the call, just trying to not change things we don't have to + keep consistency with the rest of the objects and not wonder in the future why we changed this/why it's different 👍

adrian-codecov · 2024-05-17T16:52:43Z

billing/views.py

@@ -281,7 +311,7 @@ def post(self, request, *args, **kwargs):
                self.request.META.get(StripeHTTPHeaders.SIGNATURE),
                settings.STRIPE_ENDPOINT_SECRET,
            )
-        except stripe.error.SignatureVerificationError as e:
+        except stripe.SignatureVerificationError as e:


Did we never have tests for this?

Nope we didn't haha, added in ca9c30d

adrian-codecov · 2024-05-17T16:53:11Z

plan/service.py

        if name not in USER_PLAN_REPRESENTATIONS:
            raise ValueError("Unsupported plan")
+        if not user_count:
+            raise ValueError("Quantity Needed")


We probably need a test for this

Good point, we didn't have tests for any of that function so added in ca9c30d

adrian-codecov

Post the comments you're gtg

ajay-sentry added 5 commits May 15, 2024 14:42

update prorate param

befb325

Merge branch 'main' into Ajay/delete-sub-updates

e3e2615

test updates

d20673f

push up init typings

f481955

update error types and a few others

912905e

ajay-sentry added 6 commits May 16, 2024 14:41

fix the subscription plan errors

f1e43ed

fix checkout session stuff

b0023a3

save cast

15cd044

fix and turn to dot notation

d39156c

fix tests, add some additional types and update some logs

347555b

Merge branch 'main' into stripe-upgrade-latest

35b0010

adrian-codecov reviewed May 17, 2024

View reviewed changes

adrian-codecov approved these changes May 17, 2024

View reviewed changes

update test stuff

ca9c30d

ajay-sentry added this pull request to the merge queue May 17, 2024

This was referenced May 17, 2024

dev: Upgrade stripe to latest codecov/worker#456

Merged

dev: upgrade stripe API to latest version codecov/gazebo#2884

Merged

Merged via the queue into main with commit 18722b4 May 17, 2024
21 of 22 checks passed

ajay-sentry deleted the stripe-upgrade-latest branch May 17, 2024 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev: Upgrade stripe version to latest and add typings #569

dev: Upgrade stripe version to latest and add typings #569

ajay-sentry commented May 16, 2024 •

edited

Loading

codecov bot commented May 16, 2024 •

edited

Loading

codecov-staging bot commented May 16, 2024 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented May 16, 2024 •

edited

Loading

codecov-public-qa bot commented May 16, 2024 •

edited

Loading

adrian-codecov left a comment

adrian-codecov May 17, 2024

ajay-sentry May 17, 2024

adrian-codecov May 17, 2024

ajay-sentry May 17, 2024

adrian-codecov May 17, 2024

adrian-codecov May 17, 2024

ajay-sentry May 17, 2024

adrian-codecov May 17, 2024

ajay-sentry May 17, 2024

adrian-codecov May 17, 2024

adrian-codecov May 17, 2024

ajay-sentry May 17, 2024

adrian-codecov May 17, 2024

ajay-sentry May 17, 2024

adrian-codecov left a comment

dev: Upgrade stripe version to latest and add typings #569

dev: Upgrade stripe version to latest and add typings #569

Conversation

ajay-sentry commented May 16, 2024 • edited Loading

Purpose/Motivation

Links to relevant tickets

What does this PR do?

Notes to Reviewer

Legal Boilerplate

codecov bot commented May 16, 2024 • edited Loading

Codecov Report

codecov-staging bot commented May 16, 2024 • edited by codecov-notifications bot Loading

Codecov Report

codecov-qa bot commented May 16, 2024 • edited Loading

Codecov Report

codecov-public-qa bot commented May 16, 2024 • edited Loading

Codecov Report

adrian-codecov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrian-codecov left a comment

Choose a reason for hiding this comment

ajay-sentry commented May 16, 2024 •

edited

Loading

codecov bot commented May 16, 2024 •

edited

Loading

codecov-staging bot commented May 16, 2024 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented May 16, 2024 •

edited

Loading

codecov-public-qa bot commented May 16, 2024 •

edited

Loading