Payment tests, account status, smartRoutes#828
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude finished @BilalG1's task —— View job PR Review: Payment System ImplementationThis is a comprehensive payment system implementation with significant changes across 52 files. Here's my detailed analysis focusing on code quality, security, performance, and test coverage: 🔴 Critical IssuesDatabase Schema & Migration Safety
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
♻️ Duplicate comments (10)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx (1)
13-15:paramsshouldn’t be aPromiseNext.js supplies
paramssynchronously; typing it asPromise<Params>forces an unnecessaryawaitand risks runtime misuse. Revert to{ params: Params }.apps/e2e/tests/backend/endpoints/api/v1/payments/items.test.ts (4)
5-12: Avoid directJSON.stringifywhen overriding config
This was flagged in earlier reviews – usestringifyJsonfromstack-shared/utils/jsonfor consistent escaping and error handling.
55-58: Construct URLs withnew URL()instead of template literalsString interpolation can silently produce invalid URLs (double slashes, missing encoding).
Use the existingniceBackendFetch(new URL(...))helper instead.
89-91: Same URL-construction issue as above.
125-127: Same URL-construction issue as above.apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts (1)
60-65: Emptyclient_secretweakens the testAsserting that Stripe returns an empty string allows the happy-path tests to pass even when no session was created. Consider asserting
expect.any(String)or a non-empty pattern so the test fails whenstripe-mock(or prod) returns an unexpected value.Also applies to: 82-85, 133-136
apps/e2e/tests/backend/backend-helpers.ts (1)
1110-1114: Use sharedstringifyJsonhelper instead ofJSON.stringifyPast reviews asked to standardise JSON serialisation for config overrides; please switch to
stringifyJson(config)for consistency.apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (1)
34-46: Use snake_case in JSON payloads for API-visible fields
serverOnly→server_only,includedItems→included_itemsto stay consistent with REST naming conventions previously agreed on.Also applies to: 84-96, 132-142
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page-client.tsx (1)
334-336: Silent discard still present – surface feedback to userThe
updatePriceIdbranch still exits silently on duplicates. Prior review already flagged this; please show a toast or error so users know why the edit failed.apps/backend/src/lib/payments.tsx (1)
42-47: Don’t overwrite per-priceserverOnlyflagEvery mapped price forces
serverOnly: true, discarding the originalserver_onlyvalue from the inline offer. Preserve the caller’s intent:- serverOnly: true, + serverOnly: value.server_only ?? inlineOffer.server_only,
🧹 Nitpick comments (26)
apps/dashboard/.env (1)
7-7: Follow dotenv best practices: move before STACK_SECRET_SERVER_KEY and quote valueAddressing the linter hints and avoiding inline-comments-as-values pitfalls:
- Place NEXT_PUBLIC_STACK_STRIPE_PUBLISHABLE_KEY before STACK_SECRET_SERVER_KEY.
- Quote the value and move the comment to a separate line.
Apply:
-STACK_SECRET_SERVER_KEY=# enter your Stack secret client key here. For local development, do the same as above -NEXT_PUBLIC_STACK_EXTRA_REQUEST_HEADERS=# a list of extra request headers to add to all Stack Auth API requests, as a JSON record -NEXT_PUBLIC_STACK_STRIPE_PUBLISHABLE_KEY=# enter your Stripe publishable key here +# enter your Stripe publishable key here +NEXT_PUBLIC_STACK_STRIPE_PUBLISHABLE_KEY="" +STACK_SECRET_SERVER_KEY=# enter your Stack secret client key here. For local development, do the same as above +NEXT_PUBLIC_STACK_EXTRA_REQUEST_HEADERS=# a list of extra request headers to add to all Stack Auth API requests, as a JSON recordNote: Inline comments after = can be parsed as part of the value by some dotenv parsers—use a separate comment line as shown.
apps/dashboard/src/components/smart-form.tsx (1)
116-118: Good addition for number support; consider coercing to number to avoid string values in RHF.RHF often keeps inputs as strings; coercing prevents subtle schema/type issues.
Suggestion (in NumberField implementation, outside this file):
// apps/dashboard/src/components/form-fields.tsx <Input {...field} value={field.value ?? ""} onChange={(e) => field.onChange(e.target.value === "" ? undefined : Number(e.target.value))} inputMode="numeric" type="number" min={props.min} max={props.max} />docker/dependencies/docker.compose.yaml (1)
170-170: Trim trailing spaces.Line 170 has trailing whitespace. Clean it to satisfy YAML linters.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx (1)
367-369: Avoid re-parsing env JSON on every item render
devFeaturesEnabledForProject(projectId)parses the env var each call; here it’s called per item during render. Compute it once per render instead:
- Compute
const devEnabled = useMemo(() => devFeaturesEnabledForProject(projectId), [projectId])before themap.- Use
if (item.requiresDevFeatureFlag && !devEnabled) return null;.This reduces repeated parsing and improves clarity.
apps/dashboard/src/components/payments/stripe-theme-variables.ts (1)
58-60: Narrow the theme type and validate Stripe appearance variables
- In apps/dashboard/src/components/payments/stripe-theme-variables.ts (line 58), change the signature from
to(theme: string | undefined)(theme?: 'light' | 'dark')- Cross-check every key in darkAppearanceVariables and lightAppearanceVariables (e.g. colorSecondaryText, buttonSecondaryColorBackground, actionSecondaryTextDecorationColor, etc.) against the Stripe Elements Appearance Variables API (https://stripe.com/docs/stripe-js/elements/options#appearance-variables). Remove or rename any properties Stripe doesn’t support so your custom theme is fully honored.
apps/dashboard/src/app/(main)/purchase/[code]/page-client.tsx (1)
31-36: Pluralisation helper may double-pluralIf
interval[1]is already plural (“months”),"${interval[0]} ${interval[1]}s"becomes “3 monthss”. Consider a small map or inflection util instead of blindly appending “s”.apps/backend/src/app/api/latest/internal/payments/stripe-widgets/account-session/route.ts (1)
30-42: Hard-coded features may violate least-privilegeEnabling
refund_management,dispute_management, andcapture_paymentsfor every widget session might exceed what the UI needs. Consider passing requested components from the caller or scoping to the minimum required to reduce risk.apps/dashboard/src/lib/utils.tsx (1)
20-29: Cache the parsed project-ID allow-list
parseJson(...)runs on every call, but the env var is static per process. Parse once at module top and reuse to avoid repeated JSON parsing and object allocations.apps/dashboard/src/components/data-table/payment-offer-table.tsx (1)
31-35: Render “Free Trial” more clearly
freeTrial?.join(" ")collapses the tuple without context (e.g., “30 days”). Consider formatting as “30 days” / “1 month” for readability.apps/dashboard/src/components/payments/stripe-elements-provider.tsx (1)
25-30: Memo deps could include the public key
loadStripewill be re-invoked whenstripeAccountIdchanges but not whenstripePublicKeychanges (e.g. in tests with a different.env).
Consider addingstripePublicKeyto the dependency array to keep the memo rule-of-thumb “list everything you read”.apps/dashboard/src/app/(main)/purchase/return/page.tsx (1)
14-21: UnnecessaryawaitAfter fixing the prop type you can simplify:
-export default async function Page({ searchParams }: Props) { - const params = await searchParams; +export default function Page({ searchParams: params }: Props) {packages/template/src/lib/stack-app/customers/index.ts (2)
32-33:nonNegativeQuantityshould be derived, not storedStoring both
quantityandnonNegativeQuantityrisks them drifting out of sync.
Prefer a getter so the value is always consistent:get nonNegativeQuantity(): number { return Math.max(0, this.quantity); }
34-47: Return a value from mutator methods
increaseQuantity,decreaseQuantity, andtryDecreaseQuantitymutate state but returnvoid.
Returning the new quantity (or a boolean for success) improves ergonomics and makes chaining easier.- increaseQuantity(amount: number): void, + increaseQuantity(amount: number): number, - decreaseQuantity(amount: number): void, + decreaseQuantity(amount: number): number,apps/e2e/tests/backend/endpoints/api/v1/payments/items.test.ts (1)
5-12: Factor outupdateConfighelper
updateConfigis re-declared in multiple test files. Extract it once inbackend-helpers.tsto cut duplication and keep tests DRY.Also applies to: 39-147
apps/backend/src/app/api/latest/internal/payments/setup/route.ts (1)
30-44: Make Stripe account creation idempotentConcurrent calls when
stripeAccountIdis unset can create multiple connected accounts.
Use an idempotency key (e.g. tenancy ID) or lock at the DB layer to make this operation safe in the face of retries.apps/backend/src/app/api/latest/payments/purchases/validate-code/route.ts (1)
33-37: Strip unsupported keys fromoffer.prices
valuemay contain properties other than currency codes (e.g. legacy fields).
AfterfilterUndefinedconsiderpickBy(value, (_, k) => SUPPORTED_CURRENCIES.some(c => c.code === k))to ensure no extraneous data leaks to the client.apps/dashboard/src/components/data-table/payment-item-table.tsx (1)
37-42: Render repeat interval more readably
join(" ")produces “1 month” as “1 month” but “3,month” would become “3 month”.
Consider formatting as${count} ${unit}or usingjoin(" ")only when the value is[count, unit]; otherwise show"Never".packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (2)
168-171: Cache refresh looks correct but consider refreshing dependent caches tooRefreshing
_configOverridesCacheafterupdateConfigwill give consumers the new values, butgetProject()/useProject()still serve the old config snapshot cached in_adminProjectCache.
If callers depend onproject.config.*immediately after an override, consider refreshing_adminProjectCacheas well (or invalidating both in one helper) to avoid subtle stale-data bugs.
501-514: Expose raw Stripe objects carefullyThe three new payment helpers correctly delegate to the interface, but:
createStripeWidgetAccountSession()returns the raw Stripe‐secret field in snake_case. For consumer ergonomics keep external API camelCase (clientSecret) and keep the raw response internal.createPurchaseUrl()returns a string yet does no basic URL validation; if the backend ever changes shape this will silently returnundefined. A trivialtry { new URL(url) }guard (orz.string().url()zod check) would fail fast.Both are small polish items but will save debugging time.
apps/backend/src/app/api/latest/payments/purchases/create-purchase-url/route.ts (1)
40-44: Race-condition & uniqueness on Stripe customer search
customers.searchcan return multiple hits if duplicate metadata slipped in.
Currently the code silently takesdata[0]. To guarantee idempotency you may:if (stripeCustomerSearch.data.length > 1) { throw new StackAssertionError("Multiple Stripe customers share the same customerId metadata"); }(or enforce uniqueness with
metadata[‘customerId’]: ‘${id}’ AND email:\'...').apps/dashboard/src/components/payments/checkout.tsx (2)
31-39: Missing loading / double-click guard
handleSubmitis async but the button stays enabled; double-clicking will fire two parallel submissions and may create duplicate Stripe PaymentIntents.
Add aprocessingstate to disable the button untilconfirmPaymentresolves.
45-51: Unnecessary type-cast masks real Stripe error objectsCasting the result of
confirmPaymentto{ error?: StripeError }discards other fields; TypeScript already exposesPaymentIntentResult.
Prefer:const { error } = await stripe.confirmPayment({ … });and rely on the provided type.
packages/stack-shared/src/interface/admin-interface.ts (1)
471-516: Consistency & validation improvements for new payment endpointsThe three new helpers look good; a couple of minor suggestions:
- Return camelCase (
clientSecret) increateStripeWidgetAccountSession()for consistency with the rest of the interface.createPurchaseUrl()swallows non-200 responses – a 4xx from the backend will produceresult.url === undefinedand callers will get"undefined". Consider checkingresponse.okand throwing a typed error early.These tweaks align the interface with existing patterns elsewhere in the file.
apps/backend/prisma/schema.prisma (1)
712-726: Unify enum value casingExisting enums use ALL_CAPS (
ONE_TIME_PASSWORD). Newly addedCustomerType/SubscriptionStatusintroduceUSER/activeetc. Mixed casing hampers readability and code-search. Consider either:a)
ACTIVE | TRIALING | CANCELED …
b) keep lower-camel everywhere.Pick one style and apply consistently.
apps/backend/src/lib/stripe.tsx (1)
17-28: Avoid re-creating Stripe clients on every call
getStackStripeandgetStripeForAccountinstantiate a newStripeobject each time. This is thread-safe but wasteful; cache per-process singletons instead.-export const getStackStripe = () => new Stripe(stripeSecretKey, stripeConfig); +let stackStripe: Stripe | undefined; +export const getStackStripe = () => stackStripe ??= new Stripe(stripeSecretKey, stripeConfig);Apply a similar memoisation for account-scoped clients keyed by
accountId.Also applies to: 30-32
packages/stack-shared/src/config/schema.ts (1)
116-121: Validate Stripe account prerequisites
stripeAccountSetupCompletecan betrueeven whenstripeAccountIdis missing.
Add a.when("stripeAccountSetupComplete", …)rule so an ID becomes mandatory once the flag is set.
📜 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 (51)
apps/backend/.env(1 hunks)apps/backend/.env.development(1 hunks)apps/backend/package.json(1 hunks)apps/backend/prisma/migrations/20250805195319_subscriptions/migration.sql(1 hunks)apps/backend/prisma/schema.prisma(2 hunks)apps/backend/src/app/api/latest/integrations/stripe/webhooks/route.tsx(1 hunks)apps/backend/src/app/api/latest/internal/payments/setup/route.ts(1 hunks)apps/backend/src/app/api/latest/internal/payments/stripe-widgets/account-session/route.ts(1 hunks)apps/backend/src/app/api/latest/payments/items/[customer_id]/[item_id]/route.ts(1 hunks)apps/backend/src/app/api/latest/payments/purchases/create-purchase-url/route.ts(1 hunks)apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsx(1 hunks)apps/backend/src/app/api/latest/payments/purchases/validate-code/route.ts(1 hunks)apps/backend/src/app/api/latest/payments/purchases/verification-code-handler.tsx(1 hunks)apps/backend/src/lib/payments.tsx(1 hunks)apps/backend/src/lib/stripe.tsx(1 hunks)apps/backend/src/route-handlers/smart-request.tsx(2 hunks)apps/backend/src/route-handlers/verification-code-handler.tsx(3 hunks)apps/dashboard/.env(1 hunks)apps/dashboard/package.json(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx(2 hunks)apps/dashboard/src/app/(main)/purchase/[code]/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/purchase/[code]/page.tsx(1 hunks)apps/dashboard/src/app/(main)/purchase/return/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/purchase/return/page.tsx(1 hunks)apps/dashboard/src/components/data-table/payment-item-table.tsx(1 hunks)apps/dashboard/src/components/data-table/payment-offer-table.tsx(1 hunks)apps/dashboard/src/components/form-fields.tsx(2 hunks)apps/dashboard/src/components/payments/checkout.tsx(1 hunks)apps/dashboard/src/components/payments/stripe-connect-provider.tsx(1 hunks)apps/dashboard/src/components/payments/stripe-elements-provider.tsx(1 hunks)apps/dashboard/src/components/payments/stripe-theme-variables.ts(1 hunks)apps/dashboard/src/components/smart-form.tsx(2 hunks)apps/dashboard/src/lib/env.tsx(2 hunks)apps/dashboard/src/lib/utils.tsx(2 hunks)apps/e2e/tests/backend/backend-helpers.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/items.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/validate-code.test.ts(1 hunks)docker/dependencies/docker.compose.yaml(1 hunks)packages/stack-shared/src/config/schema.ts(7 hunks)packages/stack-shared/src/interface/admin-interface.ts(1 hunks)packages/stack-shared/src/interface/crud/projects.ts(1 hunks)packages/stack-shared/src/known-errors.tsx(2 hunks)packages/stack-shared/src/schema-fields.ts(4 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(2 hunks)packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts(1 hunks)packages/template/src/lib/stack-app/customers/index.ts(1 hunks)packages/template/src/lib/stack-app/project-configs/index.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/template/src/lib/stack-app/project-configs/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/.env.development
📄 CodeRabbit Inference Engine (CLAUDE.md)
Environment variables are pre-configured in
.env.developmentfiles
Files:
apps/backend/.env.development
apps/backend/src/app/api/latest/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/backend/src/app/api/latest/**/*: Main API routes are located in /apps/backend/src/app/api/latest
The project uses a custom route handler system in the backend for consistent API responses
Files:
apps/backend/src/app/api/latest/payments/purchases/verification-code-handler.tsxapps/backend/src/app/api/latest/internal/payments/stripe-widgets/account-session/route.tsapps/backend/src/app/api/latest/integrations/stripe/webhooks/route.tsxapps/backend/src/app/api/latest/payments/items/[customer_id]/[item_id]/route.tsapps/backend/src/app/api/latest/internal/payments/setup/route.tsapps/backend/src/app/api/latest/payments/purchases/validate-code/route.tsapps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsxapps/backend/src/app/api/latest/payments/purchases/create-purchase-url/route.ts
apps/backend/prisma/schema.prisma
📄 CodeRabbit Inference Engine (CLAUDE.md)
Database models use Prisma
Files:
apps/backend/prisma/schema.prisma
🧠 Learnings (4)
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to apps/backend/prisma/schema.prisma : Database models use Prisma
Applied to files:
packages/stack-shared/src/interface/crud/projects.tsapps/backend/src/app/api/latest/payments/purchases/validate-code/route.tsapps/backend/prisma/migrations/20250805195319_subscriptions/migration.sqlapps/backend/prisma/schema.prismapackages/stack-shared/src/schema-fields.ts
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to **/.env.development : Environment variables are pre-configured in `.env.development` files
Applied to files:
apps/dashboard/src/lib/env.tsxapps/backend/.env.developmentapps/backend/.envapps/dashboard/.env
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to apps/backend/src/app/api/latest/**/* : The project uses a custom route handler system in the backend for consistent API responses
Applied to files:
apps/backend/src/app/api/latest/internal/payments/stripe-widgets/account-session/route.tsapps/backend/src/app/api/latest/integrations/stripe/webhooks/route.tsxapps/backend/src/app/api/latest/payments/items/[customer_id]/[item_id]/route.tsapps/backend/src/app/api/latest/internal/payments/setup/route.tsapps/backend/src/app/api/latest/payments/purchases/validate-code/route.tsapps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsxapps/backend/src/app/api/latest/payments/purchases/create-purchase-url/route.tsapps/backend/src/route-handlers/verification-code-handler.tsx
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to apps/backend/src/app/api/latest/**/* : Main API routes are located in /apps/backend/src/app/api/latest
Applied to files:
apps/backend/src/app/api/latest/integrations/stripe/webhooks/route.tsxapps/backend/src/app/api/latest/payments/items/[customer_id]/[item_id]/route.ts
🧬 Code Graph Analysis (7)
apps/dashboard/src/components/form-fields.tsx (2)
packages/stack-ui/src/components/ui/form.tsx (4)
FormField(171-171)FormItem(171-171)FormControl(170-170)FormMessage(172-172)packages/stack-ui/src/components/ui/input.tsx (1)
Input(10-41)
apps/dashboard/src/components/smart-form.tsx (1)
apps/dashboard/src/components/form-fields.tsx (1)
NumberField(305-342)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx (1)
apps/dashboard/src/lib/utils.tsx (1)
devFeaturesEnabledForProject(20-29)
apps/e2e/tests/backend/endpoints/api/v1/payments/items.test.ts (2)
apps/e2e/tests/backend/backend-helpers.ts (2)
updateConfig(1109-1117)niceBackendFetch(107-165)apps/e2e/tests/helpers.ts (1)
it(10-10)
apps/dashboard/src/lib/utils.tsx (2)
packages/stack-shared/src/utils/json.tsx (1)
parseJson(72-74)apps/dashboard/src/lib/env.tsx (1)
getPublicEnvVar(49-59)
packages/stack-shared/src/config/schema.ts (3)
packages/stack-shared/src/schema-fields.ts (7)
yupObject(245-249)yupString(185-188)yupBoolean(193-196)yupRecord(281-320)userSpecifiedIdSchema(373-373)offerSchema(506-524)yupNumber(189-192)packages/stack-shared/src/utils/objects.tsx (1)
typedFromEntries(281-283)packages/stack-shared/src/utils/currencies.tsx (1)
SUPPORTED_CURRENCIES(11-47)
packages/stack-shared/src/schema-fields.ts (2)
packages/stack-shared/src/utils/currencies.tsx (1)
SUPPORTED_CURRENCIES(11-47)packages/stack-shared/src/utils/objects.tsx (1)
typedFromEntries(281-283)
🪛 Gitleaks (8.27.2)
apps/backend/.env.development
51-51: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
docker/dependencies/docker.compose.yaml
176-176: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.
(stripe-access-token)
🪛 dotenv-linter (3.3.0)
apps/backend/.env
[warning] 69-69: [ValueWithoutQuotes] This value needs to be surrounded in quotes
[warning] 70-70: [ValueWithoutQuotes] This value needs to be surrounded in quotes
apps/dashboard/.env
[warning] 7-7: [UnorderedKey] The NEXT_PUBLIC_STACK_STRIPE_PUBLISHABLE_KEY key should go before the STACK_SECRET_SERVER_KEY key
[warning] 7-7: [ValueWithoutQuotes] This value needs to be surrounded in quotes
🪛 YAMLlint (1.37.1)
docker/dependencies/docker.compose.yaml
[error] 170-170: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: claude-review
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: setup-tests
- GitHub Check: Security Check
🔇 Additional comments (13)
apps/backend/src/route-handlers/smart-request.tsx (2)
42-42: Raw body attachment looks goodThe addition of bodyBuffer to SmartRequest is appropriate for webhook signature verification and other raw-body use cases.
317-317: No outdatedcreateSmartRequestcalls detected; Stripe webhook body handled correctly
- All
createSmartRequestcall sites now pass three arguments, so there are no two-arg invocations remaining.- The Stripe webhook handler in
apps/backend/src/app/api/latest/integrations/stripe/webhooks/route.tsxdecodesfullReq.bodyBufferto a string (textBody) before callingstripe.webhooks.constructEvent, satisfying the requirement for a string or Buffer.- There is no logging of the raw
bodyBuffer.– Everything looks good here.
apps/dashboard/src/lib/env.tsx (1)
21-21: Env var wiring for Stripe publishable key is correctAdded to both
_inlineEnvVarsand_postBuildEnvVarswith shape parity viasatisfies. Confirmed downstream handling of missing values:
- apps/dashboard/src/components/payments/stripe-elements-provider.tsx
• Throws viathrowErr("NEXT_PUBLIC_STACK_STRIPE_PUBLISHABLE_KEY is missing!")- apps/dashboard/src/components/payments/stripe-connect-provider.tsx
• Throws viathrowErr("No Stripe publishable key found")- apps/dashboard/src/app/(main)/purchase/return/page-client.tsx
• Falls back to an empty string ("")All usages guard against undefined at runtime. LGTM.
apps/dashboard/package.json (1)
34-37: Stripe peer dependencies verified as compatible with React 19 / Next.js 15All peer dependencies were checked and align with React 19 support:
- @stripe/connect-js@3.3.27: no peerDependencies
- @stripe/react-connect-js@3.3.24:
• react >=16.8.0
• react-dom >=16.8.0
• @stripe/connect-js >=3.3.24- @stripe/react-stripe-js@3.8.1:
• react >=16.8.0 <20.0.0
• react-dom >=16.8.0 <20.0.0
• @stripe/stripe-js >=1.44.1 <8.0.0- @stripe/stripe-js@7.7.0: no peerDependencies
React 19 falls within all declared ranges, and Next.js versions aren’t constrained by these packages. No runtime warnings or SSR issues are expected.
apps/dashboard/src/components/smart-form.tsx (1)
10-10: LGTM: NumberField import added.Import is correct and matches the new number field rendering below.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx (1)
9-9: Centralized dev-feature flag check — good changeImporting and using
devFeaturesEnabledForProjectimproves consistency across the app.apps/dashboard/src/components/payments/stripe-theme-variables.ts (1)
1-61: Stripe appearance theme variables look coherentDark/light palettes are consistent with the UI palette and return a stable object per theme. Good separation of concerns.
apps/dashboard/src/components/form-fields.tsx (1)
248-249: Minor JSX cleanup — ok
Self-closingSelectValueformatting looks fine.apps/dashboard/src/app/(main)/purchase/[code]/page-client.tsx (1)
15-17:baseUrlconstruction can drop path segments
new URL("/api/v1", apiUrl)discards any existing path onapiUrl; if the env var already ends with/api, the result is wrong. Prefernew URL("payments/", apiUrl)or explicit string concat.apps/dashboard/src/components/payments/stripe-elements-provider.tsx (1)
41-44: Guard against undefined theme
resolvedThemecan beundefinedon first render; ensureappearanceVariablesForThemecopes with that or fall back to a default to avoid a run-time throw on initial paint.apps/e2e/tests/backend/endpoints/api/v1/payments/validate-code.test.ts (1)
29-36: Good coverage for happy-path and error-pathBoth invalid-code and valid-code scenarios are exercised, ensuring regression safety for the purchase flow.
apps/backend/src/app/api/latest/internal/payments/setup/route.ts (1)
28-28: Ensure dashboard base URL env var is set
NEXT_PUBLIC_STACK_DASHBOARD_URLmust exist and be an absolute https URL.
IfgetEnvVariablethrows, callers get a 500. Consider validating once at boot and failing fast.packages/stack-shared/src/known-errors.tsx (1)
1417-1486: Good addition – covers the new payment validation cases
apps/backend/prisma/migrations/20250805195319_subscriptions/migration.sql
Show resolved
Hide resolved
|
Claude finished @BilalG1's task —— View job I'll review this comprehensive payment system PR and provide detailed feedback on code quality, security, performance, and testing. Review Tasks
🔍 Comprehensive PR Review: Payment System ImplementationThis PR introduces a sophisticated payment system with Stripe integration across 53 files. Here's my detailed analysis: 🔴 Critical Security & Reliability Issues1. Database Migration Safety
|
|
Claude finished @BilalG1's task —— View job I'll review this comprehensive payment system PR and provide detailed feedback on code quality, security, performance, and testing. Review Tasks
🔴 Critical IssuesDatabase Schema & Migration Safety
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/e2e/tests/backend/endpoints/api/v1/auth/password/send-reset-code.test.ts (1)
23-40: Harden assertions: guard find() results and reduce brittleness of exact count
- Use the existing mailbox variable for consistency.
- Add explicit existence checks before snapshotting to avoid confusing failures if a subject is missing.
- Consider asserting per-subject counts instead of
messages.length === 2to avoid brittleness if additional emails are sent in this flow.Minimal improvement (keep length=2, add existence checks and reuse mailbox):
- const messages = await backendContext.value.mailbox.fetchMessages({ noBody: true }); - expect(messages.length).toBe(2); - expect(messages.find(m => m.subject.includes("Verify your email at Stack Dashboard"))).toMatchInlineSnapshot(` + const messages = await mailbox.fetchMessages({ noBody: true }); + expect(messages.length).toBe(2); + const verifyMsg = messages.find(m => m.subject.includes("Verify your email at Stack Dashboard")); + const resetMsg = messages.find(m => m.subject.includes("Reset your password at Stack Dashboard")); + expect(verifyMsg).toBeDefined(); + expect(resetMsg).toBeDefined(); + expect(verifyMsg!).toMatchInlineSnapshot(` MailboxMessage { "from": "Stack Dashboard <noreply@example.com>", "subject": "Verify your email at Stack Dashboard", "to": ["<default-mailbox--<stripped UUID>@stack-generated.example.com>"], <some fields may have been hidden>, } `); - expect(messages.find(m => m.subject.includes("Reset your password at Stack Dashboard"))).toMatchInlineSnapshot(` + expect(resetMsg!).toMatchInlineSnapshot(` MailboxMessage { "from": "Stack Dashboard <noreply@example.com>", "subject": "Reset your password at Stack Dashboard", "to": ["<default-mailbox--<stripped UUID>@stack-generated.example.com>"], <some fields may have been hidden>, } `);More robust alternative (avoid exact count; assert exactly one per subject):
- const messages = await backendContext.value.mailbox.fetchMessages({ noBody: true }); - expect(messages.length).toBe(2); - expect(messages.find(m => m.subject.includes("Verify your email at Stack Dashboard"))).toMatchInlineSnapshot(` + const messages = await mailbox.fetchMessages({ noBody: true }); + const verifyMessages = messages.filter(m => m.subject.includes("Verify your email at Stack Dashboard")); + const resetMessages = messages.filter(m => m.subject.includes("Reset your password at Stack Dashboard")); + expect(verifyMessages).toHaveLength(1); + expect(resetMessages).toHaveLength(1); + expect(verifyMessages[0]).toMatchInlineSnapshot(` MailboxMessage { "from": "Stack Dashboard <noreply@example.com>", "subject": "Verify your email at Stack Dashboard", "to": ["<default-mailbox--<stripped UUID>@stack-generated.example.com>"], <some fields may have been hidden>, } `); - expect(messages.find(m => m.subject.includes("Reset your password at Stack Dashboard"))).toMatchInlineSnapshot(` + expect(resetMessages[0]).toMatchInlineSnapshot(` MailboxMessage { "from": "Stack Dashboard <noreply@example.com>", "subject": "Reset your password at Stack Dashboard", "to": ["<default-mailbox--<stripped UUID>@stack-generated.example.com>"], <some fields may have been hidden>, } `);Optional: if the mailbox backend is eventually consistent, consider polling until both messages arrive (or a helper like waitForMessages) to avoid flakes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/tests/backend/endpoints/api/v1/auth/password/send-reset-code.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/e2e/tests/backend/endpoints/api/v1/auth/password/send-reset-code.test.ts (1)
apps/e2e/tests/backend/backend-helpers.ts (1)
backendContext(34-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: setup-tests
- GitHub Check: Security Check
🔇 Additional comments (1)
apps/e2e/tests/backend/endpoints/api/v1/auth/password/send-reset-code.test.ts (1)
23-40: Good move to per-message assertions over array snapshotsAsserting subjects individually is a solid improvement over snapshotting the whole array. This reduces flakiness from non-deterministic ordering and hidden fields.
|
Claude finished @BilalG1's task —— View job I'll review this comprehensive payment system PR and provide detailed feedback on code quality, security, performance, and testing. Review Tasks
🔍 Comprehensive PR Review: Payment System ImplementationThis PR introduces a sophisticated payment system with Stripe integration across 53 files. Here's my detailed analysis: 🔴 Critical Security & Reliability Issues1. Database Migration Safety
|
2 participants
Important
Introduce comprehensive payment and subscription management with Stripe integration, including new models, API endpoints, UI components, and extensive tests.
apps/backend/src/lib/stripe.tsxandapps/backend/src/app/api/latest/integrations/stripe/webhooks/route.tsx.apps/backend/src/app/api/latest/payments.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments.Subscriptionmodel inprisma/schema.prismaandprisma/migrations/20250805195319_subscriptions/migration.sql.apps/e2e/tests/backend/endpoints/api/v1/payments..env.developmentanddocker.compose.yamlfor Stripe.known-errors.tsx.This description was created by
for 972c248. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Documentation