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.

Sign up for GitHub

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

Jump to bottom

[server][dashboard] Allow new Stripe customers to select their billing currency #12376

Merged
merged 5 commits into from
Sep 1, 2022
Merged

[server][dashboard] Allow new Stripe customers to select their billing currency #12376

merged 5 commits into from
Sep 1, 2022

Conversation

jankeromnes
Copy link
Contributor

Description

Allow new Stripe customers to select their billing currency.

Related Issue(s)

Fixes #12343

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview
  • /werft with-payment

@jankeromnes jankeromnes force-pushed the jx/select-currency branch 2 times, most recently from 01bc480 to 91374b6 Compare August 25, 2022 13:54
@roboquat roboquat added size/L and removed size/M labels Aug 25, 2022
@jankeromnes jankeromnes force-pushed the jx/select-currency branch 8 times, most recently from 2274614 to b8eaa99 Compare August 30, 2022 09:20
@jankeromnes jankeromnes force-pushed the jx/select-currency branch 5 times, most recently from 913cf9f to 0734efb Compare August 31, 2022 09:48
@jankeromnes jankeromnes marked this pull request as ready for review August 31, 2022 14:37
@jankeromnes jankeromnes requested a review from a team August 31, 2022 14:37
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Aug 31, 2022
easyCZ
easyCZ reviewed Aug 31, 2022
View reviewed changes
components/server/ee/src/user/stripe-service.ts
return customer;
}

async setPreferredCurrencyForCustomer(customer: Stripe.Customer, currency: string): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

How do we ensure that currency is a valid value? Does Stripe reject an invalid type of currency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currency can be any string. Then, when we choose a Gitpod Product Price in Stripe, we see if one is available in our configuration for that specific string. If there is, great. If not, we pick the USD Price.

components/server/ee/src/workspace/gitpod-server-impl.ts
if (!customer) {
customer = await this.stripeService.createCustomerForTeam(user, team!);
}
await this.stripeService.setPreferredCurrencyForCustomer(customer, currency);
Copy link
Member

Choose a reason for hiding this comment

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

Would this allow me to change my currency after I've got a subscription in one currency? And if so, is that a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would this allow me to change my currency after I've got a subscription in one currency?

Yes -- you can always cancel your Subscription, then buy a new Subscription in any available currency.

And if so, is that a problem?

No -- this is a feature. If you're paying in USD, but want to change to EUR, you can.

@jankeromnes jankeromnes requested a review from a team August 31, 2022 16:55
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Sep 1, 2022 •
edited
Loading

ERRATUM: @AlexTugarev found that you actually cannot switch currencies without first recreating a new Stripe customer.

If you subscribe with one currency, then cancel, then upgrade with another currency, Stripe will refuse the upgrade with:

Error: You cannot combine currencies on a single customer. This customer has had a subscription, coupon, or invoice item with currency eur

The only way to switch currencies is to contact Gitpod support, so that we delete the Stripe customer (or, maybe better, modify the team:${ID} metadata field such that the customer cannot be found for the team anymore leading to the creation of a new customer).

easyCZ
easyCZ approved these changes Sep 1, 2022
View reviewed changes
Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

LGTM from a backend side. I don't have as much context on the UI but looks good to me.

/hold in case you want someone else to take a look at UI side

@jankeromnes
Copy link
Contributor Author

Many thanks @easyCZ!

@AlexTugarev had a good follow-up suggestion -- for these cases where subscribing fails, we don't properly display errors in the UI. It would be good to show (maybe not the full Stripe errors, but at least some hint that things went wrong and that you may need to contact support).

AlexTugarev
AlexTugarev approved these changes Sep 1, 2022
View reviewed changes
Copy link
Member

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

LGTM!
The flow to set a currency worked nicely 👏🏻

We catched #12376 (comment)
Can be handled as follow-up

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Sep 1, 2022 •
edited by werft-gitpod-dev-com bot
Loading

Many thanks! 🚀 Let's improve in follow-ups.

/unhold

@roboquat roboquat merged commit f45f599 into main Sep 1, 2022
@roboquat roboquat deleted the jx/select-currency branch September 1, 2022 08:46
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Sep 1, 2022
@jankeromnes jankeromnes mentioned this pull request Sep 1, 2022
Closed
@jankeromnes
Copy link
Contributor Author

Filed this follow-up issue: #12580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@easyCZ easyCZ easyCZ approved these changes

@AlexTugarev AlexTugarev AlexTugarev approved these changes

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Signup with Stripe: Allow users to select their preferred currency
4 participants
@jankeromnes @AlexTugarev @easyCZ @roboquat