Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stripe API upgrade #8405

Merged
merged 4 commits into from
Nov 19, 2024
Merged

Stripe API upgrade #8405

merged 4 commits into from
Nov 19, 2024

Conversation

gbp
Copy link
Member

@gbp gbp commented Oct 9, 2024

What does this do?

Upgrades the Stripe API version used.

Why was this needed?

To hopefully help in migrating from Stripe plans to prices which in turn will help us add new prices and change existing subscriptions.

@gbp
Copy link
Member Author

gbp commented Oct 9, 2024

https://docs.stripe.com/changelog/2019-03-14/renames-date-field-invoices-created

There are a few changes to the invoice object:

  • The date property has been renamed to created.

Need to change

<strong><%= invoice.date.to_fs(:long) %></strong>
and

Should also add test examples to catch this.

@gbp
Copy link
Member Author

gbp commented Oct 9, 2024

https://docs.stripe.com/changelog/2018-02-05/plans-link-individual-products-several-fields-moved

Each plan object is now linked to a product object .... The plan object ... and name attributes have been moved to product objects, and plan objects now have a nickname attribute.

Need to change

and

Should also add test examples to catch this.

@gbp
Copy link
Member Author

gbp commented Oct 9, 2024

https://docs.stripe.com/changelog/2018-07-27/coupons-use-floats-specify-percent-off

The percent_off field of coupons was changed from Integer to Float, with a precision of two decimal places.

Need to check the calculations in

def coupon_reduction(net)
if coupon.amount_off
BigDecimal((coupon.amount_off * 0.01), 0).round(2)
else
(net * coupon.percent_off / 100)
end
end

@gbp gbp marked this pull request as ready for review October 10, 2024 13:37
@gbp
Copy link
Member Author

gbp commented Oct 10, 2024

The above comments have been addressed. While I can write failing tests for them (stripe-ruby-mock doesn't enforce valid attributes, and it does expand Stripe resources) I have ensured they are all fixed.

@gbp gbp mentioned this pull request Nov 6, 2024
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about a plan.name that's left. There's also one more mention in app/models/webhook.rb that could be updated, but I guess you'd have to call something else to get the product name - the webhook includes the new data as well as the old, I've just checked (e.g. https://dashboard.stripe.com/events/evt_1QA8dQLoAAr9vgdbpgEl1nZm ) so you could switch there, but might not be worth it.

@gbp gbp requested a review from dracos November 7, 2024 10:25
@dracos
Copy link
Member

dracos commented Nov 8, 2024

Sorry, after reviewing the last PR (incoming soon), I have a question - the commit message here says "Bumps the API version to the latest version support by stripe-ruby-mock." - but according to https://github.com/stripe-ruby-mock/stripe-ruby-mock/blob/master/CHANGELOG.md it supports up to SDK v11 which is the most recent Stripe SDK? Just checking I hadn't missed something there. Think the only major change since the one you're upgrading to here is the change from tax_percent to tax rate IDs.

@gbp
Copy link
Member Author

gbp commented Nov 8, 2024

In the stripe-ruby-mock readme they specify their target API version.

I tried later versions of the Stripe API version but unfortunately it resulted in so many broken specs.

The change to allow SDK v11 is just a change to relax the version requirement on the main Stripe ruby client gem. It does nothing to actually support those changes.

Use Stripe mocks instead of doubles. Ideally this would mean if the
attributes change upstream we get test failures, unfortunately this
doesn't appear to happen due to how stripe-ruby-mock works.
Bumps the API version to the latest version support by stripe-ruby-mock.

Also remove gem lock allowing us to upgrade both stripe-ruby-mock and
the main stripe gem.

Fixes broken tests and other changes which aren't testable, such as:
1. Invoice#date being rename Invoice#created
2. Plan#name being moved to Plan#product & Product#name and not loaded/
   expanded automatically.
These have been replaced with calls to `create` or `update`.
@gbp gbp merged commit bcc197d into develop Nov 19, 2024
6 checks passed
gbp added a commit to mysociety/whatdotheyknow-theme that referenced this pull request Nov 22, 2024
Since Stripe API version upgrade [1] in core this hasn't worked as
expected.

This is because of changes to ProAccount#update_stripe_customer required
to support the newer API and changes to prevent deprecation warnings.

[1] mysociety/alaveteli#8405
gbp added a commit to mysociety/whatdotheyknow-theme that referenced this pull request Nov 22, 2024
Since Stripe API version upgrade [1] in core this hasn't worked as
expected.

This is because of changes to ProAccount#update_stripe_customer required
to support the newer API and changes to prevent deprecation warnings.

[1] mysociety/alaveteli#8405
gbp added a commit to mysociety/whatdotheyknow-theme that referenced this pull request Nov 22, 2024
Since Stripe API version upgrade [1] in core this hasn't worked as
expected.

This is because of changes to ProAccount#update_stripe_customer required
to support the newer API and changes to prevent deprecation warnings.

[1] mysociety/alaveteli#8405
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants