-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[Stripe] Enable automatic tax on transactions #13002
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
Conversation
/werft run 👍 started the job as gitpod-build-jx-stripe-tax.1 |
/werft run 👍 started the job as gitpod-build-jx-stripe-tax.2 |
96153ca
to
6d1ad9f
Compare
Depends on #13008 |
1c0d27b
to
80b3c87
Compare
This comment was marked as outdated.
This comment was marked as outdated.
80b3c87
to
3eacb5e
Compare
9c26407
to
98b5f19
Compare
62b578c
to
fdb0663
Compare
e7eeca9
to
ee04da1
Compare
invoice_settings: { default_payment_method: setupIntent.payment_method }, | ||
...(paymentMethod.billing_details.address?.country | ||
? { address: { line1: "", country: paymentMethod.billing_details.address?.country } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why line1
is mandatory? Why don't you copy over the full address here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line1
isn't actually mandatory: https://stripe.com/docs/api/customers/create#create_customer-address-line1
However, it seems that Stripe's TypeScript bindings marked it as mandatory, so I provide an empty value (undefined
or null
were not allowed, but I believe the result is the same).
Also, the country by itself is already the full address we get from the upgrade flow (we only ask for country, not for a full billing address, so new subscribers didn't have a change to fill this in yet):
However, I'm not exactly sure what happens if:
- You subscribe from a given country
- Then, you set a complete billing address (e.g. all lines) in your Stripe customer portal
- Then, you cancel
- Then, you subscribe from a different country
It's sort of an edge case, but should also be tested. Thanks for pointing it out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
527bfda
to
8c5d1df
Compare
8c5d1df
to
30bc293
Compare
30bc293
to
361616f
Compare
…ly outdated) objects
…rs in supported regions
…m} to createStripeCustomer
…cription to only use attributionIds
Weird 🤔
Maybe let's rebase & build with-clean-slate just in case. |
started the job as gitpod-build-jx-stripe-tax.27 because the annotations in the pull request description changed |
361616f
to
244e43b
Compare
Let's try this again: /werft run 👍 started the job as gitpod-build-jx-stripe-tax.30 |
started the job as gitpod-build-jx-stripe-tax.29 because the annotations in the pull request description changed |
244e43b
to
ad8d0ac
Compare
Description
Enable automatic tax on Stripe transactions:
attributionId
more and pass around IDs instead of (possibly outdated) Stripe objectsautomatic_tax: { enabled: true }
to enable automatic tax collection for these subscriptionsRelated Issue(s)
Fixes #13000
How to test
Release Notes
Documentation
Werft options: