travis
commented
7 months ago Please can you explain why account needs to become optional?
For sure - tried to explain this in the PR description but it was end-of-day stream of consciousness.
Tests for PlansStorage (which live in w3up) don't currently do any setup, which means there isn't a record in the test Dynamo when we invoke plan/set. Since that doesn't currently require a full record (ie, doesn't error when it doesn't find a record) I was seeing errors in the test (errors trying to decode the account-less record in Dynamo) when it subsequently invoked plan/get.
Taken on its own, this suggests to me that we should rework the test and perhaps make plan/set fail if there isn't already a record in Dynamo, but it also made me wonder if account should be optional - it seems like we might want to track billing for a customer even if they have not yet set up a Stripe account and allow them to connect a Stripe account later.
It's also entirely possible that this case is already supported without making account optional, so I figured I'd pause and check in with you on the intent behind making account non-optional. @alanshaw I would love to huddle this morning to kick this around if you have a few minutes!
We'd like to let users change their plan directly from console or the CLI, which means we need to teach
PlansStorage#sethow to update Stripe.I've introduced a new
BillingProviderinterface 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 throughBillingProvider.The linter is currently failing because I made the
accountfield 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 undefinedaccountbut 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.