feat: update Stripe plan in PlansStorage#set
#320
Conversation
We'd like to let users change their plan directly from console or the CLI, which means we need to teach `PlansStorage#set` how to update Stripe. I've introduced a new `BillingProvider` interface to start abstracting Stripe functionality and especially to make it easier to test systems that interact with Stripe. I have not done a thorough audit of the various ways we interact with Stripe, but I'd like to move toward having all Stripe interactions go through `BillingProvider`. The linter is currently failing because I made the `account` field in the the billing customer schema optional and a fair amount of the billing logic relies on it being not-`undefined`. I could just go in and update logic to deal with undefined `account` but wanted to get some feedback from @alanshaw before I dig into that: I can see arguments for either option (roughly "we might want to track customers who don't have an account set up in Stripe" vs "the point of this table is to track Stripe accounts") but testing will require a bit more work if we want to preserve non-optionality (since the test stores don't get initialized from the test billing provider in the same way - in production they get initialized via the webhook handler, which doesn't currently have an analog in testing code). Either of the options here will require a bit more work, so I'd love to know if anyone has strong feelings before I finish this off.
linter still failing because I'm returning unexpected error types, need to update w3up one more time..
it gets initialized before our system, so should always have a record.
Co-authored-by: Alan Shaw <[email protected]>
also respect no-implicit-coercion
found this in the organization secrets in GitHub, so let's re-use it
This reverts commit d8ad82d.
I also rolled this in Stripe
I was not doing it correctly, which is I think why the integration tests are failing
I updated them in the Stripe test environment to match what we use in production, since we don't support using different plan names across the board.
View stack outputs
|
this should be set using the SST secrets tool
this ensures MAILSLURP_API_KEY will be set in local tests
|
I created this PR since the integration test env for #318 seems to be broken - integration tests run fine here but consistently fail there |
|
Most of these changes were approved here: I'm going to wait for one more review since there have been a few changes, but the bulk of this should be ready to go! |
| export function createStripeBillingProvider(stripe) { | ||
| return { | ||
| async hasCustomer(customer) { | ||
| const customersResponse = await stripe.customers.list({ email: toEmail(/** @type {import('@web3-storage/did-mailto').DidMailto} */(customer)) }) |
There was a problem hiding this comment.
this layer should probably normalize these email addresses before reading/writing.
e.g. lowercase the domain part of the email address before passing to stripe.
https://stackoverflow.com/questions/9807909/are-email-addresses-case-sensitive
(ideally stripe would do this for you but I wouldn't count on it)
There was a problem hiding this comment.
I wonder if we should also normalize the part before the @ for email providers (like gmail, outlook and yahoo) that treat it as case-insensitive: https://www.linkedin.com/pulse/unraveling-mystery-email-addresses-case-sensitive-warmy#:~:text=Gmail's%20Approach%20to%20Case%20Sensitivity.&text=This%20means%20that%20regardless%20of,email%20address%20in%20Gmail's%20system.
for gmail I think this would also mean stripping out . from that part of the email address: https://support.google.com/mail/answer/7436150?hl=en
Co-authored-by: Benjamin Goering <[email protected]>
per review comment, thanks @vascosantos!
We'd like to let users change their plan directly from console or the CLI, which means we need to teach PlansStorage#set how to update Stripe.
I've introduced a new BillingProvider interface to start abstracting Stripe functionality and especially to make it easier to test systems that interact with Stripe. I have not done a thorough audit of the various ways we interact with Stripe, but I'd like to move toward having all Stripe interactions go through BillingProvider.