-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Dry up subscription status #382
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
Conversation
const selectedValue = e.target.value == '' ? null : e.target.value; | ||
|
||
if (selectedValue === 'clear-all') { | ||
setSubcriptionStatusFilter([]); | ||
} else { | ||
setSubcriptionStatusFilter((prevValue) => { | ||
if (prevValue.includes(selectedValue as SubscriptionStatus)) { | ||
return prevValue.filter((val) => val !== selectedValue); | ||
} else { | ||
return [...prevValue, selectedValue as SubscriptionStatus]; | ||
} | ||
}); | ||
} |
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.
Not ideal but best we can do for the time being.
// becomes 'unpaid' and finally 'expired' (i.e. 'deleted'). | ||
// NOTE: ability to pause or trial a subscription is something that has to be additionally configured in the lemon squeezy dashboard. | ||
// becomes 'unpaid' and finally 'expired' (i.e. 'deleted'). | ||
// NOTE: ability to pause or trial a subscription is something that has to be additionally configured in the lemon squeezy dashboard. | ||
// If you do enable these features, make sure to handle these statuses here. | ||
if (status === 'past_due' || status === 'active') { |
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 didn't change these because they're lemonSqueezy status strings, not ours (although lexically the same at the moment).
Same goes for Stripe.
@@ -106,7 +109,7 @@ async function handleSubscriptionCreated(data: Subscription, userId: string, pri | |||
lemonSqueezyId, | |||
userId, | |||
subscriptionPlan: planId, | |||
subscriptionStatus: status, | |||
subscriptionStatus: status as SubscriptionStatus, |
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 some of these assertions won't be necessary once we merge #377
@@ -1,6 +1,11 @@ | |||
import { requireNodeEnvVar } from '../server/utils'; | |||
|
|||
export type SubscriptionStatus = 'past_due' | 'cancel_at_period_end' | 'active' | 'deleted'; | |||
export enum SubscriptionStatus { |
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.
We attacked the same thing: https://github.com/wasp-lang/open-saas/pull/379/files#diff-4840b1aee17262f7d02b6e108dc85da00de2b5a91f5e92eb83b862aed83a6fadR4 with different tools :D
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 used the enum for the definition, and referenced the enum in Zod during validation using z.nativeEnum
.
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.
We gonna have some merge conflicts, but I think it's better for me to merge the validations after you merge this because my validations are based on existing code which you improve in this PR.
Subscription statuses were repeated all over the app.
They're now defined in a single place and imported where necessary.