refactor Stripe webhook #200

vincanger · 2024-06-28T14:31:23Z

Fixes issue #199

Martinsos

Left comments, nice improvements!
We will also want to add some info to the docs regarding the Stripe API version, right?

template/app/src/server/stripe/stripeClient.ts

template/app/src/server/webhooks/stripe.ts

template/app/src/shared/constants.ts

template/app/src/server/webhooks/stripe.ts

template/app/src/server/webhooks/stripeUserPaymentDetails.ts

template/app/src/server/webhooks/stripe.ts

Co-authored-by: Martin Šošić <[email protected]>

…-lang/open-saas into refactor-subscription-logic

template/app/src/client/app/AccountPage.tsx

template/app/src/client/app/PricingPage.tsx

template/app/src/server/webhooks/stripeEventHandlers.ts

sodic

Answered some of the questions/dilemmas I was tagged on.

I might look at more stuff later :)

template/app/src/server/webhooks/stripeEventHandlers.ts

sodic · 2024-07-04T09:41:14Z

template/app/src/server/webhooks/stripeEventHandlers.ts

+import { HttpError } from 'wasp/server';
+import { emailSender } from 'wasp/server/email';
+
+const getCustomerIdStringOrThrow = (userStripeId: unknown): string => {


Arrow functions

This is the original explanation: wasp-lang/wasp#487

But I wrote it back when we were using JavaScript (there's an important caveat for TypeScript, also explained in Effective TypeScript, Item 12).

Anyway, I am no longer sure what to advocate. I like the "important stuff on top benefit", but perhaps I'd put more weight on consistency- You'll definitely have to use arrows a lot (that's how our hooks and operations work), so you might as well use them all the time.

unknown and validation

We should do validation as early as possible (i.e., when we receive the request). As soon as we ensure something has the type we expect it to have, we should stop validating the data and just rely on TypeScript.

The rule is: Make sure something is of type T before you say it's of type T. In other words, all validation should happen before the assertion, not after it.

Therefore, this function shouldn't be doing the validation. The validation should be done before the event.data.object as Stripe.Checkout.Session expression:

If receiving malformed data is considered normal (i.e., it's the user's direct input, Stripe can return the data in multiple formats, some customers don't have IDs...), we shouldn't raise an exception. We should instead parse the data and propagate the appropriate error to the user (using normal control flow).

If receiving malformed isn't considered normal (i.e., Stripe accidentally broke its API), we should assert the data has the proper format and throw an exception otherwise. I assume this is the case here. To do such an assertion, you can either use a library (e.g., Zod), or write it yourself. But since already using Zod, I recommend sticking with that.

Let me know if you get stuck, I'm happy to help out!

sodic · 2024-07-04T10:14:15Z

template/app/src/server/webhooks/userStripePaymentDetails.ts

+type UserStripePaymentDetails = {
+  userStripeId: string;
+  subscriptionPlan?: SubscriptionPlanId
+  subscriptionStatus?: SubscriptionStatusOptions;
+  numOfCreditsPurchased?: number;
+  datePaid?: Date;
+};


I don't know the domain that well, but something about this type seems like it's another candidate for what Martin talked about here: https://github.com/wasp-lang/open-saas/pull/200/files#r1664746512

I could be wrong though.

sodic · 2024-07-04T10:19:43Z

template/app/src/server/webhooks/userStripePaymentDetails.ts

+export const updateUserStripePaymentDetails = async (args: UserStripePaymentDetails, userDelegate: PrismaUserDelegate ) => {
+  let data: any = {};
+  if (args.datePaid) data.datePaid = args.datePaid;
+  if (args.subscriptionPlan) data.subscriptionPlan = args.subscriptionPlan;
+  if (args.subscriptionStatus) data.subscriptionStatus = args.subscriptionStatus;
+  if (args.numOfCreditsPurchased) {
+    data.credits = { increment: args.numOfCreditsPurchased };
+  }
+
+  return await userDelegate.update({
+    where: {
+      stripeId: args.userStripeId,
+    },
+    data
+  });
+};


To fix this, check Effective TypeScript (2nd edition), item 21.

Also, we should find a better name for data and type it properly (without using any).

I'll help you out here since it's a little weird and the book doesn't do it optimally (I'll assume the type of data is Data):

const data: Data = { ...args.datePaid && { datePaid: args.datePaid }, ...args.subscriptionPlan && { subscriptionPlan: args.subscriptionPlan }, ...subscriptionStatus && { subscriptionStatus: args.subscriptionStatus }, ...args.numberOfCreditsPurchased && { credits: { increment: args.numberOfCreditsPurchased } }, }

@sodic I was considering this, but was wondering if it indeed is a better solution. Maybe I have to read that item 21 which I don't ahve time for now, but I gave up on this different approach as it didn't seem to improve anything, and some people might say it is more convoluted even (I hit such resistance in the past in our PRs when I tried to do this).

Yes, I'm fairly confident that you'll agree with this approach once you read Item 21, so I suggest we follow it.

The benefits will become more apparent when data gets a proper type.

template/app/src/shared/types.ts

template/app/src/shared/constants.ts

sodic

I left some comments and links to material that should make them clearer, but you might have trouble figuring out some of my suggestions.

If that happens, call me up and I'll elaborate (or we can even jump on a call).

sodic · 2024-07-04T15:55:25Z

@vincanger Pro tip:

When you want to link a pull request to an issue, use GitHub's special words.

This way, once you merge the PR, the issue automatically gets closed.

In this case, you can edit your description to say "Closes #199" or "Fixes #199" instead of "addresses issue #199".

You'll see how GitHub immediately underlines it and add the PR under the issue's title

template/app/src/client/app/AccountPage.tsx

template/app/src/server/actions.ts

template/app/src/server/webhooks/stripe.ts

template/app/src/client/app/PricingPage.tsx

template/app/src/server/webhooks/stripeEventHandlers.ts

template/app/src/shared/constants.ts

Martinsos · 2024-07-05T00:33:24Z

@sodic @vincanger I wanted to try out myself some things I was suggesting (mostly regarding consolidating types), so that I don't suggest stuff that is not doable or won't look good once it is done, and I got pulled in and ended up implementing stuff.

First I ended up with this PR: #217, and then I, on top of that one, made a bit different attempt with this one: #218 . I am not sure which one I prefer, maybe the first one, but it is tight: both are a bit clunky in some aspects, but also both work quite well I think.

Please take a look and see if there is anything useful you can use.

Also, since I will be traveling in the following days, and @sodic already got quite involved, I suggest @sodic that you take over the reviewing of this PR and drive it to the end with @vincanger . I think I covered all I could from my side, and you will be able to contribute some extra knowledge on the TS side.

…or-subscription-logic

vincanger

Thanks for all your comments and help here @sodic @Martinsos -- this was a chunky boy!

There was a lot of new insight here for me, so it took me a while to get a grasp of everything.

I merged Martin's ideas from PR #217 for consolidating types. I think we're close to being done here.

I also added docs on stripe api/client versioning. The only thing left to do is create the new app_diff but I think I will do this in a new PR at another time because of there have been so many big changes and I don't want to rush it.

template/app/src/client/app/PricingPage.tsx

template/app/src/server/webhooks/stripeEventHandlers.ts

template/app/src/client/app/AccountPage.tsx

template/app/src/server/actions.ts

template/app/src/client/app/PricingPage.tsx

template/app/src/server/webhooks/stripeEventHandlers.ts

template/app/src/shared/constants.ts

Martinsos

@vincanger left some extra comments.
Pls go through the "conversation" tab in Github and resolve all the comments that are still open but can be resolved!

opensaas-sh/blog/src/content/docs/guides/deploying.md

template/app/src/server/webhooks/stripe.ts

template/app/src/server/webhooks/stripePaymentDetails.ts

Martinsos

Nice, we got it all in a good shape!

vincanger · 2024-07-10T11:08:44Z

Nice, we got it all in a good shape!

updated docs and app_diff as well

vincanger added 2 commits June 28, 2024 16:26

rename TierIds to PaymentPlanIds

7c06409

refactor webhook and util functions

5b736ab

vincanger changed the title ~~rename TierIds to PaymentPlanIds~~ refactor Stripe webhook Jul 2, 2024

pass userDelegate to function

41f8884

vincanger marked this pull request as ready for review July 2, 2024 14:40

vincanger added 4 commits July 2, 2024 17:20

Merge branch 'main' into refactor-subscription-logic

cf079ac

Update dbSeeds.ts

2ed6d6b

Merge branch 'main' into refactor-subscription-logic

1f9c766

update app diff

f8c65b3

Martinsos requested changes Jul 2, 2024

View reviewed changes

vincanger and others added 3 commits July 3, 2024 15:46

Update template/app/src/server/stripe/stripeClient.ts

b63924b

Co-authored-by: Martin Šošić <[email protected]>

extract event handlers and more

eed60a3

Merge branch 'refactor-subscription-logic' of https://github.com/wasp…

3c17bdb

…-lang/open-saas into refactor-subscription-logic

Martinsos reviewed Jul 3, 2024

View reviewed changes

sodic reviewed Jul 4, 2024

View reviewed changes

template/app/src/shared/types.ts Outdated Show resolved Hide resolved

sodic reviewed Jul 4, 2024

View reviewed changes

template/app/src/shared/constants.ts Outdated Show resolved Hide resolved

sodic reviewed Jul 4, 2024

View reviewed changes

template/app/src/shared/constants.ts Outdated Show resolved Hide resolved

Update AccountPage.tsx

4a5a69a

sodic reviewed Jul 4, 2024

View reviewed changes

address filips pro effective typescripting and stuff

77f0517

Martinsos reviewed Jul 4, 2024

View reviewed changes

template/app/src/server/webhooks/stripeEventHandlers.ts Outdated Show resolved Hide resolved

template/app/src/shared/constants.ts Outdated Show resolved Hide resolved

Martinsos added 2 commits July 5, 2024 01:34

Martin's attempt at consolidating types.

373cb5e

fix

de108da

fix webhook events and validation

b013f21

vincanger added 6 commits July 5, 2024 13:30

small changes

1b8dae1

put stripe event handlers back for marty merge

3d40416

Merge branch 'refactor-subscription-logic-martin-attempt' into refact…

cbc9d66

…or-subscription-logic

merge consilidated types from martin

73089dc

move some types around

744c7db

add docs for stripe api version

62e918b

vincanger commented Jul 6, 2024

View reviewed changes

vincanger added 5 commits July 8, 2024 12:26

Update AccountPage.tsx

71242c0

Update stripe.ts

d88ad0f

update SubscriptionStatus type

d89a35f

Update actions.ts

d62d186

add assertUnreachable util

6829173

Martinsos reviewed Jul 8, 2024

View reviewed changes

infomiho approved these changes Jul 9, 2024

View reviewed changes

vincanger added 2 commits July 9, 2024 20:08

more small changes

4af43d4

Update deploying.md

d5ae7c4

Martinsos approved these changes Jul 10, 2024

View reviewed changes

vincanger added 2 commits July 10, 2024 12:28

update accountPage and docs

3f9836f

update app_diff

b2174db

vincanger merged commit 78a9189 into main Jul 10, 2024
1 check passed

vincanger deleted the refactor-subscription-logic branch July 10, 2024 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor Stripe webhook #200

refactor Stripe webhook #200

vincanger commented Jun 28, 2024 •

edited

Loading

Martinsos left a comment

sodic left a comment

sodic Jul 4, 2024 •

edited

Loading

sodic Jul 4, 2024 •

edited

Loading

sodic Jul 4, 2024 •

edited

Loading

Martinsos Jul 4, 2024

sodic Jul 4, 2024

sodic left a comment

sodic commented Jul 4, 2024

Martinsos commented Jul 5, 2024

vincanger left a comment

Martinsos left a comment

Martinsos left a comment

vincanger commented Jul 10, 2024

refactor Stripe webhook #200

refactor Stripe webhook #200

Conversation

vincanger commented Jun 28, 2024 • edited Loading

Martinsos left a comment

Choose a reason for hiding this comment

sodic left a comment

Choose a reason for hiding this comment

sodic Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

Arrow functions

unknown and validation

sodic Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

sodic Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

Martinsos Jul 4, 2024

Choose a reason for hiding this comment

sodic Jul 4, 2024

Choose a reason for hiding this comment

sodic left a comment

Choose a reason for hiding this comment

sodic commented Jul 4, 2024

Martinsos commented Jul 5, 2024

vincanger left a comment

Choose a reason for hiding this comment

Martinsos left a comment

Choose a reason for hiding this comment

Martinsos left a comment

Choose a reason for hiding this comment

vincanger commented Jul 10, 2024

vincanger commented Jun 28, 2024 •

edited

Loading

sodic Jul 4, 2024 •

edited

Loading

`unknown` and validation

sodic Jul 4, 2024 •

edited

Loading

sodic Jul 4, 2024 •

edited

Loading