Add support for stripeMode in stripeIntegrationSettingsSchema#3679
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHandlers and utilities now derive stripeMode from each workspace’s installed Stripe integration settings, validate it, create per-request Stripe clients with that mode, thread stripeMode into Stripe-related calls, and early-return when no Stripe installation exists. Changes
Sequence Diagram(s)sequenceDiagram
participant Server as "Server / Handler"
participant Prisma as "Prisma DB"
participant Parser as "stripeIntegrationSettingsSchema"
participant StripeApp as "stripeAppClient"
participant StripeAPI as "Stripe API"
Server->>Prisma: find workspace / installedIntegration (STRIPE_INTEGRATION_ID)
Prisma-->>Server: installedIntegration.settings (or none)
alt installedIntegration exists
Server->>Parser: parse(settings) -> stripeMode
Parser-->>Server: stripeMode
Server->>StripeApp: instantiate client with mode=rgba(0,128,255,0.5)
StripeApp-->>Server: stripe client
Server->>StripeAPI: create/disable/search (with stripeConnectId, stripeMode)
StripeAPI-->>Server: result
else no installation
Server-->>Server: early-return / log
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/lib/actions/partners/create-discount.ts (1)
65-88: Consider centralizing Stripe integration resolution.This exact query + parse + error-handling block is now copy-pasted across the discount-code route, this action, and the customer-search route. A shared
getStripeIntegrationSettingsOrThrow(workspaceId)helper would keep the canonical-row selection and legacy-settings fallback in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-discount.ts` around lines 65 - 88, The code duplicates the Stripe integration lookup, validation, and parsing (installedStripeIntegration query, stripeIntegrationSettingsSchema.parse, and stripeAppClient instantiation); extract this into a shared helper getStripeIntegrationSettingsOrThrow(workspaceId) that encapsulates the prisma.installedIntegration.findFirst selection, legacy-settings fallback, throws the same STRIPE_CONNECTION_REQUIRED error when missing, and returns the parsed settings so callers (e.g., create-discount action, discount-code route, customer-search) can call getStripeIntegrationSettingsOrThrow(workspace.id) and then pass the returned settings to stripeAppClient({ mode: settings.stripeMode }).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/lib/actions/partners/create-discount.ts`:
- Around line 65-88: The code duplicates the Stripe integration lookup,
validation, and parsing (installedStripeIntegration query,
stripeIntegrationSettingsSchema.parse, and stripeAppClient instantiation);
extract this into a shared helper
getStripeIntegrationSettingsOrThrow(workspaceId) that encapsulates the
prisma.installedIntegration.findFirst selection, legacy-settings fallback,
throws the same STRIPE_CONNECTION_REQUIRED error when missing, and returns the
parsed settings so callers (e.g., create-discount action, discount-code route,
customer-search) can call getStripeIntegrationSettingsOrThrow(workspace.id) and
then pass the returned settings to stripeAppClient({ mode: settings.stripeMode
}).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08e06b0f-926a-44c3-ac35-3d97a482719c
📒 Files selected for processing (12)
apps/web/app/(ee)/api/cron/discount-codes/create/route.tsapps/web/app/(ee)/api/cron/discount-codes/delete/route.tsapps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.tsapps/web/app/(ee)/api/customers/search-stripe/route.tsapps/web/app/(ee)/api/discount-codes/route.tsapps/web/app/(ee)/api/workflows/partner-approved/route.tsapps/web/lib/actions/partners/create-discount.tsapps/web/lib/api/customers/get-customer-stripe-invoices.tsapps/web/lib/api/discounts/create-discount-code.tsapps/web/lib/integrations/stripe/schema.tsapps/web/lib/stripe/create-stripe-discount-code.tsapps/web/lib/stripe/disable-stripe-discount-code.ts
|
@coderabbitai full review please, any breaking changes? |
🧩 Analysis chainI'll do a thorough investigation before the full review to check for any breaking changes. 🏁 Script executed: Length of output: 63631
SummaryThis PR replaces hardcoded Breaking Changes1. Function signature changes (internal breaking)Three functions had their signatures updated in a breaking way:
✅ All known call sites within the repo have been updated consistently in this PR — no orphaned callers found. Issues Found🔴
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/integrations/stripe/schema.ts (1)
3-19:⚠️ Potential issue | 🔴 CriticalAdd null-coalescing to all parse calls and backfill legacy stripe settings.
The schema's
.optional().default("live")only handlesundefined; Zod will throw on actualnullvalues. SinceInstalledIntegration.settingsis nullable in Prisma, nine of ten parse sites will fail for legacy installs withnullsettings. Onlycustomer-subscription-created.tssafely uses?? {}. Add null-coalescing to all remaining parse calls (partner-approved,discount-codes,cron/delete,cron/create,cron/remap,search-stripe,create-discount,get-customer-stripe-invoices, etc.), backfill or migrate legacy rows to ensuresettingsis nevernull, and verify with a workspace that previously had empty Stripe settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/stripe/schema.ts` around lines 3 - 19, The stripeIntegrationSettingsSchema currently uses .optional().default("live") which doesn't accept actual nulls from InstalledIntegration.settings; update every place that calls stripeIntegrationSettingsSchema.parse to coalesce null to an object (e.g., pass (installedIntegration.settings ?? {}) into stripeIntegrationSettingsSchema.parse) — specifically update parse sites such as partner-approved, discount-codes, cron/delete, cron/create, cron/remap, search-stripe, create-discount, get-customer-stripe-invoices, customer-subscription-created (ensure it already uses ?? {}), etc.; additionally add a DB backfill/migration to set legacy InstalledIntegration.settings null rows to {} so future reads are safe and verify the fix against a workspace that previously had empty Stripe settings.
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/cron/discount-codes/create/route.ts (1)
49-53: Select onlysettingsfrominstalledIntegrations.The default relation load pulls the full
InstalledIntegrationscalar payload, includingcredentials, but this handler only readssettings. Narrowing the select avoids moving sensitive JSON around unnecessarily.✂️ Suggested query tightening
installedIntegrations: { where: { integrationId: STRIPE_INTEGRATION_ID, }, + select: { + settings: true, + }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/discount-codes/create/route.ts around lines 49 - 53, The installedIntegrations relation is currently loading the full InstalledIntegration record (including credentials) but this handler only needs settings; tighten the query by adding a select that returns only settings for installedIntegrations (e.g., under the installedIntegrations block add select: { settings: true }) so you still filter by STRIPE_INTEGRATION_ID but avoid pulling sensitive fields like credentials; update the query where installedIntegrations is used (the object keyed installedIntegrations in this route handler) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/discount-codes/route.ts:
- Around line 69-90: The current code uses prisma.installedIntegration.findFirst
(referenced as installedStripeIntegration) making stripeMode nondeterministic;
replace this with a deterministic workspace-level lookup helper (e.g.,
getWorkspaceIntegrationSettings) that queries prisma.installedIntegration for
the given projectId + integrationId and returns the canonical workspace settings
(for example by selecting the row where userId is null / isWorkspace flag is
true, or enforcing a single row via findMany + consistency check or choosing the
most recent canonical record), then pass that result into
stripeIntegrationSettingsSchema.parse instead of the ad-hoc findFirst result to
ensure the Stripe environment is always resolved deterministically.
In `@apps/web/lib/stripe/disable-stripe-discount-code.ts`:
- Line 2: The StripeMode union type is inconsistent across packages (one defines
"sandbox" while the other does not); update the type so both packages accept the
same literals. Edit the StripeMode declaration used by the Stripe app package
(the StripeMode type in the utils/types.ts that the Stripe app exports/imports)
to include "sandbox" (or alternatively remove "sandbox" from the web package)
and ensure all imports refer to the unified type name StripeMode; consider
centralizing/exporting the canonical StripeMode type from a single module and
update references to use that shared type so both codebases remain aligned.
---
Outside diff comments:
In `@apps/web/lib/integrations/stripe/schema.ts`:
- Around line 3-19: The stripeIntegrationSettingsSchema currently uses
.optional().default("live") which doesn't accept actual nulls from
InstalledIntegration.settings; update every place that calls
stripeIntegrationSettingsSchema.parse to coalesce null to an object (e.g., pass
(installedIntegration.settings ?? {}) into
stripeIntegrationSettingsSchema.parse) — specifically update parse sites such as
partner-approved, discount-codes, cron/delete, cron/create, cron/remap,
search-stripe, create-discount, get-customer-stripe-invoices,
customer-subscription-created (ensure it already uses ?? {}), etc.; additionally
add a DB backfill/migration to set legacy InstalledIntegration.settings null
rows to {} so future reads are safe and verify the fix against a workspace that
previously had empty Stripe settings.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/discount-codes/create/route.ts:
- Around line 49-53: The installedIntegrations relation is currently loading the
full InstalledIntegration record (including credentials) but this handler only
needs settings; tighten the query by adding a select that returns only settings
for installedIntegrations (e.g., under the installedIntegrations block add
select: { settings: true }) so you still filter by STRIPE_INTEGRATION_ID but
avoid pulling sensitive fields like credentials; update the query where
installedIntegrations is used (the object keyed installedIntegrations in this
route handler) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a84f7589-a5d8-477e-8ea1-1acbb877ad41
📒 Files selected for processing (12)
apps/web/app/(ee)/api/cron/discount-codes/create/route.tsapps/web/app/(ee)/api/cron/discount-codes/delete/route.tsapps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.tsapps/web/app/(ee)/api/customers/search-stripe/route.tsapps/web/app/(ee)/api/discount-codes/route.tsapps/web/app/(ee)/api/workflows/partner-approved/route.tsapps/web/lib/actions/partners/create-discount.tsapps/web/lib/api/customers/get-customer-stripe-invoices.tsapps/web/lib/api/discounts/create-discount-code.tsapps/web/lib/integrations/stripe/schema.tsapps/web/lib/stripe/create-stripe-discount-code.tsapps/web/lib/stripe/disable-stripe-discount-code.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/stripe/integration/route.ts:
- Around line 21-27: The handler currently parses stripeMode (alongside
stripeAccountId) but never writes it to the installation/settings record, so
stripeMode silently defaults to "live"; update the create/update settings path
that currently reads/writes settings (refer to the settings variable and
wherever stripeAccountId is persisted) to also persist stripeMode on both new
installs and updates by adding stripeMode into the settings object saved to the
database (the same save/createOrUpdate call that stores stripeAccountId),
ensuring reads later will return the persisted stripeMode.
In `@packages/stripe-app/src/utils/dub.ts`:
- Line 2: The StripeMode type in packages/stripe-app/src/utils/types.ts is too
narrow (only "test" | "live"); update the StripeMode type declaration to include
"sandbox" (e.g., "live" | "test" | "sandbox") so imports like the one in
packages/stripe-app/src/utils/dub.ts accept sandbox mode; update any related
validation or switch statements that rely on StripeMode to handle the new
"sandbox" variant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 90fb56da-597e-45a0-9581-bf431487756b
📒 Files selected for processing (4)
apps/web/app/(ee)/api/stripe/integration/callback/route.tsapps/web/app/(ee)/api/stripe/integration/route.tspackages/stripe-app/src/utils/dub.tspackages/stripe-app/src/views/AppSettings.tsx
💤 Files with no reviewable changes (1)
- apps/web/app/(ee)/api/stripe/integration/callback/route.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/integrations/install.ts (1)
30-32:⚠️ Potential issue | 🟠 MajorPersist
settingson upsert update path as well.
settingsis accepted and written on create, but ignored on update. Re-installs/upserts can silently keep stale settings.Suggested fix
update: { credentials, + settings, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/install.ts` around lines 30 - 32, The upsert/update path for integrations currently only persists credentials and omits settings, causing re-installs to keep stale settings; update the upsert (or update) call that builds the update payload (the object currently containing update: { credentials, }) to also include settings (e.g., update: { credentials, settings }) and ensure the same settings value used on create is passed through so settings are written on both create and update in the function that performs the integration install/upsert.
♻️ Duplicate comments (1)
apps/web/app/(ee)/api/stripe/integration/route.ts (1)
65-104:⚠️ Potential issue | 🟠 Major
stripeModeis still not persisted for existing installations.The new value is saved only when no installation exists. For existing installations,
stripeModeupdates are ignored, so mode can remain stale.Suggested fix
waitUntil( (async () => { const installation = await prisma.installedIntegration.findUnique({ @@ if (!installation) { await installIntegration({ @@ settings: { stripeMode, }, }); + return; } @@ if (installation && stripeAccountId === null) { await prisma.installedIntegration.delete({ where: { id: installation.id, }, }); + return; } + + await prisma.installedIntegration.update({ + where: { id: installation.id }, + data: { + settings: { + ...((installation.settings as Record<string, unknown> | null) ?? {}), + stripeMode, + }, + }, + }); })(), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/stripe/integration/route.ts around lines 65 - 104, The code only saves stripeMode when creating a new installation; for existing installations you must persist changes to stripeMode as well. After fetching installation (from prisma.installedIntegration.findUnique) and before the deletion branch, detect if installation exists and stripeMode is different (or not null) and call prisma.installedIntegration.update (or use installIntegration update path) to set settings: { ...installation.settings, stripeMode } for that installation.id; keep the existing deletion logic for when stripeAccountId === null.
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/stripe/integration/webhook/customer-subscription-created.ts (1)
51-53: Inconsistent fallback operator:||vs??used elsewhere in the codebase.This line uses
||(logical OR) whereasupdate-stripe-settings.tsand likely other Stripe integration code uses??(nullish coalescing):// In update-stripe-settings.ts (line 28): const current = (installedIntegration.settings as any) ?? {};With
??, onlynull/undefinedare coerced to{}; with||, any falsy value (false,0,"") would also be coerced. Since the Prismasettingsfield isJson?and can technically hold any JSON value, this creates a subtle behavioral difference.If this change is intentional for more defensive handling, consider updating other usages for consistency. If unintentional, revert to
??to match the existing pattern.Suggested fix to align with existing pattern
const stripeIntegrationSettings = stripeIntegrationSettingsSchema.parse( - workspace.installedIntegrations[0].settings || {}, + workspace.installedIntegrations[0].settings ?? {}, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/stripe/integration/webhook/customer-subscription-created.ts around lines 51 - 53, The fallback for reading integration settings uses logical OR which coerces falsy values; change the expression passed into stripeIntegrationSettingsSchema.parse so it uses the nullish coalescing operator instead of || — i.e., replace the use of workspace.installedIntegrations[0].settings || {} with workspace.installedIntegrations[0].settings ?? {} in the code that sets stripeIntegrationSettings (the call to stripeIntegrationSettingsSchema.parse) to match the pattern used in update-stripe-settings.ts and avoid incorrectly overriding valid falsy JSON values.apps/web/app/(ee)/api/cron/discount-codes/create/route.ts (1)
49-53: Select onlysettingsfrominstalledIntegrationsto reduce payload.You only read
settingslater, so narrowing the relation select keeps this cron query leaner.Suggested refactor
installedIntegrations: { where: { integrationId: STRIPE_INTEGRATION_ID, }, + select: { + settings: true, + }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/discount-codes/create/route.ts around lines 49 - 53, The query currently selects the entire installedIntegrations relation but only needs settings; update the query that builds installedIntegrations (the object with where: { integrationId: STRIPE_INTEGRATION_ID }) to use a select that returns only settings (e.g., installedIntegrations: { where: { integrationId: STRIPE_INTEGRATION_ID }, select: { settings: true } }) so the cron in route.ts fetches a smaller payload while preserving access to settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/lib/integrations/install.ts`:
- Around line 30-32: The upsert/update path for integrations currently only
persists credentials and omits settings, causing re-installs to keep stale
settings; update the upsert (or update) call that builds the update payload (the
object currently containing update: { credentials, }) to also include settings
(e.g., update: { credentials, settings }) and ensure the same settings value
used on create is passed through so settings are written on both create and
update in the function that performs the integration install/upsert.
---
Duplicate comments:
In `@apps/web/app/`(ee)/api/stripe/integration/route.ts:
- Around line 65-104: The code only saves stripeMode when creating a new
installation; for existing installations you must persist changes to stripeMode
as well. After fetching installation (from
prisma.installedIntegration.findUnique) and before the deletion branch, detect
if installation exists and stripeMode is different (or not null) and call
prisma.installedIntegration.update (or use installIntegration update path) to
set settings: { ...installation.settings, stripeMode } for that installation.id;
keep the existing deletion logic for when stripeAccountId === null.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/discount-codes/create/route.ts:
- Around line 49-53: The query currently selects the entire
installedIntegrations relation but only needs settings; update the query that
builds installedIntegrations (the object with where: { integrationId:
STRIPE_INTEGRATION_ID }) to use a select that returns only settings (e.g.,
installedIntegrations: { where: { integrationId: STRIPE_INTEGRATION_ID },
select: { settings: true } }) so the cron in route.ts fetches a smaller payload
while preserving access to settings.
In
`@apps/web/app/`(ee)/api/stripe/integration/webhook/customer-subscription-created.ts:
- Around line 51-53: The fallback for reading integration settings uses logical
OR which coerces falsy values; change the expression passed into
stripeIntegrationSettingsSchema.parse so it uses the nullish coalescing operator
instead of || — i.e., replace the use of
workspace.installedIntegrations[0].settings || {} with
workspace.installedIntegrations[0].settings ?? {} in the code that sets
stripeIntegrationSettings (the call to stripeIntegrationSettingsSchema.parse) to
match the pattern used in update-stripe-settings.ts and avoid incorrectly
overriding valid falsy JSON values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d306d391-f428-4a30-a457-4ddbbf54dbc5
📒 Files selected for processing (11)
apps/web/app/(ee)/api/cron/discount-codes/create/route.tsapps/web/app/(ee)/api/cron/discount-codes/delete/route.tsapps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.tsapps/web/app/(ee)/api/customers/search-stripe/route.tsapps/web/app/(ee)/api/discount-codes/route.tsapps/web/app/(ee)/api/stripe/integration/route.tsapps/web/app/(ee)/api/stripe/integration/webhook/customer-subscription-created.tsapps/web/app/(ee)/api/workflows/partner-approved/route.tsapps/web/lib/actions/partners/create-discount.tsapps/web/lib/api/customers/get-customer-stripe-invoices.tsapps/web/lib/integrations/install.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/app/(ee)/api/cron/discount-codes/delete/route.ts
- apps/web/app/(ee)/api/discount-codes/route.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts
- apps/web/app/(ee)/api/customers/search-stripe/route.ts
- apps/web/app/(ee)/api/workflows/partner-approved/route.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/stripe/integration/route.ts (2)
82-85: Consider improving type safety when merging settings.The
as anycast loses type safety and the optional chaining oninstallation?.settingsis redundant sinceinstallationis guaranteed non-null after the check at line 46.♻️ Suggested improvement
data: { settings: { - ...((installation?.settings as any) || {}), + ...((installation.settings as Record<string, unknown>) ?? {}), stripeMode, }, },Alternatively, parse existing settings through the schema for full type safety:
const existingSettings = stripeIntegrationSettingsSchema .partial() .safeParse(installation.settings); const mergedSettings = { ...(existingSettings.success ? existingSettings.data : {}), stripeMode, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/stripe/integration/route.ts around lines 82 - 85, The current merge uses (installation?.settings as any) which loses type safety and the optional chaining is unnecessary because installation is non-null; instead validate/parse installation.settings with stripeIntegrationSettingsSchema.partial().safeParse (or .parse) to get a typed existingSettings and then merge { ...(existingSettings.success ? existingSettings.data : {}), stripeMode } into the settings field; update the code paths that reference installation.settings and remove the redundant ?. and any casts to any to preserve type safety.
66-90: Silent failure inwaitUntilblock may cause settings inconsistency.If the
prisma.installedIntegration.updateordeletefails (e.g., transient DB error), the failure is swallowed silently. The response already succeeded with the project update, but the installation'sstripeModewould remain stale.This may be acceptable for background work, but consider logging failures at minimum.
♻️ Add error logging
waitUntil( (async () => { + try { // Uninstall the integration if the stripe account id is null if (installation && stripeAccountId === null) { await prisma.installedIntegration.delete({ where: { id: installation.id, }, }); // else, update the Stripe mode for the installation } else { await prisma.installedIntegration.update({ where: { id: installation.id, }, data: { settings: { ...((installation?.settings as any) || {}), stripeMode, }, }, }); } + } catch (error) { + console.error( + `[Stripe Integration] Failed to update installation settings for workspace ${workspace.id}:`, + error, + ); + } })(), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/stripe/integration/route.ts around lines 66 - 90, The background waitUntil IIFE that calls prisma.installedIntegration.delete or update can fail silently; wrap the async function body in a try/catch and log any error (including the caught error, installation.id, stripeAccountId and stripeMode) so failures are visible for debugging; specifically modify the anonymous async function passed to waitUntil (the block that calls prisma.installedIntegration.delete and prisma.installedIntegration.update) to catch exceptions and call a logger (or console.error) with a clear message and context about the installation and operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/app/`(ee)/api/stripe/integration/route.ts:
- Around line 82-85: The current merge uses (installation?.settings as any)
which loses type safety and the optional chaining is unnecessary because
installation is non-null; instead validate/parse installation.settings with
stripeIntegrationSettingsSchema.partial().safeParse (or .parse) to get a typed
existingSettings and then merge { ...(existingSettings.success ?
existingSettings.data : {}), stripeMode } into the settings field; update the
code paths that reference installation.settings and remove the redundant ?. and
any casts to any to preserve type safety.
- Around line 66-90: The background waitUntil IIFE that calls
prisma.installedIntegration.delete or update can fail silently; wrap the async
function body in a try/catch and log any error (including the caught error,
installation.id, stripeAccountId and stripeMode) so failures are visible for
debugging; specifically modify the anonymous async function passed to waitUntil
(the block that calls prisma.installedIntegration.delete and
prisma.installedIntegration.update) to catch exceptions and call a logger (or
console.error) with a clear message and context about the installation and
operation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 489973eb-fe37-428d-b3fd-21b414d0c032
📒 Files selected for processing (1)
apps/web/app/(ee)/api/stripe/integration/route.ts
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
@coderabbitai full review, any breaking changes, incorrect code? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 31795 🏁 Script executed: Length of output: 42727
✅ No Breaking Changes to Public APIAll three functions that changed signatures ( ✅ Issues from Previous Review — Now Resolved
export type StripeMode = "live" | "test" | "sandbox";🔴 Incorrect Code:
|
| Area | Verdict |
|---|---|
AppSettings.tsx sandbox detection (isSandbox ? "sandbox" : environment.mode) |
✅ Correct |
getRedirectUrl("sandbox") maps to "test/" URL |
✅ Correct for Stripe sandbox |
settings || {} vs settings ?? {} |
✅ Cosmetically different, not a bug |
Schema default stripeMode: "live" — no migration needed |
✅ |
| Deleted OAuth callback route — install now via Dub OAuth flow | ✅ Intentional |
All cron routes, API routes, and server actions pass stripeMode correctly |
✅ |
Summary
The PR is logically correct with no breaking changes to the public surface. Two items worth addressing before merge:
- Add
settingsto theupdatepath ininstallIntegration— not a regression from this PR, but a latent bug exposed by the newsettingsparameter. - Align
stripeAppClientfallback from"test"to"live"to match schema default.
✏️ Learnings added
Learnt from: steven-tey
URL: https://github.com/dubinc/dub/pull/3679
Timestamp: 2026-04-01T18:57:38.990Z
Learning: In dubinc/dub, the `installIntegration` function in `apps/web/lib/integrations/install.ts` uses a Prisma upsert where the `update` path only persists `credentials` but NOT `settings`. The `settings` field is only written on the `create` path. For the Stripe integration, this does not cause issues because settings are managed via the PATCH `/api/stripe/integration` route directly. But if any other integration adopts the `settings` parameter via `installIntegration` and expects it to be updated on re-install, the upsert update path must also include `settings`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: steven-tey
Repo: dubinc/dub PR: 3679
File: apps/web/lib/stripe/disable-stripe-discount-code.ts:2-2
Timestamp: 2026-04-01T17:57:42.171Z
Learning: In dubinc/dub, there is a known StripeMode type divergence between packages: `apps/web/lib/types.ts` defines `StripeMode = "test" | "sandbox" | "live"`, while `packages/stripe-app/src/utils/types.ts` defines `StripeMode = "test" | "live"` (missing "sandbox"). This is intentional for now; the Stripe app package needs to be updated separately to support "sandbox" mode. Do not flag this divergence as a blocking issue in future reviews.
Learnt from: steven-tey
Repo: dubinc/dub PR: 3679
File: apps/web/app/(ee)/api/discount-codes/route.ts:69-90
Timestamp: 2026-04-01T17:57:13.716Z
Learning: In the dubinc/dub codebase, a workspace can only ever have one installed Stripe integration at a time. Therefore, `prisma.installedIntegration.findFirst({ where: { projectId, integrationId: STRIPE_INTEGRATION_ID } })` is always deterministic and there is no risk of returning the wrong row. Do not flag this pattern as nondeterministic in future reviews.
|
@coderabbitai addressed your feedback, can you review again? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of the two files where I flagged issues. 🏁 Script executed: Length of output: 3290
✅
|
|
@coderabbitai the PR is fully backwards compatible right? |
|
Tip For best results, initiate chat on the files or code changes.
One minor behavioral note: several routes now return 🧠 Learnings used |
2 participants
Summary by CodeRabbit
New Features
Improvements
Error Handling
Breaking Changes