feat: add multi subscription support #734
Conversation
|
Important Auto Review SkippedAuto reviews are limited to the following labels: coderabbit. Please add one of these labels to enable auto reviews. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe changes reflect a shift in the application's billing and subscription management, particularly with Stripe integration. Yearly plans and free plans have been removed, and there's a focus on monthly pricing and handling multiple subscriptions per user. The codebase has been updated to accommodate these changes, including updates to environment variables, TypeScript declarations, and database schema adjustments. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
| if (foundSubscription) { | ||
| return getPortalSession({ | ||
| customerId: stripeCustomer.id, | ||
| returnUrl: `${process.env.NEXT_PUBLIC_WEBAPP_URL}/settings/billing`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Just noting we can actually give the user a Stripe session for that exact subscription if that's what we want.
Meaning if they go through the portal for a given price ID (somehow), then they can see the Stripe UI specifically set to that subscription.
| -- Custom migration statement | ||
| DELETE FROM "Subscription" WHERE "planId" IS NULL OR "priceId" IS NULL; |
There was a problem hiding this comment.
Note, this is custom.
Since planId and priceID should not actually be null anymore we need to drop all previous instances where it is null.
| @@ -10,8 +10,6 @@ declare namespace NodeJS { | |||
| NEXT_PRIVATE_ENCRYPTION_KEY: string; | |||
|
|
|||
| NEXT_PUBLIC_STRIPE_COMMUNITY_PLAN_MONTHLY_PRICE_ID: string; | |||
There was a problem hiding this comment.
Note, this remains as it is still required in the marketing website.
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- render.yaml
- turbo.json
Files selected for processing (22)
- .env.example (1 hunks)
- apps/marketing/process-env.d.ts (1 hunks)
- apps/marketing/src/components/(marketing)/pricing-table.tsx (2 hunks)
- apps/web/process-env.d.ts (1 hunks)
- apps/web/src/app/(dashboard)/admin/users/data-table-users.tsx (4 hunks)
- apps/web/src/app/(dashboard)/admin/users/page.tsx (2 hunks)
- apps/web/src/app/(dashboard)/settings/billing/billing-plans.tsx (1 hunks)
- apps/web/src/app/(dashboard)/settings/billing/create-billing-portal.action.ts (1 hunks)
- apps/web/src/app/(dashboard)/settings/billing/create-checkout.action.ts (1 hunks)
- apps/web/src/app/(dashboard)/settings/billing/page.tsx (2 hunks)
- packages/ee/server-only/limits/server.ts (2 hunks)
- packages/ee/server-only/stripe/get-customer.ts (2 hunks)
- packages/ee/server-only/stripe/get-prices-by-interval.ts (3 hunks)
- packages/ee/server-only/stripe/get-prices-by-type.ts (1 hunks)
- packages/ee/server-only/stripe/webhook/on-subscription-deleted.ts (2 hunks)
- packages/ee/server-only/stripe/webhook/on-subscription-updated.ts (2 hunks)
- packages/lib/server-only/admin/get-users-stats.ts (1 hunks)
- packages/lib/server-only/subscription/get-subscriptions-by-user-id.ts (1 hunks)
- packages/lib/server-only/user/create-user.ts (4 hunks)
- packages/prisma/migrations/20231206073509_add_multple_subscriptions/migration.sql (1 hunks)
- packages/prisma/schema.prisma (3 hunks)
- packages/tsconfig/process-env.d.ts (1 hunks)
Additional comments: 56
.env.example (1)
- 77-86: The
.env.examplefile now includes theNEXT_PUBLIC_STRIPE_COMMUNITY_PLAN_MONTHLY_PRICE_IDvariable, which is not mentioned in the summary. Please ensure that the summary reflects all changes made to the file for accurate documentation.apps/marketing/process-env.d.ts (1)
- 6-11: The changes in the environment variables reflect the PR's objective to support multiple subscriptions per user and the shift away from single plan IDs. Ensure that the removal of
NEXT_PUBLIC_STRIPE_COMMUNITY_PLAN_YEARLY_PRICE_IDandNEXT_PUBLIC_STRIPE_FREE_PLAN_IDis reflected in the application logic where these variables were previously used.
Verification result:
The removal of
NEXT_PUBLIC_STRIPE_COMMUNITY_PLAN_YEARLY_PRICE_IDandNEXT_PUBLIC_STRIPE_FREE_PLAN_IDfrom the environment variables is consistent with the PR's objectives to support multiple subscriptions per user. The search for these variables in the codebase did not yield any results, indicating that they are not being used elsewhere and the removal should not cause any issues.apps/marketing/src/components/(marketing)/pricing-table.tsx (3)
3-4: The reorganization of import statements and the use of the
typekeyword foruseStatealign with TypeScript best practices.21-21: The direct initialization of the
useStatehook for theperiodvariable to'MONTHLY'is consistent with the changes described in the summary.18-24: The modifications to the
PricingTablefunction, including the removal ofuseSearchParamsand the direct state initialization, are in line with the PR's objectives to support the new subscription model.apps/web/src/app/(dashboard)/admin/users/data-table-users.tsx (4)
11-11: The import statement correctly uses the
typekeyword for type-only imports.19-24: The
UserDatatype has been updated to support an array ofSubscriptionLite, which aligns with the PR's objective.35-39: The
UsersDataTablePropstype has been extended to includeindividualPriceIds, which is necessary for the new subscription filtering logic.107-117: The logic within the 'Subscription' column's
cellfunction has been updated to handle multiple subscriptions, which is consistent with the new feature requirements.apps/web/src/app/(dashboard)/admin/users/page.tsx (3)
1-1: The addition of the
getPricesByTypeimport aligns with the PR's objective to handle multiple subscriptions.19-24: The integration of
getPricesByTypewithinAdminManageUsersand passing its result toUsersDataTableis consistent with the PR's objectives.29-35: The update to
UsersDataTableto acceptindividualPriceIdsis a necessary adjustment for the new subscription model.apps/web/src/app/(dashboard)/settings/billing/billing-plans.tsx (2)
7-7: The change to import
PriceIntervalsas a type is consistent with TypeScript best practices for type-only imports and aligns with the PR's objectives to enhance type safety.4-10: No further issues found in the reviewed hunk.
apps/web/src/app/(dashboard)/settings/billing/create-billing-portal.action.ts (2)
3-13: The refactoring of the
createBillingPortalfunction to usegetStripeCustomerByUsersimplifies the logic for Stripe customer management and aligns with the PR's objective to support multiple subscriptions per user.13-13: Ensure that the
NEXT_PUBLIC_WEBAPP_URLenvironment variable is correctly set in the deployment environment to avoid any issues with the return URL after the billing portal session is created.
Verification result:
The review comment regarding the
NEXT_PUBLIC_WEBAPP_URLenvironment variable cannot be verified because the.env.localfile does not exist in the directory where the command was executed. This could mean that the environment variable is not set, or it might be set in a different environment file or at a different level (like global environment variables on the server or in a container orchestration system).Please ensure that the
NEXT_PUBLIC_WEBAPP_URLenvironment variable is correctly set in the appropriate configuration file or environment variable management system used in the deployment environment to avoid any issues with the return URL after the billing portal session is created.apps/web/src/app/(dashboard)/settings/billing/create-checkout.action.ts (2)
3-33: The changes in the
createCheckoutfunction reflect the PR's objective to support multiple subscriptions per user. The use ofgetStripeCustomerByUserandgetSubscriptionsByUserIdaligns with the updated logic for handling Stripe customers and subscriptions. Ensure that the new functions are thoroughly tested, especially since they replace previous functions likegetStripeCustomerByIdandgetStripeCustomerByEmail.20-33: The logic to handle existing subscriptions and redirect to the billing portal or create a new checkout session appears to be correctly implemented. It's important to verify that the
returnUrlis correctly configured and that the environment variableNEXT_PUBLIC_WEBAPP_URLis properly set in all deployment environments.apps/web/src/app/(dashboard)/settings/billing/page.tsx (9)
8-8: The summary does not mention the addition of the
getProductByPriceIdimport from@documenso/ee/server-only/stripe/get-product-by-price-id, which is present in the hunk.6-6: The summary does not mention the addition of the
getPricesByIntervalimport from@documenso/ee/server-only/stripe/get-prices-by-interval, which is present in the hunk.10-10: The summary does not mention the addition of the
getServerComponentFlagimport from@documenso/lib/server-only/feature-flags/get-server-component-feature-flag, which is present in the hunk.9-9: The summary does not mention the addition of the
getRequiredServerComponentSessionimport from@documenso/lib/next-auth/get-server-component-session, which is present in the hunk.2-24: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [1-1]
The summary does not mention the addition of the
redirectimport fromnext/navigation, which is present in the hunk.
3-3: The summary does not mention the addition of the
matchimport fromts-pattern, which is present in the hunk.17-18: The summary does not mention the addition of
BillingPlansandBillingPortalButtoncomponents, which are present in the hunk.15-15: The summary does not mention the addition of the
LocaleDateimport from~/components/formatter/locale-date, which is present in the hunk.27-32: The summary does not mention the logic for redirecting if billing is not enabled or if the user does not have a customerId, which is present in the hunk.
packages/ee/server-only/limits/server.ts (1)
- 64-64: The logic for updating the quota and remaining values checks if both
documentsandrecipientsare higher in the current quota. Consider if this is the intended behavior, as it may not update the quota if only one of the values is higher.packages/ee/server-only/stripe/get-customer.ts (3)
1-12: The changes to import statements and function signatures are consistent with the PR objectives and the summary provided.
32-81: The implementation of
getStripeCustomerByUserappears to correctly handle the retrieval or creation of a Stripe customer based on the user'scustomerId. It also updates the user record with the newcustomerIdif a new Stripe customer is created. Ensure that the error handling and subscription syncing logic are thoroughly tested, as these are critical operations.83-96: The
syncStripeCustomerSubscriptionsfunction correctly retrieves Stripe subscriptions and processes them concurrently usingPromise.all. This approach is efficient, but ensure that the system can handle the potential load and that there is proper error handling for each subscription update.packages/ee/server-only/stripe/get-prices-by-interval.ts (5)
1-1: The import statement has been correctly updated to import
Stripeas a type, aligning with TypeScript best practices for type-only imports.10-15: The introduction of
GetPricesByIntervalOptionswith an optionalfilterTypeproperty is a good practice for encapsulating function parameters, especially when dealing with options that may expand in the future.17-20: The function
getPricesByIntervalhas been correctly updated to accept an options object, which is a good practice for functions that may need more parameters in the future or have optional parameters.29-32: The filtering logic based on the
filterTypeparameter is implemented correctly, allowing the function to be more flexible and support the new feature of filtering by product type.26-27: The use of a type assertion for
price.productis necessary due to the use of theexpandoption in Stripe's API call, which is not reflected in the basePricetype. However, it's important to ensure that the expanded product is always present to avoid runtime errors.packages/ee/server-only/stripe/webhook/on-subscription-deleted.ts (3)
1-1: The import statement for the
Stripetype has been correctly updated to use thetypekeyword, aligning with TypeScript best practices for type-only imports.9-15: The
onSubscriptionDeletedfunction has been updated to useplanIdfrom thesubscriptionobject, which aligns with the PR objectives to handle multiple subscriptions per user and the shift to usingplanIdfor subscription identification.7-15: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-15]
The changes in the provided hunks are consistent with the PR objectives and summaries, and there are no apparent issues with logic, security, performance, or maintainability.
packages/ee/server-only/stripe/webhook/on-subscription-updated.ts (4)
3-3: The change to the import statement using the
typekeyword is correct and aligns with TypeScript best practices for importing types.26-26: Verify that
planIdis the correct and unique identifier for a subscription in the context of the application's logic and database schema.23-29: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [28-37]
The use of
subscription.idforplanIdin bothcreateandupdateblocks of theupsertoperation is consistent with the PR's objectives to handle multiple subscriptions.
- 1-6: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [10-11]
Ensure that the
subscription.customerfield is always present and correctly formatted to avoid potential issues when determiningcustomerId.packages/lib/server-only/admin/get-users-stats.ts (1)
- 9-17: The logic change in
getUsersWithSubscriptionsCountto count users with any active subscriptions aligns with the PR's objective to support multiple subscriptions per user.packages/lib/server-only/subscription/get-subscriptions-by-user-id.ts (1)
- 9-15: The implementation of
getSubscriptionsByUserIdaligns with the PR's objective to support multiple subscriptions per user and appears to be correctly fetching subscriptions based on theuserId.packages/prisma/migrations/20231206073509_add_multple_subscriptions/migration.sql (5)
11-11: The custom SQL statement to delete
Subscriptionrecords with nullplanIdorpriceIdis in line with the PR objectives to clean up the data before applying new constraints.20-21: The alteration of the
Subscriptiontable to setplanIdandpriceIdas NOT NULL is consistent with the PR objectives to enforce data integrity for subscriptions.24-24: The addition of the
customerIdcolumn to theUsertable aligns with the PR objectives to manage Stripe customer IDs.27-27: The creation of a unique index on the
planIdcolumn of theSubscriptiontable is consistent with the PR objectives to ensure unique subscriptions per plan.30-30: The creation of a unique index on the
customerIdcolumn of theUsertable is consistent with the PR objectives to ensure unique Stripe customer IDs per user.packages/prisma/schema.prisma (5)
21-27: The addition of the
customerIdfield to theUsermodel with the@uniquedirective is consistent with the PR objectives to manage Stripe customer IDs.38-38: The change from a singular to a plural
Subscriptionrelation in theUsermodel supports the objective of allowing multiple subscriptions per user.76-76: Verify that the
planIdfield in theSubscriptionmodel being marked as@uniquealigns with the intended business logic, as this enforces that each plan ID can only be associated with one subscription at a time.80-80: The removal of the
@uniquedirective from theuserIdfield in theSubscriptionmodel is consistent with the objective of supporting multiple subscriptions per user.79-79: The addition of the
periodEndfield in theSubscriptionmodel is not mentioned in the summary. Ensure that this new field is integrated into the application logic where necessary.packages/tsconfig/process-env.d.ts (1)
- 10-15: The hunk provided does not show the changes mentioned in the summary regarding
NEXT_PUBLIC_STRIPE_COMMUNITY_PLAN_YEARLY_PRICE_IDandNEXT_PUBLIC_STRIPE_FREE_PLAN_ID. Ensure that the changes are correctly reflected in the file and that any code relying on these environment variables is updated accordingly.
| DROP INDEX "Subscription_customerId_key"; | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "Subscription_userId_key"; |
There was a problem hiding this comment.
💡 NOTE
Codebase verification is a beta feature.
The migration script drops indexes on customerId and userId from the Subscription table, and verification shows that these indexes are not recreated or modified later in the migration process. This change should be carefully reviewed to ensure it is intentional and does not negatively impact the database's performance or integrity.
Original review comment
The migration script drops indexes related to customerId and userId on the Subscription table. This change was not mentioned in the summary and should be verified to ensure it aligns with the intended database schema changes.
|
|
||
| export type CreateCheckoutOptions = { | ||
| priceId: string; | ||
| }; | ||
|
|
||
| export const createCheckout = async ({ priceId }: CreateCheckoutOptions) => { | ||
| const { user } = await getRequiredServerComponentSession(); | ||
| const session = await getRequiredServerComponentSession(); |
There was a problem hiding this comment.
can just have const { user } = await ... ?
packages/prisma/migrations/20231206073509_add_multple_subscriptions/migration.sql
Show resolved
Hide resolved
|
I'll trust ya on the rest, reading this is making me tired |
|
It's pretty critical since it changes billing a lot so could you please do a mini migration test as well to confirm? |
Co-authored-by: Lucas Smith <me@lucasjamessmith.me>
dguyen
force-pushed
the
feat/multi-subscription-support
branch
from
6ebc112
to
f52eef8
Compare
packages/prisma/schema.prisma
Outdated
| planId String? | ||
| priceId String? | ||
| planId String @unique | ||
| priceId String | ||
| customerId String |
There was a problem hiding this comment.
Should we be tossing the customerId here since it's on the user?
There was a problem hiding this comment.
That's true, I've removed it but it required some changes to the webhook handlers
Also noticed that we aren't actually updating cancelAtPeriodEnd when a subscription changes so I've included those changes
|
Thank you for following the naming conventions for pull request titles! 💚🚀 |
github-actions
bot
added
apps: web
Description
Previously we assumed that there can only be 1 subscription per user. However, that will soon no longer the case with the introduction of the Teams subscription.
This PR will apply the required migrations to support multiple subscriptions.
Changes Made
SubscriptionsperUsercustomerIdfield to theUsermodelTesting Performed
Will require a lot of additional testing.
Checklist
Additional Notes
Added the following custom SQL statement to the migration:
Prior to deployment this will require changes to Stripe products:
typemeta attributeSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Style
Chores
Revert