-
Notifications
You must be signed in to change notification settings - Fork 0
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
Stripe template TS migration #122
base: main
Are you sure you want to change the base?
Conversation
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.
🎩 works well!
A couple of things that should be looked at, in addition to comments:
- I see log statements in the browser console in
web/routes/index.tsx
- access control issue on index route in frontend when not signed in (ie. fresh app, first experience)
- yearly switch does not work when products set up with monthly pricing in Stripe
https://github.com/user-attachments/assets/31762d08-6f43-409b-8aca-3a447ffb632d - the
api/routes/GET-hello
route is present
@@ -0,0 +1,103 @@ | |||
import { stripe } from "../stripe"; | |||
|
|||
type StripeProduct = { |
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.
I think we can get types from Stripe
https://github.com/stripe/stripe-node#usage-with-typescript
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.
I tried using this before but it caused more issues
I don't see the GET-hello route in my app |
Ignore! That was bad copy-paste on my part when testing! |
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.
Two tophat issues:
- Still get an unauthenticated warning in dev harness when I visit the index route in the frontend. We have signed in components to check for this status and I think we may need to reconsider how the Provider is positioned
- We don't properly handle subscription cancellations. When I cancel in Stripe's dashboard, we delete the record and I still have access to my "app". We only check for the stripeCustomerId and not the status of the subscription
request: request, | ||
endpointSecret: process.env.STRIPE_SUBSCRIPTION_WEBHOOK_SECRET, | ||
endpointSecret: String(process.env.STRIPE_SUBSCRIPTION_WEBHOOK_SECRET), | ||
}); | ||
} catch (err) { | ||
return await reply.status(400).send(); |
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.
When I cancel a stripe subscription we delete the subscription. Instead, we should keep the record and update the status field, then check to make sure the user has an active subscription
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.
I've changed the Stripe subscription logic to immediately end the subscription so the delete webhook will be fired
I might have forgotten to look into the permissions error issue. Also not sure how you're getting that when you should only be calling getProducts from the /billing page. I'll remove the ability to see the homepage when there's no sub |
No description provided.