Conversation
chronark
requested review from
MichaelUnkey,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis pull request introduces significant updates to billing and subscription management features. Workspace creation now includes new properties such as organization IDs and plan tiers. The billing UI is refactored, removing legacy components and introducing new components for handling subscriptions, usage metrics, and confirmation dialogs. Additionally, Stripe integration endpoints and flows have been updated, including new TRPC procedures for creating, updating, and canceling subscriptions. The changes extend to enhancements on the pricing page, new database schema definitions for quotas, and updates to environment settings and tooling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as Billing UI (Client/Components)
participant T as TRPC Endpoint
participant S as Stripe API
U->>B: Initiates subscription action
B->>T: Calls createSubscription/updateSubscription/cancelSubscription
T->>S: Makes API call to Stripe (checkout, update, or cancel)
S-->>T: Returns session/confirmation details
T-->>B: Provides subscription update response
B->>U: Updates UI / Redirects user appropriately
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
tools/migrate/stripe.ts (1)
66-67: 🛠️ Refactor suggestionAdd process exit handling
The main function is asynchronous but missing proper handling of its promise. If an error occurs, the script might terminate without reporting the error.
-main(); +main() + .then(() => { + console.log("Migration completed successfully"); + process.exit(0); + }) + .catch((error) => { + console.error("Migration failed:", error); + process.exit(1); + });
🧹 Nitpick comments (16)
tools/migrate/stripe.ts (2)
1-7: Consider using TypeScript imports for better type safetyThe current import uses CommonJS-style
require()while settingtypescript: truein the Stripe options. For better type safety and consistency, consider using ES module imports.-const Stripe = require("stripe"); +import Stripe from "stripe";
9-63: Extract duplicated metadata structure into a helper functionThe script contains repetitive code for updating products with the same metadata structure. Consider refactoring this to reduce duplication and improve maintainability.
async function main() { const stripe = new Stripe(process.env.STRIPE_SECRET_KEY, { apiVersion: "2023-10-16", typescript: true, }); + const productUpdates = [ + { id: "prod_Rt7pECOwzDIiHC", requestsPerMonth: "250000" }, + { id: "prod_Rt7phP6Jg6CEAX", requestsPerMonth: "500000" }, + { id: "prod_Rt7skQe4k40vB9", requestsPerMonth: "1000000" }, + { id: "prod_Rt7sxA99W66fei", requestsPerMonth: "2000000" }, + { id: "prod_Rt7sdc9yBJlixM", requestsPerMonth: "10000000" }, + { id: "prod_Rt7s7o5GODuYEc", requestsPerMonth: "50000000" }, + { id: "prod_Rt7tA07o4WOVmc", requestsPerMonth: "100000000" }, + ]; + + for (const product of productUpdates) { + await stripe.products.update(product.id, { + metadata: { + quota_requests_per_month: product.requestsPerMonth, + quota_logs_retention_days: "90", + quota_audit_logs_retention_days: "90", + }, + }); + }tools/local/src/main.ts (1)
85-85: Optionally allow skipping the dev server.Removing the confirmation prompt to run the dev server simplifies the process but may reduce flexibility for users who don’t want to run it right away. Consider adding a simple flag to skip starting the dev server if necessary.
You could reintroduce a similar check as follows:
- execSync(`pnpm --dir=apps/${app} dev`, { cwd: "../..", stdio: "inherit" }); + if (!passedOptions["no-dev"]) { + execSync(`pnpm --dir=apps/${app} dev`, { cwd: "../..", stdio: "inherit" }); + } else { + console.log("Skipping dev server..."); + }apps/dashboard/pages/api/v1/stripe/webhooks.ts (1)
130-159: Extract common “revert to free” logic.Repeatedly applying the same tier and quota reset across multiple subscription lifecycle events can be tidy and less error-prone if extracted into a helper function. This centralizes changes and improves maintainability.
internal/db/src/schema/workspaces.ts (3)
31-31: Consider adding a foreign key constraintIf
orgIdreferences an organizations table, explicitly linking via a foreign key can help ensure data integrity.
35-35: Plan column deprecation reminderSince the comment indicates
planis deprecated, plan a future cleanup to remove or fully replace the column once everything is migrated totier.
37-38: Align casing for tier valuesThe
planenum values are lower case, whereastierdefaults to"Free". Consider standardizing the case to avoid confusion.apps/www/app/pricing/page.tsx (1)
28-57: Centralized bucket configurationDefining the pricing buckets in a local array is fine, but consider extracting them into a shared config or environment variable if they will be managed or updated frequently.
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (1)
81-142: Handling partial failuresWrap the multi-step logic (retrieving the session, updating DB, creating the subscription) in try/catch blocks. Otherwise, an API issue could leave your DB partially updated with inconsistent data.
apps/www/app/pricing/select.tsx (2)
22-24: Encapsulate Hard-Coded Brand Colors in Theme TokensBy referencing
#FFD600directly, you may face maintainability challenges if branding changes. Consider defining a theme token or CSS variable instead, ensuring a cohesive approach to branding across your project.
114-116: Enhance Hover & Focus States for AccessibilityWhile the selected item has clear styling, you might consider introducing distinct hover/focus states with accessible color contrasts or aria attributes for improved user experience, particularly for keyboard navigation or screen readers.
apps/dashboard/app/(app)/settings/billing/page.tsx (5)
35-36: Clarify Distinction Between Legacy & New PlansCurrently,
isLegacychecks for any workspace subscriptions, whereasisNewchecks for a missingstripeCustomerId. Adding comments or restructuring the checks can help future maintainers quickly understand the logic behind these conditions.
55-57: Graceful Handling of Stripe Subscription Retrieval ErrorsIf the Stripe subscription retrieval call fails or returns a 404, it may be beneficial to surface an error message or provide a retry option rather than silently returning
null. This approach can improve user experience in cases of transient network failures.
67-95: Refactor Common Billing LogicBoth the Legacy and new plan usage calculations rely on the same ClickHouse metrics but differ slightly in UI. Consider extracting the common logic into a shared helper function or component, thereby reducing duplication and simplifying future maintenance.
120-140: Mark Numeric Fields as Read-OnlyWhen displaying usage metrics in
<Input>elements, marking them asreadOnlycan prevent user confusion and accidental edits, ensuring these fields are recognized as informational rather than interactive.
290-338: Improve ProgressCircle AccessibilityWhile the visual representation is intuitive, adding an
aria-labelor textual fallback within the same container helps screen readers convey usage progress accurately to visually impaired users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/api/src/pkg/testutil/harness.ts(2 hunks)apps/dashboard/app/(app)/settings/billing/page.tsx(3 hunks)apps/dashboard/app/(app)/settings/billing/plans/button.tsx(0 hunks)apps/dashboard/app/(app)/settings/billing/plans/constants.ts(0 hunks)apps/dashboard/app/(app)/settings/billing/plans/navigation.tsx(0 hunks)apps/dashboard/app/(app)/settings/billing/plans/page.tsx(0 hunks)apps/dashboard/app/(app)/settings/billing/stripe/page.tsx(2 hunks)apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx(0 hunks)apps/dashboard/components/settings-card.tsx(1 hunks)apps/dashboard/lib/env.ts(1 hunks)apps/dashboard/lib/trpc/routers/workspace/create.ts(1 hunks)apps/dashboard/pages/api/v1/stripe/webhooks.ts(2 hunks)apps/docs/libraries/ts/nextjs.mdx(2 hunks)apps/www/app/pricing/discover.tsx(0 hunks)apps/www/app/pricing/page.tsx(6 hunks)apps/www/app/pricing/select.tsx(1 hunks)apps/www/package.json(1 hunks)internal/db/src/schema/index.ts(1 hunks)internal/db/src/schema/quota.ts(1 hunks)internal/db/src/schema/workspaces.ts(4 hunks)tools/local/src/main.ts(1 hunks)tools/migrate/stripe.ts(1 hunks)
💤 Files with no reviewable changes (6)
- apps/dashboard/app/(app)/settings/billing/plans/constants.ts
- apps/dashboard/app/(app)/settings/billing/plans/navigation.tsx
- apps/dashboard/app/(app)/settings/billing/plans/page.tsx
- apps/dashboard/app/(app)/settings/billing/plans/button.tsx
- apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx
- apps/www/app/pricing/discover.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (1)
Learnt from: mcstepp
PR: unkeyed/unkey#2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-11-10T16:45:16.994Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (36)
apps/www/package.json (1)
28-28: New dependency added for React Select componentAdding the Radix UI Select component, which seems to be part of the changes supporting the new Stripe subscription functionality mentioned in the PR. This is a well-maintained UI component library that follows accessibility best practices.
internal/db/src/schema/index.ts (1)
10-10: New quota module exportedThe addition of this export makes the quota schema available throughout the application. This is likely part of the subscription functionality for managing workspace quotas based on subscription tiers.
apps/dashboard/components/settings-card.tsx (1)
30-30: Enhanced border styling flexibilityThis change adds the ability to remove the bottom border when the
borderprop is set to "top", completing the styling options for border combinations. The implementation is clean and consistent with the existing pattern.apps/api/src/pkg/testutil/harness.ts (1)
258-258:Details
✅ Verification successful
New tier property for workspace objects
Adding the
tierproperty to test workspace objects aligns with the broader schema changes in this PR. This ensures test fixtures properly mirror the new subscription model being implemented.Note that both
planandtierproperties exist simultaneously. Based on the summary,tieris meant to replace the deprecatedplanproperty. You may want to consider a future update to remove theplanproperty for consistency.Also applies to: 279-279
🏁 Script executed:
#!/bin/bash # Check if tier is used in other workspace-related files to confirm consistent usage rg -A 2 -B 2 "tier:" --type ts .Length of output: 9906
Update Verified: Test Harness Correctly Implements the New Subscription Model
The addition of the
tierproperty in workspace objects has been verified across the codebase. In fileapps/api/src/pkg/testutil/harness.ts(lines 258 and 279), thetierproperty is now set (e.g.,"Enterprise"and"Pro Max"), while the legacyplanproperty still exists. This change aligns with the broader schema updates and ensures that our test fixtures mirror the new subscription model accurately.
- Note: The
planproperty remains for backward compatibility. Consider a future update to removeplanfor consistency once the transition is complete.apps/dashboard/lib/trpc/routers/workspace/create.ts (2)
43-43:Details
❓ Verification inconclusive
Removed defaultProSubscriptions replacement
The previous call to
defaultProSubscriptions()has been replaced with an empty object. Verify that this doesn't affect downstream functionality.
🏁 Script executed:
#!/bin/bash # Search for usage of subscriptions field echo "Searching for usage of workspace subscriptions field..." rg --type-add 'ts:*.{ts,tsx}' --type ts "workspace.*subscriptions" -C2Length of output: 5118
Action: Verify downstream billing/invoicing behavior with an empty subscriptions object.
The change replacing the previousdefaultProSubscriptions()call with an empty object{}in workspace creation now means that new workspaces start with no default subscription values. Our search confirmed that several parts of the codebase (billing pages, subscription settings, and invoicing workflows) rely on checkingworkspace.subscriptions—typically verifying whether the object contains any keys. Please ensure that:
- Billing and subscription pages (e.g., in
apps/dashboard/app/(app)/settings/billing/) correctly handle the empty subscriptions state.- Invoicing workflows (e.g., in
apps/workflows/src/workflows/invoicing.ts) appropriately filter or process workspaces based on an empty subscriptions object.- Any expected default behavior previously provided by
defaultProSubscriptions()is either no longer necessary or has been reimplemented elsewhere.Review these downstream functionalities to confirm that the removal of default subscriptions does not introduce unintended side effects.
35-35:Details
❓ Verification inconclusive
Ensure "Free" tier aligns with Stripe subscription model
The workspace is initialized with a "Free" tier but still has plan "pro". Verify this is the intended behavior and consistent with the new subscription model.
🏁 Script executed:
#!/bin/bash # Check for other locations where the tier property is used echo "Searching for tier usage in workspaces..." rg --type-add 'ts:*.{ts,tsx}' --type ts "tier.*Free" -C2 echo "Checking workspace schema for tier definition..." rg --type-add 'ts:*.{ts,tsx}' --type ts -A10 "workspaces\s*=\s*mysqlTable" internal/db/src/schemaLength of output: 2744
Subscription Model Consistency: Verify Workspace Tier and Plan Usage
We have verified that across the codebase the workspace creation currently sets:
- In
apps/dashboard/lib/trpc/routers/workspace/create.ts, a workspace is created withplan: "pro"alongsidetier: "Free".- In the Stripe webhook updates (
apps/dashboard/pages/api/v1/stripe/webhooks.ts), thetierfield is similarly set to"Free".- The database schema (
internal/db/src/schema/workspaces.ts) shows a default of"free"for the legacyplanand defaults"Free"for the newtierfield (with an inline note indicating thattierreplacesplan).Please confirm if setting
planto"pro"while using a"Free"tier is intentional and aligns with the updated Stripe subscription model—or if both fields should be harmonized to reflect our current subscription strategy.internal/db/src/schema/quota.ts (3)
5-11: Well-documented schema with clear JSDoc commentsThe documentation for this new schema is excellent. The JSDoc comments clearly explain the purpose of the table and its role in the system.
12-17:Details
✅ Verification successful
Table name inconsistency between singular and plural
The table is named "quota" (singular) while the exported constant is named "quotas" (plural). Consider making these consistent to avoid confusion. Also ensure this matches your naming conventions for other tables.
-export const quotas = mysqlTable("quota", { +export const quotas = mysqlTable("quotas", {
🏁 Script executed:
#!/bin/bash # Check naming conventions in other schema files echo "Checking naming conventions in other schema files..." grep -r "mysqlTable" --include="*.ts" internal/db/src/schema/ | grep -v quota.ts | head -10Length of output: 993
Address table naming inconsistency
We've confirmed that our naming convention across the schema files uses plural names (e.g., "workspaces", "vercel_integrations"), and the table in
internal/db/src/schema/quota.tsshould follow that pattern. Please update the table name from"quota"to"quotas"to maintain consistency.
- File:
internal/db/src/schema/quota.ts(Lines 12-17)-export const quotas = mysqlTable("quota", { +export const quotas = mysqlTable("quotas", {
25-25:Details
✅ Verification successful
Consider type safety for requestsPerMonth
The
requestsPerMonthis usingbigintwith number mode, which is good for large values. However, be aware that JavaScript has precision limitations for large numbers. If you expect values that could exceed Number.MAX_SAFE_INTEGER (9007199254740991), consider keeping it as a bigint.
🏁 Script executed:
#!/bin/bash # Check usage of requestsPerMonth field echo "Checking usage of requestsPerMonth field..." rg --type-add 'ts:*.{ts,tsx}' --type ts "requestsPerMonth" -C2Length of output: 3574
Type Safety Check – requestsPerMonth
After reviewing the field’s usage across the codebase, the current numeric values (e.g. 250000, 2_5000_000, etc.) remain well within JavaScript’s safe integer range. Using
bigintwith{ mode: "number" }is thus operating as expected in our applications. However, if there’s any future possibility of exceeding Number.MAX_SAFE_INTEGER (9007199254740991), it would be prudent to consider preserving the value in its native bigint format.apps/docs/libraries/ts/nextjs.mdx (1)
47-49: ValidateapiIdenvironment variable before usage.Using
process.env.UNKEY_API_ID!bypasses null checks and can lead to runtime errors if the variable is undefined. Consider validating this variable to ensure more robust error handling.Here is a sample diff illustrating a validation approach:
export const POST = withUnkey(async (req) => { // ... return new Response('Your API key is valid!'); -}, { apiId: process.env.UNKEY_API_ID! }); +}, (() => { + if (!process.env.UNKEY_API_ID) { + throw new Error("Missing UNKEY_API_ID environment variable"); + } + return { apiId: process.env.UNKEY_API_ID }; +})());apps/dashboard/pages/api/v1/stripe/webhooks.ts (2)
2-3: Imports look good.The updated imports from
@/lib/dband@/lib/envare appropriate. No issues found here.
52-129:Details
❓ Verification inconclusive
Ensure all subscription statuses and metadata usage are valid.
pausedstatus: Stripe typically returns standard statuses (e.g.,trialing,active,incomplete,canceled, etc.). Verify that your integration handles or expects this custompausedstatus as intended.- Product metadata keys: If the product metadata values contain non-numeric inputs or are missing,
Number.parseInt()calls will fail. Consider adding defensive checks to handle unexpected metadata.Below is a script that searches for references to
"paused"and checks for additional references that might confirm or conflict with the intended usage:
🏁 Script executed:
#!/bin/bash # Searching for "paused" references in the entire codebase to verify usage rg 'paused'Length of output: 85
Action Required: Confirm Custom "paused" Status and Validate Metadata Handling
- The search confirms that the
"paused"status is only used inapps/dashboard/pages/api/v1/stripe/webhooks.tsas a custom case. Please verify with your Stripe integration and documentation that this non-standard status is intentional and handled appropriately.- Review and add defensive checks around the metadata values used in
Number.parseInt(...)(for keys likequota_requests_per_month,quota_logs_retention_days, andquota_audit_logs_retention_days) to ensure they contain numeric values and handle any missing or unexpected inputs.internal/db/src/schema/workspaces.ts (3)
17-17: Import statement looks correct
118-118: One-to-one relation introductionAdding
oneto manage relations withquotasis a valid approach for the new schema design.
137-137: Verify foreign key constraints for quotaEnsure the
quotastable defines the corresponding references or constraints to make this one-to-one relationship consistent.apps/www/app/pricing/page.tsx (12)
1-2: Client component setupAdopting
"use client"in this context is appropriate for a dynamic pricing page, and importing the CTA is straightforward.
6-6: New icon importNo concerns;
CheckandStarsicons are appropriately used later in the layout.
8-9: Using Next.js Link and React stateThese imports are standard and correctly applied.
26-26: Custom Select componentImporting the select elements from "./select" is clear and well-structured.
60-62: Intl.NumberFormat usageUsing compact notation for large numbers is a neat approach. Just be mindful of edge cases where the output might be ambiguous for some end-users.
117-119: Updated free-tier quotaIncreasing to "150k valid requests / month" can offer more flexibility for users scaling on the free tier.
122-123: Clarified logs retentionMentioning "7-day logs retention" provides transparent expectations for free tier users.
132-134: Spacing changeNo functional impact; this is purely a UI spacing tweak.
165-182: Configurable Pro tierGood implementation of a dynamic dropdown and cost display using
buckets. The state-based approach is straightforward.
191-191: Dynamic request labelCorrectly referencing the selected bucket’s request limit for display.
196-196: Pro tier loggingSwitching to a 30-day logs retention clarifies the upgrade benefits.
211-217: Transparent overage policyClear instructions about usage exceedance helps users feel more secure with their plan.
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (8)
1-1: New Code component importAllowing more expressive debugging or display with
<Code />is sensible.
3-3: Extended DB utilitiesPulling in
eqandschemais standard practice for typed DB updates.
57-58: Potential domain mismatchRelying on
process.env.VERCEL_URLmight conflict with custom hosting. Also, not all clients send arefererheader. Consider a safer configuration for redirects.
60-80: Portal actionThe code checks
ws.stripeCustomerIdbefore generating a billing portal session. If older workspaces never set up Stripe, ensure the UI or user flow directs them to create or link a customer first.
143-167: Setup mode sessionRedirecting to a Stripe session is straightforward. Fallback logic checks if
session.urlis available, which is crucial for preventing blank redirects.
168-186: Subscription update flowUsing
flow_datato guide subscription updates in the billing portal is a clean and maintainable approach.
188-207: Subscription cancellationConsistent approach as with the update flow. Good job ensuring correct references to
ws.stripeSubscriptionId.
210-217: Fallback for unmatched actionsGraceful user-facing message if an unknown action is requested.
apps/dashboard/app/(app)/settings/billing/page.tsx (1)
170-172: Verify Subscription Cancelation FlowEnsure that
/settings/billing/stripe?action=subscription_cancelproperly cancels the subscription. Would you like me to generate a shell script to search for the related route or handler in the codebase for confirmation?
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (1)
1-222: 🛠️ Refactor suggestionMissing error handling for Stripe API calls
Throughout the file, there's minimal error handling for Stripe API calls that could fail. Consider wrapping each action case with try/catch blocks to handle API errors gracefully.
Additionally, consider extracting common error UI patterns into reusable functions to reduce duplication across the different error states.
Example implementation for a reusable error component:
function ErrorUI({ title, description, code }: { title: string; description: string; code?: string }) { return ( <Empty> <Empty.Title>{title}</Empty.Title> <Empty.Description>{description}</Empty.Description> {code && <Code>{code}</Code>} <Empty.Description> Please contact support@unkey.dev for assistance. </Empty.Description> </Empty> ); }This could then be used throughout the file to standardize error UI.
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (3)
31-35: Consider adding a comment explaining the auditLogBuckets usageThe workspace query now includes auditLogBuckets with a specific filter for "unkey_mutations", but it's not clear where this data is used in the component.
with: { auditLogBuckets: { where: (table, { eq }) => eq(table.name, "unkey_mutations"), + // Used for tracking mutation counts for billing purposes }, },
147-171: Improve error handling for checkout session creationThe error handling for the checkout session creation only checks if the URL is missing but doesn't handle other potential errors from the Stripe API.
case "payment_intent": { + try { const session = await stripe.checkout.sessions.create({ // ...existing configuration }); if (!session.url) { // ...existing error UI } return redirect(session.url); + } catch (error) { + console.error("Failed to create checkout session:", error); + return ( + <Empty> + <Empty.Title>Failed to create checkout session</Empty.Title> + <Empty.Description> + An error occurred while setting up your payment. Please contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + } }
214-221: Consider providing more specific error informationThe fallback error UI is very generic. Consider adding more context about what action was attempted and failed.
return ( <Empty> <Empty.Title>Stripe Error</Empty.Title> <Empty.Description> - You should have been redirected, please report this to support@unkey.dev + Failed to process action: "{props.searchParams.action}". Please report this to support@unkey.dev and include this information. </Empty.Description> </Empty> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (1)
Learnt from: mcstepp
PR: unkeyed/unkey#2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-11-10T16:45:16.994Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (2)
10-19: Type definition updated to support more Stripe actionsThe searchParams type has been improved to support multiple actions beyond just plan changes, which provides better type safety and makes the component's capabilities clearer.
64-84: Well-structured portal action handlingThe portal action handling is well-implemented with proper error states when a customer ID is missing and appropriate redirection to the Stripe portal when available.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx (3)
40-43: Consider using environment variable for Stripe API version.The Stripe API version is hardcoded. This could cause maintenance issues when Stripe updates their API.
- const stripe = new Stripe(e.STRIPE_SECRET_KEY, { - apiVersion: "2023-10-16", - typescript: true, - }); + const stripe = new Stripe(e.STRIPE_SECRET_KEY, { + apiVersion: e.STRIPE_API_VERSION || "2023-10-16", + typescript: true, + });
45-49: Use environment variables for production URL.Hardcoding the production URL could be problematic if it changes in the future.
- const baseUrl = process.env.VERCEL - ? process.env.VERCEL_TARGET_ENV === "production" - ? "https://app.unkey.com" - : `https://${process.env.VERCEL_BRANCH_URL}` - : "http://localhost:3000"; + const baseUrl = process.env.VERCEL + ? process.env.VERCEL_TARGET_ENV === "production" + ? process.env.PRODUCTION_URL || "https://app.unkey.com" + : `https://${process.env.VERCEL_BRANCH_URL}` + : process.env.LOCAL_URL || "http://localhost:3000";
61-72: Improve error message clarity.The current error message is confusing. It states the session is empty, then says it doesn't exist, which is contradictory.
if (!session.url) { return ( <Empty> - <Empty.Title>Empty Session</Empty.Title> - <Empty.Description>The Stripe session</Empty.Description> - <Code>{session.id}</Code> - <Empty.Description> - you are trying to access does not exist. Please contact support@unkey.dev. - </Empty.Description> + <Empty.Title>Missing Session URL</Empty.Title> + <Empty.Description> + The Stripe session was created but no redirect URL was provided. + Please contact support@unkey.dev with reference ID: <Code>{session.id}</Code> + </Empty.Description> </Empty> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx(1 hunks)apps/dashboard/app/(app)/settings/billing/subscriptions/page.tsx(0 hunks)apps/dashboard/app/(app)/settings/billing/user-payment-method.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- apps/dashboard/app/(app)/settings/billing/subscriptions/page.tsx
- apps/dashboard/app/(app)/settings/billing/user-payment-method.tsx
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test API / API Test Local
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/settings/billing/components/shell.tsx (3)
1-2: Consider clarifying naming for imported components.Importing two different components with the same name (
Navbar) and renaming one toSubMenucould lead to confusion. Consider using more descriptive naming or restructuring the imports for better clarity.
22-23: Avoid hardcoding external URLs in components.The calendar URL is hardcoded, which makes it difficult to update if the URL changes. Consider moving this to a constants file or environment variable.
33-34: Consider improving responsive behavior.The fixed width of 760px might not be responsive on smaller screens. Consider using relative units or responsive classes like
max-w-[760px] w-fullfor better mobile support.- <div className="w-[760px] mt-4 flex flex-col justify-center items-center gap-5"> + <div className="max-w-[760px] w-full mt-4 flex flex-col justify-center items-center gap-5">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/dashboard/app/(app)/settings/billing/components/shell.tsx(1 hunks)apps/dashboard/lib/env.ts(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts(1 hunks)apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts(1 hunks)apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts(1 hunks)apps/dashboard/lib/trpc/routers/workspace/changePlan.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/lib/trpc/routers/workspace/changePlan.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/lib/trpc/routers/index.ts
- apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts
- apps/dashboard/lib/env.ts
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Docs
- GitHub Check: autofix
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/dashboard/app/(app)/settings/billing/components/shell.tsx (1)
10-44: LGTM! Well-structured component layout.The overall structure of the Shell component provides a clean and consistent layout for the billing settings interface. The component properly uses composition with children props, allowing for flexible content rendering.
apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (2)
80-82: Guard against missing default price.This code forcibly uses
product.default_price!without verifying its existence, potentially causing a runtime error if no default price is set.You can apply a safeguard like this:
- price: product.default_price!.toString(), + price: product.default_price + ? product.default_price.toString() + : (() => { + throw new TRPCError({ + code: "NOT_FOUND", + message: "No default price set for this product.", + }); + })(),
100-105: Validate product metadata fields.Using
Number.parseInt(product.metadata.quota_requests_per_month)assumes this metadata entry always exists and is valid. Consider adding a fallback or guard to prevent errors if the metadata is missing or non-numeric.- requestsPerMonth: Number.parseInt(product.metadata.quota_requests_per_month), + requestsPerMonth: Number.parseInt( + product.metadata.quota_requests_per_month || "0" + ),apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts (2)
76-77: Avoid forced non-null assertion ondefault_price.Directly accessing
newProduct.default_price!.toString()can lead to runtime errors if the product has no default price set.- price: newProduct.default_price!.toString(), + if (!newProduct.default_price) { + throw new TRPCError({ + code: "PRECONDITION_FAILED", + message: "No default price found for the new product.", + }); + } + await stripe.subscriptionItems.update(item.id!, { + price: newProduct.default_price.toString(), + proration_behavior: "always_invoice", + });
90-108: Verify new product metadata before parsing.Like in
createSubscription.ts, parsing numeric metadata without checks may throw errors if values are missing or invalid. Consider safe fallbacks.- requestsPerMonth: Number.parseInt(newProduct.metadata.quota_requests_per_month), + requestsPerMonth: Number.parseInt( + newProduct.metadata.quota_requests_per_month || "0" + ),
….tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx (5)
51-60: 🛠️ Refactor suggestionAdd cancel_url and error handling to checkout session creation.
The checkout session creation is missing a cancel_url parameter, which prevents users from easily aborting the checkout process. Additionally, there's no error handling for potential failures during session creation.
if (!props.searchParams.session_id) { + try { const session = await stripe.checkout.sessions.create({ client_reference_id: ws.id, billing_address_collection: "auto", mode: "setup", success_url: `${baseUrl}/settings/billing/stripe/checkout?session_id={CHECKOUT_SESSION_ID}`, + cancel_url: `${baseUrl}/settings/billing`, currency: "USD", customer_creation: "always", }); + } catch (error) { + return ( + <Empty> + <Empty.Title>Failed to create checkout session</Empty.Title> + <Empty.Description> + There was an error creating your checkout session. Please try again or contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + }
77-88: 🛠️ Refactor suggestionAdd try/catch for session retrieval.
Missing error handling when retrieving the Stripe session.
+ try { const session = await stripe.checkout.sessions.retrieve(props.searchParams.session_id); if (!session) { return ( <Empty> <Empty.Title>Stripe session not found</Empty.Title> <Empty.Description> The Stripe session you are trying to access does not exist. Please contact support@unkey.dev. </Empty.Description> </Empty> ); } + } catch (error) { + return ( + <Empty> + <Empty.Title>Failed to retrieve Stripe session</Empty.Title> + <Empty.Description> + There was an error retrieving your checkout session. Please try again or contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + }
89-101: 🛠️ Refactor suggestionImprove type safety and add error handling for customer retrieval.
The code uses type assertion and lacks error handling for the Stripe API call.
- const customer = await stripe.customers.retrieve(session.customer as string); + if (typeof session.customer !== 'string') { + return ( + <Empty> + <Empty.Title>Invalid customer ID</Empty.Title> + <Empty.Description> + The session does not contain a valid customer ID. Please contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + } + + let customer; + try { + customer = await stripe.customers.retrieve(session.customer); + } catch (error) { + return ( + <Empty> + <Empty.Title>Failed to retrieve customer</Empty.Title> + <Empty.Description> + There was an error retrieving your customer information. Please try again or contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + }
113-123: 🛠️ Refactor suggestionImprove error handling and type safety for setup intent retrieval.
Add error handling and proper type checking for the setup intent.
- const setupIntent = await stripe.setupIntents.retrieve(session.setup_intent.toString()); - if (!setupIntent.payment_method) { + let setupIntent; + try { + setupIntent = await stripe.setupIntents.retrieve( + typeof session.setup_intent === 'string' ? session.setup_intent : session.setup_intent.toString() + ); + } catch (error) { + return ( + <Empty> + <Empty.Title>Failed to retrieve setup intent</Empty.Title> + <Empty.Description> + There was an error retrieving your payment setup information. Please try again or contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + } + if (!setupIntent.payment_method || typeof setupIntent.payment_method !== 'string' && typeof setupIntent.payment_method !== 'object') {
124-128: 🛠️ Refactor suggestionAdd error handling for customer update.
Missing error handling when updating the customer with the payment method.
- await stripe.customers.update(customer.id, { - invoice_settings: { - default_payment_method: setupIntent.payment_method.toString(), - }, - }); + try { + await stripe.customers.update(customer.id, { + invoice_settings: { + default_payment_method: typeof setupIntent.payment_method === 'string' + ? setupIntent.payment_method + : setupIntent.payment_method.toString(), + }, + }); + } catch (error) { + return ( + <Empty> + <Empty.Title>Failed to update payment method</Empty.Title> + <Empty.Description> + There was an error setting your default payment method. Please try again or contact support@unkey.dev. + </Empty.Description> + </Empty> + ); + }
🧹 Nitpick comments (5)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx (5)
61-72: Improve error message for missing session URL.The current error message for a missing session URL is unclear and fragmented.
if (!session.url) { return ( <Empty> <Empty.Title>Empty Session</Empty.Title> - <Empty.Description>The Stripe session</Empty.Description> - <Code>{session.id}</Code> - <Empty.Description> - you are trying to access does not exist. Please contact support@unkey.dev. - </Empty.Description> + <Empty.Description> + The Stripe session {session.id} does not have a valid checkout URL. + Please contact support@unkey.dev. + </Empty.Description> </Empty> ); }
93-98: Improve error message formatting for customer not found.Similar to the session URL error message, this error message is fragmented and could be more clearly formatted.
<Empty> <Empty.Title>Stripe customer not found</Empty.Title> - <Empty.Description>The Stripe customer</Empty.Description> - <Code>{session.customer as string}</Code> - <Empty.Description> - you are trying to access does not exist. Please contact support@unkey.dev. - </Empty.Description> + <Empty.Description> + The Stripe customer <Code>{session.customer as string}</Code> you are trying to access + does not exist. Please contact support@unkey.dev. + </Empty.Description> </Empty>
106-110: Improve error message formatting for setup intent.Similar to other error messages, this one is also fragmented and could be formatted more clearly.
<Empty> <Empty.Title>Stripe setup intent not found</Empty.Title> - <Empty.Description>Stripe did not return a</Empty.Description> - <Code>setup_intent</Code> - <Empty.Description>id. Please contact support@unkey.dev.</Empty.Description> + <Empty.Description> + Stripe did not return a <Code>setup_intent</Code> id. Please contact support@unkey.dev. + </Empty.Description> </Empty>
137-137: Consider prefixing unused error variable with underscore.The static analysis tool flagged the
errorvariable as unused. If this is intentional (i.e., you're catching the error but not using it), consider prefixing it with an underscore to indicate it's intentionally unused.- } catch (error) { + } catch (_error) {🧰 Tools
🪛 Biome (1.9.4)
[error] 137-137: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend error with an underscore.(lint/correctness/noUnusedVariables)
45-50: Improve environment detection and base URL determination.The current approach for determining the base URL could be enhanced for clarity and maintainability.
- const baseUrl = process.env.VERCEL - ? process.env.VERCEL_TARGET_ENV === "production" - ? "https://app.unkey.com" - : `https://${process.env.VERCEL_BRANCH_URL}` - : "http://localhost:3000"; + // Determine base URL based on environment + const baseUrl = (() => { + if (process.env.VERCEL) { + if (process.env.VERCEL_TARGET_ENV === "production") { + return "https://app.unkey.com"; + } + return `https://${process.env.VERCEL_BRANCH_URL}`; + } + return "http://localhost:3000"; + })();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx
[error] 137-137: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend error with an underscore.
(lint/correctness/noUnusedVariables)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/dashboard/app/(app)/settings/billing/stripe/checkout/page.tsx (1)
130-146: Great job adding error handling for database update.The error handling for the database update is well implemented. This is a good pattern to follow for other API calls in this file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 137-137: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend error with an underscore.(lint/correctness/noUnusedVariables)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
tools/migrate/seed_quotas.ts (1)
52-53: 🛠️ Refactor suggestionAdd proper error handling for script execution.
The call to
main()doesn't include any error handling. If the function throws an unhandled exception, it may not provide clear error information.-main(); +main().catch(error => { + console.error('Migration script failed:', error); + process.exit(1); +});
🧹 Nitpick comments (8)
apps/engineering/content/infrastructure/stripe/subscriptions.mdx (5)
13-17: Free Tier Efficiency – Consider Punctuation Consistency.
The section clearly explains the benefits for free tier users. However, in the bullet on line 16, consider adding a period at the end for consistent punctuation. For example:- - Keeps Stripe dashboard clean and focused on paying customers + - Keeps Stripe dashboard clean and focused on paying customers.🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: A period might be missing here.
Context: ...e dashboard clean and focused on paying customers 3. Frictionless Upgrades - Seam...(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
73-88: Technical Implementation is Thorough.
The "Stripe Environment Configuration" is detailed and lists necessary environment variables along with examples.
A minor suggestion for the product IDs description:- - The order does not matter. Displayed products will be sorted by their cost. + - The order of IDs is not important as products are automatically sorted by cost.🧰 Tools
🪛 LanguageTool
[style] ~84-~84: To elevate your writing, try using an alternative expression here.
Context: ...ting your Pro tier plans - The order does not matter. Displayed products will be sorted by t...(MATTERS_RELEVANT)
🪛 Gitleaks (8.21.2)
80-80: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
116-128: User Flow – Adding a Payment Method.
The steps appropriately detail the process from initiating a payment method to storing the customer ID. Consider reviewing steps 5 and 6 for potential redundancy; if the association and storage are parts of one cohesive action, combining them might improve clarity.🧰 Tools
🪛 LanguageTool
[duplication] ~126-~126: Possible typo: you repeated a word.
Context: ... associates the payment method with the customer 6. Customer ID is stored in the workspace record #...(ENGLISH_WORD_REPEAT_RULE)
167-176: Billing Portal – Minor Grammar Adjustments Recommended.
Consider these small improvements for enhanced readability:-2. System creates a portal session with the customer ID +2. System creates a portal session with the customer ID.-3. User is redirected to Stripe portal +3. User is redirected to the Stripe portal.-5. Upon completion, user is redirected back to the app +5. Upon completion, the user is redirected back to the app.These edits help with consistency and clarity.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~173-~173: You might be missing the article “the” here.
Context: ...he customer ID 3. User is redirected to Stripe portal 4. User can manage payment metho...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~175-~175: You might be missing the article “the” here.
Context: ...view invoices, etc. 5. Upon completion, user is redirected back to the app ## Legac...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~175-~175: A period might be missing here.
Context: ...pletion, user is redirected back to the app ## Legacy Plans The system handles us...(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
80-80: Verify Placeholder API Key Usage.
The exampleSTRIPE_SECRET_KEY(sk_test_51MpJhKLoBBjyJTsUAbcXYZ...) appears to be a generic placeholder. Ensure this key is clearly marked as an example and that no sensitive values are committed.🧰 Tools
🪛 Gitleaks (8.21.2)
80-80: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tools/migrate/seed_quotas.ts (3)
30-46: Eliminate duplicate code in quota creation.The two quota insertion blocks have mostly identical values with only a few differences based on the
stripeCustomerIdcondition. This can be simplified to remove duplication.- if (workspace.stripeCustomerId) { - await db.insert(schema.quotas).values({ - workspaceId: workspace.id, - team: true, - requestsPerMonth: 250_000, - logsRetentionDays: 30, - auditLogsRetentionDays: 90, - }); - } else { - await db.insert(schema.quotas).values({ - workspaceId: workspace.id, - team: false, - requestsPerMonth: 250_000, - logsRetentionDays: 7, - auditLogsRetentionDays: 30, - }); - } + const hasStripeCustomer = Boolean(workspace.stripeCustomerId); + await db.insert(schema.quotas).values({ + workspaceId: workspace.id, + team: hasStripeCustomer, + requestsPerMonth: 250_000, + logsRetentionDays: hasStripeCustomer ? 30 : 7, + auditLogsRetentionDays: hasStripeCustomer ? 90 : 30, + });
34-37: Extract hardcoded quota values to constants.The quota values (requestsPerMonth, logsRetentionDays, auditLogsRetentionDays) are hardcoded in the implementation, making future updates difficult and prone to inconsistencies.
+// Define quota constants at the top of the file +const QUOTA_DEFAULTS = { + REQUESTS_PER_MONTH: 250_000, + TEAM_LOGS_RETENTION_DAYS: 30, + TEAM_AUDIT_LOGS_RETENTION_DAYS: 90, + FREE_LOGS_RETENTION_DAYS: 7, + FREE_AUDIT_LOGS_RETENTION_DAYS: 30, +}; + async function main() { // ...existing code... // Then replace the hardcoded values with constants - requestsPerMonth: 250_000, - logsRetentionDays: 30, - auditLogsRetentionDays: 90, + requestsPerMonth: QUOTA_DEFAULTS.REQUESTS_PER_MONTH, + logsRetentionDays: QUOTA_DEFAULTS.TEAM_LOGS_RETENTION_DAYS, + auditLogsRetentionDays: QUOTA_DEFAULTS.TEAM_AUDIT_LOGS_RETENTION_DAYS, // And similarly for the free tier - requestsPerMonth: 250_000, - logsRetentionDays: 7, - auditLogsRetentionDays: 30, + requestsPerMonth: QUOTA_DEFAULTS.REQUESTS_PER_MONTH, + logsRetentionDays: QUOTA_DEFAULTS.FREE_LOGS_RETENTION_DAYS, + auditLogsRetentionDays: QUOTA_DEFAULTS.FREE_AUDIT_LOGS_RETENTION_DAYS,Also applies to: 42-45
14-20: Consider adding a transaction for batch processing.Each batch of workspace quota insertions should ideally be wrapped in a transaction to ensure data consistency if the script fails mid-execution.
do { + let transaction; + try { + transaction = db.transaction(async (tx) => { const workspaces = await db.query.workspaces.findMany({ where: (table, { gt }) => gt(table.id, cursor), with: { quota: true }, limit: 1000, orderBy: (table, { asc }) => asc(table.id), }); cursor = workspaces.at(-1)?.id ?? ""; for (const workspace of workspaces) { // Process workspace... // Use tx instead of db for insert operations } + }); + await transaction; + } catch (error) { + console.error('Transaction failed:', error); + } } while (cursor);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/engineering/content/infrastructure/stripe/subscriptions.mdx(1 hunks)tools/migrate/seed_quotas.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/infrastructure/stripe/subscriptions.mdx
[uncategorized] ~16-~16: A period might be missing here.
Context: ...e dashboard clean and focused on paying customers 3. Frictionless Upgrades - Seam...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~53-~53: Loose punctuation mark.
Context: ...plan tier (free, pro, etc.) - quota: Usage limits (primarily `requestsPerMon...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~84-~84: To elevate your writing, try using an alternative expression here.
Context: ...ting your Pro tier plans - The order does not matter. Displayed products will be sorted by t...
(MATTERS_RELEVANT)
[duplication] ~126-~126: Possible typo: you repeated a word.
Context: ... associates the payment method with the customer 6. Customer ID is stored in the workspace record #...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~173-~173: You might be missing the article “the” here.
Context: ...he customer ID 3. User is redirected to Stripe portal 4. User can manage payment metho...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~175-~175: You might be missing the article “the” here.
Context: ...view invoices, etc. 5. Upon completion, user is redirected back to the app ## Legac...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~175-~175: A period might be missing here.
Context: ...pletion, user is redirected back to the app ## Legacy Plans The system handles us...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
🪛 Gitleaks (8.21.2)
apps/engineering/content/infrastructure/stripe/subscriptions.mdx
80-80: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
apps/engineering/content/infrastructure/stripe/subscriptions.mdx (17)
1-4: Front Matter Formatting Looks Good.
The YAML front matter includes a clear title and description, and the document delimiters are correctly placed.
6-7: Clear Introduction.
The opening statement effectively sets the context for the billing integration objectives.
8-12: Well-Defined Calendar Month Alignment.
The bullet points neatly enumerate how aligning billing cycles with calendar months benefits both predictability and reporting.
18-22: Frictionless Upgrades Section is Clear.
The points succinctly detail the seamless process from free to paid subscriptions along with trial and usage visibility.
25-28: System Overview is Well-Structured.
The overview concisely explains the key aspects of subscription management, including legacy support.
29-45: Informative User Flow Diagram.
The ASCII diagram effectively illustrates the complete user journey for subscription management, making the flow easy to visualize.
47-54: Key Components Section Clarity.
The components (e.g., Stripe customer/subscription IDs, tier, quota) are well enumerated and easy to understand.🧰 Tools
🪛 LanguageTool
[uncategorized] ~53-~53: Loose punctuation mark.
Context: ...plan tier (free, pro, etc.) -quota: Usage limits (primarily `requestsPerMon...(UNLIKELY_OPENING_PUNCTUATION)
55-59: Tiered Products Section is Clear.
The explanation of the tiered products is succinct. For even greater clarity, you might consider briefly elaborating on how the upgrade/downgrade paths are implemented.
60-65: Stripe Integration Section is Concise.
The listed items effectively capture key aspects of Stripe integration including payment setup, subscription management, and billing operations.
66-70: Usage Tracking Details are Adequate.
The description clearly outlines how usage is monitored and presented, enhancing transparency.
89-108: Workspace Database Schema is Well Documented.
The TypeScript snippet clearly defines the workspace schema and highlights fields that may be null or legacy.
109-115: Trial Management Section is Clear.
The bullet points effectively communicate the 14-day trial process along with payment method collection and transition details.
129-140: User Flow – Starting a Subscription.
The sequential steps clearly address both cases (with and without an existing customer ID) and detail subsequent actions like quota updates and audit logging.
141-153: User Flow – Changing Plans.
The description comprehensively covers the plan change process, including subscription updates, proration, and UI feedback.
154-166: User Flow – Cancelling a Subscription.
This section thoroughly outlines the cancellation steps, including backend updates and UI adjustments.
177-185: Legacy Plans Section is Clear and Concise.
The section effectively describes legacy handling and the migration prompt, ensuring backward compatibility.
186-201: Implementation Notes Provide Good Technical Insight.
The details on product metadata and usage calculation are comprehensive and offer good guidance on how the system determines quotas and limits.
2 participants
Summary by CodeRabbit
New Features- Enhanced billing experience with a revamped interface that clearly distinguishes between legacy and current subscription models, offering updated usage metrics, trial prompts, and support options.
Revamped pricing page featuring an interactive dropdown for selecting updated pricing tiers with clearer details on request limits and plan benefits.
Improved subscription management with streamlined actions for portal access, trial initiation, and subscription updates.
New select component introduced for enhanced user interaction in pricing selections.
New properties added to workspaces for better configuration management, including organization ID and tier.
New quotas management schema implemented to track workspace usage limits and retention policies.
Refactor- Removed legacy plan change and pricing calculator components to deliver a more cohesive and simplified user experience.
Chores- Added a new dependency for Radix UI's select component to enhance UI capabilities.
Summary by CodeRabbit
New Features
Billing settings now feature a streamlined interface that clearly distinguishes legacy and new subscription plans, with enhanced usage metrics and an improved “Change Plan” dialog for smoother subscription modifications.
The pricing page has been revamped with an interactive dropdown and updated tier details, offering clearer, more predictable pricing information.
Refactor
Summary by CodeRabbit
New Features
Revamped billing interface now distinguishes legacy from subscription plans with clearer usage metrics and an improved visual layout.
Interactive pricing page introduces an intuitive dropdown to select tiers with real-time cost details.
Streamlined subscription management provides smoother plan changes and clearer error feedback.
New components for managing billing settings, including
Client,ChangePlanButton,Confirm, andShell, enhance user interaction.Added
Usagecomponent for displaying usage statistics visually.Introduced a custom select component for enhanced dropdown functionality.
Backend Enhancements
Enhanced workspace configuration for more accurate plan handling and quota management.
New subscription management procedures for creating, updating, and canceling subscriptions via Stripe.
Documentation
Summary by CodeRabbit
New Features
Refactor
Chores