Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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:
📝 WalkthroughWalkthroughAdds Stripe customer search and invoice import endpoints, Zod schemas for Stripe data, Stripe import UI and modal API changes, invoice-driven commission flows and helpers, an invoice-fetching client, a backfill script, and related auth/user field expansions. Changes
Sequence DiagramsequenceDiagram
actor User
participant Modal as "Add Customer Modal"
participant API as "Search Stripe API (server)"
participant Stripe as "Stripe API"
participant DB as "Prisma DB"
User->>Modal: Enter email & click "Search Stripe"
Modal->>API: GET /api/customers/search-stripe?q=<email>
API->>Stripe: customers.search(query, expand=subscriptions) [stripeAccount=scoped]
Stripe-->>API: Stripe customer records
API->>DB: SELECT customers WHERE stripeCustomerId IN (...)
DB-->>API: Matching local customers (dubCustomerId)
API-->>Modal: Enriched customer list (id,email,name,country,subscriptions,dubCustomerId)
User->>Modal: Select customer to import
Modal->>Modal: Populate form fields, set imported state
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/customers/search-stripe/route.ts (1)
28-36: Consider wrapping the Stripe API call in a try/catch for better error messages.If the Stripe API call fails (e.g., account disconnected, rate-limited, invalid connect account), the raw Stripe error will bubble up as a 500. The
discount-codes/route.ts(Lines 160-167) already demonstrates explicit Stripe error handling. A similar pattern here would give users actionable feedback.♻️ Suggested pattern
+ try { const { data } = await stripe.customers.search( { query: `email~"${search}"`, expand: ["data.subscriptions"], }, { stripeAccount: workspace.stripeConnectId, }, ); + } catch (error) { + throw new DubApiError({ + code: "bad_request", + message: "Failed to search Stripe customers. Please check your Stripe integration.", + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/customers/search-stripe/route.ts around lines 28 - 36, The stripe.customers.search call is unprotected and will surface raw Stripe errors as 500s; wrap the API call in a try/catch (around the stripe.customers.search invocation in this route handler) and catch Stripe errors to translate them into clearer responses (inspect error.type / statusCode / message) similar to discount-codes/route.ts handling, log context including workspace.stripeConnectId, and return a user-friendly error payload/status instead of letting the raw error bubble up.apps/web/ui/modals/add-customer-modal.tsx (1)
334-342: Addrel="noopener noreferrer"to thetarget="_blank"link.The "Already on Dub" link opens in a new tab but lacks
rel="noopener noreferrer". While this is an internal link and the risk is low, it's good practice to include it for consistency with the Stripe dashboard link on line 324.Proposed fix
<a href={`/${slug}/program/customers/${customer.dubCustomerId}`} target="_blank" + rel="noopener noreferrer" className="rounded bg-neutral-200/80 px-1.5 py-0.5 text-neutral-500 hover:bg-neutral-200 hover:text-neutral-900" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/add-customer-modal.tsx` around lines 334 - 342, The anchor rendering "Already on Dub" uses target="_blank" but is missing rel="noopener noreferrer"; update the JSX anchor (the element that checks customer.dubCustomerId and renders the link to `/${slug}/program/customers/${customer.dubCustomerId}`) to include rel="noopener noreferrer" alongside target="_blank" to prevent potential window.opener issues and match the pattern used for the Stripe dashboard link.
🤖 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/customers/search-stripe/route.ts:
- Around line 28-36: The Stripe Search query is built by interpolating the
user-provided variable `search` directly into the query string passed to
`stripe.customers.search`, which allows DSL injection when `search` contains `"`
or `\`; fix this by escaping `search` before interpolation (replace `\` with
`\\` and `"` with `\"`) and then use the escaped value in the `query:
\`email="${escaped}"\`` argument (while leaving `stripeAccount:
workspace.stripeConnectId` unchanged); alternatively, if you want an exact email
lookup, replace the `stripe.customers.search` call with the List Customers API
that accepts an `email` parameter to avoid the query DSL entirely.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/customers/search-stripe/route.ts:
- Around line 28-36: The stripe.customers.search call is unprotected and will
surface raw Stripe errors as 500s; wrap the API call in a try/catch (around the
stripe.customers.search invocation in this route handler) and catch Stripe
errors to translate them into clearer responses (inspect error.type / statusCode
/ message) similar to discount-codes/route.ts handling, log context including
workspace.stripeConnectId, and return a user-friendly error payload/status
instead of letting the raw error bubble up.
In `@apps/web/ui/modals/add-customer-modal.tsx`:
- Around line 334-342: The anchor rendering "Already on Dub" uses
target="_blank" but is missing rel="noopener noreferrer"; update the JSX anchor
(the element that checks customer.dubCustomerId and renders the link to
`/${slug}/program/customers/${customer.dubCustomerId}`) to include rel="noopener
noreferrer" alongside target="_blank" to prevent potential window.opener issues
and match the pattern used for the Stripe dashboard link.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx (2)
276-308:⚠️ Potential issue | 🟡 MinorMisleading submit tooltip when Stripe invoice fetch fails;
stripeInvoicesErrormissing from depsWhen a non-400 fetch error occurs,
stripeInvoicesErroris set butstripeInvoicesstays[]. The memo falls through to thestripeInvoices.length === 0branch and returns "No paid Stripe invoices found" — but the real cause is a fetch failure.stripeInvoicesErroris also absent from the dependency array, which can cause a stale tooltip if an error fires whileisStripeInvoicesLoadingis alreadyfalse(e.g., SWR error-retry after a previously successful fetch).🐛 Proposed fix
if (commissionType === "sale") { if (useExistingEvents) { if (isStripeInvoicesLoading) { return "Loading Stripe invoices..."; } if (noStripeCustomerId) { return "This customer doesn't have a Stripe customer ID. Add one in the customer profile before proceeding."; } + if (stripeInvoicesError) { + return "Failed to load Stripe invoices. Please try again."; + } + if (stripeInvoices.length === 0) { return "No paid Stripe invoices found for this customer."; } } else {}, [ commissionType, partnerId, linkId, customerId, amount, saleAmount, useExistingEvents, noStripeCustomerId, stripeInvoices.length, isStripeInvoicesLoading, + stripeInvoicesError, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx around lines 276 - 308, The memo's validation logic wrongly treats an empty stripeInvoices array (on fetch failure) as "No paid Stripe invoices found" and also omits stripeInvoicesError from the dependency array; update the commission-type branch (where commissionType === "sale" and useExistingEvents is true) to first check if stripeInvoicesError is set (and isStripeInvoicesLoading is false) and return a clear error message like "Error fetching Stripe invoices: <error message>" and add stripeInvoicesError to the memo dependency array so the tooltip updates when an error occurs; keep the existing checks for isStripeInvoicesLoading, noStripeCustomerId, and stripeInvoices.length after this new error check.
580-607:⚠️ Potential issue | 🟡 Minor"Import from Stripe" mode for
leadtype renders an empty sectionThe event source toggle is displayed for all commission types when a customer is selected. However, the Stripe invoices list (lines 609–686) is only rendered when
commissionType === "sale", and the commission detail fields (lines 688–965) are hidden whenuseExistingEvents === true. Selecting "Import from Stripe" whilecommissionType === "lead"therefore leaves the area below the toggle completely empty, with no user guidance. The submit button also remains enabled in that state sincesubmitDisabledMessagehas no lead-specific guard foruseExistingEvents === true.If the "Import from Stripe" path is intentionally unsupported for
leadcommissions, guarding the toggle is the cleanest fix:🐛 Proposed fix
- {customerId && ( + {customerId && commissionType === "sale" && ( <div> <label htmlFor="eventSource">If
lead + useExistingEventsis a valid backend path, the Stripe invoices section or equivalent UI should also be exposed for that type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx around lines 580 - 607, The event-source ToggleGroup (rendered when customerId is truthy and using eventSourceOptions/selectAction that toggles useExistingEvents) is shown for all commission types but the Stripe invoices UI and detail fields are only implemented for commissionType === "sale", leaving a blank area for lead commissions; either hide the toggle for non-sale commissions or implement the invoices path for leads. Fix by wrapping the ToggleGroup and its descriptive <p> in a commissionType check (e.g., only render when commissionType === "sale"), and update submitDisabledMessage logic to consider useExistingEvents only when commissionType === "sale"; alternatively, if lead + useExistingEvents is supported, render the invoice/list UI for commissionType === "lead" where the code currently renders the Stripe invoices block.
🧹 Nitpick comments (3)
apps/web/lib/actions/partners/create-manual-commission.ts (1)
383-387:Promise.allSettledsilently swallows commission-creation failures.The results of
Promise.allSettledon commission creation are discarded. If a commission fails to create (e.g., DB constraint violation), the function proceeds to update link stats and trigger the cron job as though it succeeded. Consider inspecting the results and logging/handling any rejections.Suggested improvement
- await Promise.allSettled( - commissionsToCreate.map((commission) => - createPartnerCommission(commission), - ), - ); + const commissionResults = await Promise.allSettled( + commissionsToCreate.map((commission) => + createPartnerCommission(commission), + ), + ); + const failures = commissionResults.filter((r) => r.status === "rejected"); + if (failures.length > 0) { + console.error("Failed to create commissions:", failures); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-manual-commission.ts` around lines 383 - 387, The Promise.allSettled call over commissionsToCreate with createPartnerCommission currently discards results; change it to capture the settled results, iterate over them to detect any rejected promises, log each rejection with identifying data from the corresponding commission, and either throw or return an error status when any creation fails so downstream steps (e.g., updateLinkStats and the cron trigger) don't run as if all commissions succeeded; keep references to commissionsToCreate, createPartnerCommission, updateLinkStats and the partner commission cron trigger when adding the error handling and logging.apps/web/app/(ee)/api/customers/[id]/stripe-invoices/route.ts (1)
36-44: Stripeinvoices.listreturns only one page of results (default 10 items) — add pagination or auto-pagination.This endpoint currently fetches only the first page of paid invoices. If a customer has more than 10 invoices, the results will silently truncate. Either increase
limit: 100for a practical improvement (covers up to 100 invoices), or use.autoPagingToArray()to fetch all invoices. Consider the intended behavior: if partial results are acceptable for your use case, clarify this in a comment; otherwise, implement full pagination.Option 1: Increase limit to 100
const { data } = await stripe.invoices.list( { customer: customer.stripeCustomerId, status: "paid", + limit: 100, }, { stripeAccount: workspace.stripeConnectId, }, );Option 2: Fetch all invoices with auto-pagination
- const { data } = await stripe.invoices.list( + const data = await stripe.invoices.autoPagingToArray( { customer: customer.stripeCustomerId, status: "paid", }, { stripeAccount: workspace.stripeConnectId, }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/customers/[id]/stripe-invoices/route.ts around lines 36 - 44, The call to stripe.invoices.list in route.ts currently only fetches the first page (default 10 items); update the code to handle pagination by either adding a limit: 100 to the request options or (preferably) using Stripe's auto-pagination to fetch all results (e.g., use stripe.invoices.list(...).autoPagingToArray() or the async iterator) so paid invoices for customer.stripeCustomerId aren't truncated; modify the stripe.invoices.list invocation and downstream handling accordingly while keeping stripeAccount: workspace.stripeConnectId and status: "paid".apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx (1)
157-170:eventSourceOptionsis a static constant — hoist it outside the componentThe array has no dependency on props or state and is recreated on every render. Moving it to module scope avoids the unnecessary allocation.
♻️ Proposed refactor
+const eventSourceOptions = [ + { + value: "new", + label: "Create new event", + description: + "Create a new sale event for the partner (e.g. for one-time purchases)", + }, + { + value: "existing", + label: "Import from Stripe", + description: + "Fetch the customer's paid invoices from Stripe (e.g. for recurring subscriptions)", + }, +]; + function CreateCommissionSheetContent({ setIsOpen, }: CreateCommissionSheetProps) { ... - const eventSourceOptions = [ - { - value: "new", - ... - }, - { - value: "existing", - ... - }, - ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx around lines 157 - 170, Hoist the static array eventSourceOptions out of the component in create-commission-sheet.tsx into module scope so it isn't recreated each render; locate the const eventSourceOptions declaration inside the component, move it to top-level (above the component function), and leave usage in the component unchanged to avoid reallocation and unnecessary renders.
🤖 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/lib/actions/partners/create-manual-commission.ts`:
- Line 196: The code hardcodes currency: "usd" in the create-manual-commission
flow which will mis-record commissions for non-USD invoices; update the logic in
the create-manual-commission action (and its input/schema) to derive currency
from the Stripe invoice object (e.g., invoice.currency) or accept a currency
field in the action input and validate it in the schema instead of using the
hardcoded "usd"; ensure any downstream uses (DB save or CommissionCreate
payload) read that passed/derived currency value.
- Around line 155-224: The leadEventTimestamp is never set in the
useExistingEvents sale path; after creating leadEventData (via
leadEventSchemaTBWithTimestamp) assign leadEventTimestamp to the lead event time
(e.g., new Date(leadEventData.timestamp) or finalLeadEventDate) so the later
updateLinkStatsForImporter uses the actual Tinybird lead timestamp; place this
assignment right after
tbEventsToRecord.push(recordLeadWithTimestamp(leadEventData)) so
leadEventTimestamp, tbEventsToRecord, and commissionsToCreate remain consistent.
- Around line 239-250: The branch handling useExistingEvents with commissionType
=== "lead" is incomplete and leads to phantom link stats because post-processing
still increments link.clicks/link.leads without real events; update the branch
in create-manual-commission.ts (the block checking commissionType === "lead" and
customer.linkId) to either throw a clear "not implemented" error (e.g., throw
new Error("useExistingEvents for lead commissions not implemented")) or perform
an early return so the later post-processing that updates link.clicks and
link.leads is skipped; ensure you reference the existing symbols commissionType,
useExistingEvents, and customer.linkId when making the change.
---
Outside diff comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 276-308: The memo's validation logic wrongly treats an empty
stripeInvoices array (on fetch failure) as "No paid Stripe invoices found" and
also omits stripeInvoicesError from the dependency array; update the
commission-type branch (where commissionType === "sale" and useExistingEvents is
true) to first check if stripeInvoicesError is set (and isStripeInvoicesLoading
is false) and return a clear error message like "Error fetching Stripe invoices:
<error message>" and add stripeInvoicesError to the memo dependency array so the
tooltip updates when an error occurs; keep the existing checks for
isStripeInvoicesLoading, noStripeCustomerId, and stripeInvoices.length after
this new error check.
- Around line 580-607: The event-source ToggleGroup (rendered when customerId is
truthy and using eventSourceOptions/selectAction that toggles useExistingEvents)
is shown for all commission types but the Stripe invoices UI and detail fields
are only implemented for commissionType === "sale", leaving a blank area for
lead commissions; either hide the toggle for non-sale commissions or implement
the invoices path for leads. Fix by wrapping the ToggleGroup and its descriptive
<p> in a commissionType check (e.g., only render when commissionType ===
"sale"), and update submitDisabledMessage logic to consider useExistingEvents
only when commissionType === "sale"; alternatively, if lead + useExistingEvents
is supported, render the invoice/list UI for commissionType === "lead" where the
code currently renders the Stripe invoices block.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/customers/[id]/stripe-invoices/route.ts:
- Around line 36-44: The call to stripe.invoices.list in route.ts currently only
fetches the first page (default 10 items); update the code to handle pagination
by either adding a limit: 100 to the request options or (preferably) using
Stripe's auto-pagination to fetch all results (e.g., use
stripe.invoices.list(...).autoPagingToArray() or the async iterator) so paid
invoices for customer.stripeCustomerId aren't truncated; modify the
stripe.invoices.list invocation and downstream handling accordingly while
keeping stripeAccount: workspace.stripeConnectId and status: "paid".
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 157-170: Hoist the static array eventSourceOptions out of the
component in create-commission-sheet.tsx into module scope so it isn't recreated
each render; locate the const eventSourceOptions declaration inside the
component, move it to top-level (above the component function), and leave usage
in the component unchanged to avoid reallocation and unnecessary renders.
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 383-387: The Promise.allSettled call over commissionsToCreate with
createPartnerCommission currently discards results; change it to capture the
settled results, iterate over them to detect any rejected promises, log each
rejection with identifying data from the corresponding commission, and either
throw or return an error status when any creation fails so downstream steps
(e.g., updateLinkStats and the cron trigger) don't run as if all commissions
succeeded; keep references to commissionsToCreate, createPartnerCommission,
updateLinkStats and the partner commission cron trigger when adding the error
handling and logging.
Collaborator
Author
|
@coderabbitai full review, any breaking changes? |
Contributor
|
Breaking Changes Assessment: ✅ None DetectedAll changes in this PR appear to be additive and non-breaking: New Additions (Non-Breaking)
Modified Without Breaking Changes
Key Observations
I'm now triggering a comprehensive review of all the changes to provide detailed feedback on code quality, best practices, and potential issues. ✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/lib/actions/partners/create-manual-commission.ts (1)
289-306:⚠️ Potential issue | 🟡 Minor
lastLeadAtnot assigned in thecommissionType === "sale"pathA lead event is always recorded (lines 277–287) with timestamp
finalLeadEventDate, butlastLeadAtis only assigned inside thecommissionType === "lead"guard (line 305). WhencommissionType === "sale",lastLeadAtstaysundefined, and the fallback on line 391 (lastLeadAt || new Date()) silently stamps the link'slastLeadAtwith the current wall-clock time rather than the actual event date.🐛 Proposed fix
tbEventsToRecord.push(recordLeadWithTimestamp(leadEventData)); + // Always track the lead event timestamp for link stats + lastLeadAt = new Date(leadEventData.timestamp); + if (commissionType === "lead") { commissionsToCreate.push({ ... }); - // Track the lead event timestamp for link stats update - lastLeadAt = new Date(leadEventData.timestamp); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-manual-commission.ts` around lines 289 - 306, The bug is that lastLeadAt is only set inside the commissionType === "lead" branch, so when commissionType === "sale" the link gets stamped with the current time instead of the actual lead event date; update the sale path (the branch handling commissionType === "sale" where a lead event is always recorded with finalLeadEventDate/leadEventData) to assign lastLeadAt = new Date(finalLeadEventDate) (or new Date(leadEventData.timestamp) if that variable is used there) so lastLeadAt is populated with the actual event timestamp before the link stats update.apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx (1)
261-308:⚠️ Potential issue | 🟡 Minor
stripeInvoicesErroris missing fromsubmitDisabledMessage— misleading tooltip on fetch failure.When the invoice fetch throws,
stripeInvoicesDatabecomesundefined,stripeInvoicesfalls back to[], and the submit tooltip displays "No paid Stripe invoices found for this customer." — even though the problem is a network/server error, not an empty invoice list. Additionally,stripeInvoicesErroris absent from theuseMemodependency array, so the memo won't recompute when the error changes.🔧 Proposed fix
if (useExistingEvents) { if (isStripeInvoicesLoading) { return "Loading Stripe invoices..."; } if (noStripeCustomerId) { return "This customer doesn't have a Stripe customer ID. Add one in the customer profile before proceeding."; } + if (stripeInvoicesError) { + return "Failed to load Stripe invoices. Please try again."; + } if (stripeInvoices.length === 0) { return "No paid Stripe invoices found for this customer."; } }And add
stripeInvoicesErrorto the dependency array:], [ commissionType, partnerId, linkId, customerId, amount, saleAmount, useExistingEvents, noStripeCustomerId, stripeInvoices.length, isStripeInvoicesLoading, + stripeInvoicesError, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx around lines 261 - 308, The submitDisabledMessage useMemo incorrectly treats a fetch failure as "no invoices" because it doesn't check stripeInvoicesError and doesn't include it in the dependency list; update the logic inside submitDisabledMessage to return a clear error tooltip (e.g., "Error loading Stripe invoices. Please try again.") when stripeInvoicesError is truthy (before checking stripeInvoices.length) and add stripeInvoicesError to the useMemo dependency array so the memo recomputes on fetch failures; locate references to submitDisabledMessage, stripeInvoices, stripeInvoicesData, and stripeInvoicesError to implement this change.
🧹 Nitpick comments (6)
apps/web/scripts/programs/backfill-reuse-commission.ts (3)
323-323: Redundant guard —commissionTypeis always"lead" | "sale".
commissionTypeis typed"lead" | "sale"(line 34), so["lead", "sale"].includes(commissionType)is alwaystrue. This check can be removed for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/programs/backfill-reuse-commission.ts` at line 323, The guard if (["lead", "sale"].includes(commissionType)) is redundant because commissionType is already typed as "lead" | "sale"; remove that if-statement and unindent/inline its body so the logic runs directly using the existing commissionType variable (refer to commissionType where it’s declared and the surrounding backfill logic) to simplify the code and improve clarity.
358-358: Add error handling on the top-levelmain()call.An unhandled rejection will print a stack trace but may not set a non-zero exit code in all Node versions. Standard pattern for scripts:
-main(); +main() + .then(() => process.exit(0)) + .catch((err) => { + console.error(err); + process.exit(1); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/programs/backfill-reuse-commission.ts` at line 358, Wrap the top-level main() invocation with a promise catch to handle rejections from main(): call main().catch(err => { console.error("Uncaught error in main:", err); process.exitCode = 1; }) (or use process.exit(1)) so any unhandled rejection logs the error and sets a non-zero exit code; update the existing top-level main() call to this pattern referencing the main() function.
231-247: Spreading...customercopies managed fields (createdAt,updatedAt) into the new record.The full Prisma model object includes
createdAtandupdatedAt. The duplicate customer will inherit the original's timestamps, which may or may not be intended. If Prisma'sCustomermodel gains new non-column fields in the future, those would also be passed and could cause runtime errors.Consider explicitly selecting only the fields you want to carry over instead of spreading the entire object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/programs/backfill-reuse-commission.ts` around lines 231 - 247, The code currently spreads ...customer into tx.customer.create (in the block creating the duplicate using duplicateCustomerId, link, clickEventData, recordSaleEvents, totalSales, totalSaleAmount), which copies managed fields like createdAt/updatedAt and may pull in future non-column fields; instead, replace the spread with an explicit list of allowed customer properties to carry over (e.g., name, email, phone, address, externalId — whatever your domain uses) or explicitly strip managed fields first (const { createdAt, updatedAt, ...safeCustomer } = customer) and use safeCustomer when calling tx.customer.create; keep id: duplicateCustomerId, linkId: link.id, programId: link.programId, partnerId: link.partnerId, clickId: clickEventData.click_id, clickedAt: new Date(clickEventData.timestamp), country logic, and the conditional sales/saleAmount bits intact.apps/web/lib/actions/partners/create-manual-commission.ts (2)
353-365:Promise.allSettledsilently swallows commission-creation failuresUsing
allSettledmeans a failedcreatePartnerCommissioncall is discarded without surfacing an error to the caller. The user sees a success response even when no commissions were created. Consider switching toPromise.allso that a single failure aborts and propagates the error, or at minimum inspect the settled results and throw if any are rejected.♻️ Proposed fix
- await Promise.allSettled( - commissionsToCreate.map((commission) => - createPartnerCommission(commission), - ), - ); + const commissionResults = await Promise.allSettled( + commissionsToCreate.map((commission) => + createPartnerCommission(commission), + ), + ); + + const failedCommissions = commissionResults.filter( + (r) => r.status === "rejected", + ); + if (failedCommissions.length > 0) { + throw new Error( + `Failed to create ${failedCommissions.length} commission(s): ${failedCommissions.map((r) => (r as PromiseRejectedResult).reason).join(", ")}`, + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-manual-commission.ts` around lines 353 - 365, The current use of Promise.allSettled for commissionsToCreate hides failures from createPartnerCommission and returns success even when some creations failed; update the block that calls commissionsToCreate.map(...) so that either (a) you use await Promise.all(commissionsToCreate.map(c => createPartnerCommission(c))) to let any rejection propagate, or (b) keep Promise.allSettled but inspect the results array and throw a consolidated error if any result.status === "rejected" (include the rejection reasons). Make the change around the commissionsToCreate variable and createPartnerCommission calls and ensure the thrown error includes identifying info from the rejected results so the caller sees the failure.
354-365: Debugconsole.logcalls in production pathLines 285, 323, 354, and 363–365 emit raw objects via
console.logon every invocation. These are noisy in production and could inadvertently log PII (customer email surfaced in error paths adjacent to these lines). Consider replacing with a structured logger or removing before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-manual-commission.ts` around lines 354 - 365, Replace raw console.log calls that print objects (calls printing prettyPrint(tbRes) and prettyPrint(commissionsToCreate) as well as the console.log before Promise.allSettled) with structured logging or remove them: use the existing app logger (e.g., processLogger.info/debug) instead of console.log, avoid printing full objects (sanitize or log only non-PII fields like counts or IDs), and guard detailed debug output behind an environment check (e.g., isDevelopment) so createPartnerCommission, commissionsToCreate, and tbRes are not dumped in production.apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx (1)
157-170:eventSourceOptionsis a static constant — lift to module scope.It has no dependency on props, state, or context, so re-allocating it inside the component on every render is unnecessary.
♻️ Proposed refactor
+const eventSourceOptions = [ + { + value: "new", + label: "Create new event", + description: + "Create a new sale event for the partner (e.g. for one-time purchases)", + }, + { + value: "existing", + label: "Import from Stripe", + description: + "Fetch the customer's paid invoices from Stripe (e.g. for recurring subscriptions)", + }, +]; + function CreateCommissionSheetContent({ setIsOpen }: CreateCommissionSheetProps) { ... - const eventSourceOptions = [ - { - value: "new", - ... - }, - ... - ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx around lines 157 - 170, Move the eventSourceOptions constant out of the React component and into module scope so it isn't re-created on every render: locate the eventSourceOptions declaration in create-commission-sheet.tsx and cut it to the top-level of the module (above the component function, e.g. above CreateCommissionSheet or the default-exported component). Keep it as a const and, if your component or other modules expect a specific shape, add a concise type annotation (e.g. an array of option objects) or export it if used elsewhere; update any local references to use the module-scoped eventSourceOptions.
🤖 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/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 49-66: The fetcherStripeInvoices function currently treats any 400
with a message as noStripeCustomerId; update it to check a specific error code
from the API (e.g., body?.error?.code === "no_stripe_customer_id") instead of
only res.status === 400, returning { invoices: [], noStripeCustomerId: true,
message } only when that code matches; for other non-ok responses throw new
Error(body?.error?.message ?? "Failed to load invoices") so unrelated 400s
surface correctly, and ensure the API returns a stable error.code string that
fetcherStripeInvoices can match against.
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 186-187: The code sets lastConversionAt from
stripeCustomerInvoices[0] but stripeCustomerInvoices is sorted ascending, so
that uses the oldest invoice; change lastConversionAt to use the most recent
invoice (the last element of stripeCustomerInvoices) while keeping lastLeadAt
set to the earliest as intended—update the assignment that references
stripeCustomerInvoices[0].created to instead reference the last item’s created
value (e.g., stripeCustomerInvoices[stripeCustomerInvoices.length - 1].created)
so lastConversionAt reflects the latest invoice date.
- Line 428: In the useExistingEvents branch update the firstSaleAt assignment to
use the earliest invoice timestamp (stripeCustomerInvoices[0].created) rather
than new Date(); convert the Stripe timestamp to a JS Date (multiply by 1000 if
it’s in seconds) and only set it when customer.firstSaleAt is falsy and
stripeCustomerInvoices[0] exists—replace the line setting firstSaleAt with a
Date constructed from stripeCustomerInvoices[0].created to accurately reflect
the customer’s first sale.
- Around line 422-427: In the non-useExistingEvents branch ensure totalSales and
totalSaleAmount are initialized so the customer update increments correctly:
inside the else branch that handles manual sales set totalSales = 1 (or
totalSales = saleAmount ? 1 : 1) and totalSaleAmount = saleAmount ?? 0 (or
saleAmount if provided, otherwise 0) so the customer update block that
increments sales/saleAmount (variables totalSales and totalSaleAmount) reflects
the manual sale; adjust the same symbols (totalSales, totalSaleAmount,
saleAmount, useExistingEvents) used in the existing link update logic to keep
counts consistent.
In `@apps/web/lib/api/customers/get-customer-stripe-invoices.ts`:
- Around line 4-6: The current stripe client instantiation spreads
{(process.env.VERCEL_ENV && { mode: "live" })} which treats any truthy
VERCEL_ENV (including "preview" and "development") as production; update the
condition in the stripeAppClient call to only set mode: "live" when
process.env.VERCEL_ENV === "production" so that stripeAppClient(...) uses live
mode strictly in production (refer to the stripe variable and stripeAppClient
call and the process.env.VERCEL_ENV check).
- Around line 15-23: The call to stripe.invoices.list(...) silently returns only
10 invoices by default; update the call in get-customer-stripe-invoices (the
stripe.invoices.list invocation using stripeCustomerId and stripeConnectId) to
fetch all needed invoices by either adding a limit: 100 parameter to the request
object or replace the single-page call with the SDK's auto-pagination (e.g., use
the autoPagingIterator/autoPagingToArray on stripe.invoices.list with the same
customer and stripeAccount options) so you don't truncate results for customers
with >10 paid invoices.
In `@apps/web/scripts/programs/backfill-reuse-commission.ts`:
- Around line 317-321: The commission creation currently uses Promise.allSettled
on commissionsToCreate with createPartnerCommission but never inspects results,
so failures are silently ignored; change this to capture the settled results,
iterate over them to log any rejected items (include the commission input and
error from the rejected result) and then bail out (throw or process.exit(1)) if
any creation failed before proceeding to the workflow/aggregation steps;
alternatively use Promise.all and allow a rejection to be caught and logged, but
ensure the logic references commissionsToCreate and createPartnerCommission and
prevents further execution when there are errors.
- Around line 95-99: The parsed click payload can end up with timestamp: "" when
existingClickEvent is undefined, causing new Date("") → Invalid Date and
persisting as clickedAt; update the merge that builds clickEventData (symbols:
existingClickEvent, newClickAttributes, recordClickZodSchema, clickEventData) to
guard the timestamp value before parsing and provide a sensible fallback (e.g.
omit timestamp, use current time, or use the customer's createdAt) so
recordClickZodSchema never receives an empty string; ensure the chosen fallback
is normalized to a valid ISO/timestamp type accepted by recordClickZodSchema and
persisted as clickedAt.
- Around line 122-140: leadEventTimestamp is only set inside the commissionType
=== "lead" branch, so when processing sales the original lead timestamp is lost
and lastLeadAt falls back to now; ensure you capture the lead event timestamp
whenever you record a lead (e.g., wherever you push to Tinybird or increment
leads and when you push a sale commission into commissionsToCreate) by assigning
leadEventTimestamp = new Date(leadEventData.timestamp + "Z") (or similar UTC
parsing) at the same place you record the lead event; update the code paths that
create sale commissions (and any other path that records a lead, see symbols
commissionType, leadEventData, commissionsToCreate, and leadEventTimestamp) to
set this timestamp so lastLeadAt uses the original event time.
- Line 132: Replace the fragile pattern of appending "Z" directly to timestamps
by adding a helper function (e.g., toUTCDate) that normalizes the incoming
string: only append "Z" when the string neither endsWith("Z") nor contains a
timezone offset ("+"), construct a Date from the normalized string, and throw on
NaN to surface invalid timestamps; then replace all occurrences of new
Date(leadEventData.timestamp + "Z") (and other new Date(... + "Z") uses in this
file) with toUTCDate(leadEventData.timestamp) (or the appropriate variable) to
make timestamp parsing consistent and robust.
- Line 54: tbEventsToRecord is currently populated by calling recordClickZod,
recordLeadWithTimestamp, and recordSaleWithTimestamp immediately (they perform
HTTP calls), which can produce orphaned Tinybird events if the subsequent Prisma
transaction (the $transaction block around the duplicateCustomerId persistence)
fails; change the flow so you collect only event payloads or deferred callables
(e.g., objects describing event type and payload or zero-arg functions that call
the recorder) instead of invoking
recordClickZod/recordLeadWithTimestamp/recordSaleWithTimestamp when pushing into
tbEventsToRecord, then after the await prisma.$transaction(...) resolves
successfully, iterate tbEventsToRecord and invoke the actual recorder functions
to send events to Tinybird; update references to tbEventsToRecord,
recordClickZod, recordLeadWithTimestamp, recordSaleWithTimestamp, and the
transaction block to implement this deferred-send pattern.
In `@apps/web/ui/modals/add-customer-modal.tsx`:
- Around line 291-292: The stripeUrl is always built for the live dashboard;
change the client to use a dashboard URL or mode returned by the search API and
construct the link accordingly: update the API response to include either a full
dashboardUrl or a stripeMode/test flag, then in add-customer-modal.tsx (where
stripeUrl is assigned) check for the returned dashboardUrl first and use it if
present, otherwise use the mode flag to prefix "/test/" when stripeMode ===
"test" (fall back to the current live URL if neither is available). Ensure the
variable stripeUrl is set from these fields so test-mode customers open
https://dashboard.stripe.com/test/customers/{id} while live customers use
https://dashboard.stripe.com/customers/{id}.
- Around line 89-106: When the modal reopens we currently reset several
Stripe-related states but forget to clear stripeSearchLoading, causing the
Search button to remain stuck in loading if a request was in-flight; inside the
useEffect that checks "if (showModal && !prevShowModal.current)" (the block that
calls setHasStripeCustomerId, setShowStripeImport, setStripeSearchEmail,
setStripeSearchResults, setStripeSearchError, and reset(...)), also call the
setter to clear loading (i.e., setStripeSearchLoading(false)) so
stripeSearchLoading is reset on modal open and the Search button is not
permanently stuck.
---
Outside diff comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 261-308: The submitDisabledMessage useMemo incorrectly treats a
fetch failure as "no invoices" because it doesn't check stripeInvoicesError and
doesn't include it in the dependency list; update the logic inside
submitDisabledMessage to return a clear error tooltip (e.g., "Error loading
Stripe invoices. Please try again.") when stripeInvoicesError is truthy (before
checking stripeInvoices.length) and add stripeInvoicesError to the useMemo
dependency array so the memo recomputes on fetch failures; locate references to
submitDisabledMessage, stripeInvoices, stripeInvoicesData, and
stripeInvoicesError to implement this change.
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 289-306: The bug is that lastLeadAt is only set inside the
commissionType === "lead" branch, so when commissionType === "sale" the link
gets stamped with the current time instead of the actual lead event date; update
the sale path (the branch handling commissionType === "sale" where a lead event
is always recorded with finalLeadEventDate/leadEventData) to assign lastLeadAt =
new Date(finalLeadEventDate) (or new Date(leadEventData.timestamp) if that
variable is used there) so lastLeadAt is populated with the actual event
timestamp before the link stats update.
---
Duplicate comments:
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Line 225: The currency is hardcoded to "usd" — update the
StripeCustomerInvoiceSchema to include a currency field and replace the
hardcoded value with the invoice.currency value where commissions are built (in
create-manual-commission.ts around the code that constructs the invoice
object/commission payload); ensure any downstream usage (e.g., Tinybird sale
event payload) consumes the new currency field instead of assuming "usd".
---
Nitpick comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 157-170: Move the eventSourceOptions constant out of the React
component and into module scope so it isn't re-created on every render: locate
the eventSourceOptions declaration in create-commission-sheet.tsx and cut it to
the top-level of the module (above the component function, e.g. above
CreateCommissionSheet or the default-exported component). Keep it as a const
and, if your component or other modules expect a specific shape, add a concise
type annotation (e.g. an array of option objects) or export it if used
elsewhere; update any local references to use the module-scoped
eventSourceOptions.
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 353-365: The current use of Promise.allSettled for
commissionsToCreate hides failures from createPartnerCommission and returns
success even when some creations failed; update the block that calls
commissionsToCreate.map(...) so that either (a) you use await
Promise.all(commissionsToCreate.map(c => createPartnerCommission(c))) to let any
rejection propagate, or (b) keep Promise.allSettled but inspect the results
array and throw a consolidated error if any result.status === "rejected"
(include the rejection reasons). Make the change around the commissionsToCreate
variable and createPartnerCommission calls and ensure the thrown error includes
identifying info from the rejected results so the caller sees the failure.
- Around line 354-365: Replace raw console.log calls that print objects (calls
printing prettyPrint(tbRes) and prettyPrint(commissionsToCreate) as well as the
console.log before Promise.allSettled) with structured logging or remove them:
use the existing app logger (e.g., processLogger.info/debug) instead of
console.log, avoid printing full objects (sanitize or log only non-PII fields
like counts or IDs), and guard detailed debug output behind an environment check
(e.g., isDevelopment) so createPartnerCommission, commissionsToCreate, and tbRes
are not dumped in production.
In `@apps/web/scripts/programs/backfill-reuse-commission.ts`:
- Line 323: The guard if (["lead", "sale"].includes(commissionType)) is
redundant because commissionType is already typed as "lead" | "sale"; remove
that if-statement and unindent/inline its body so the logic runs directly using
the existing commissionType variable (refer to commissionType where it’s
declared and the surrounding backfill logic) to simplify the code and improve
clarity.
- Line 358: Wrap the top-level main() invocation with a promise catch to handle
rejections from main(): call main().catch(err => { console.error("Uncaught error
in main:", err); process.exitCode = 1; }) (or use process.exit(1)) so any
unhandled rejection logs the error and sets a non-zero exit code; update the
existing top-level main() call to this pattern referencing the main() function.
- Around line 231-247: The code currently spreads ...customer into
tx.customer.create (in the block creating the duplicate using
duplicateCustomerId, link, clickEventData, recordSaleEvents, totalSales,
totalSaleAmount), which copies managed fields like createdAt/updatedAt and may
pull in future non-column fields; instead, replace the spread with an explicit
list of allowed customer properties to carry over (e.g., name, email, phone,
address, externalId — whatever your domain uses) or explicitly strip managed
fields first (const { createdAt, updatedAt, ...safeCustomer } = customer) and
use safeCustomer when calling tx.customer.create; keep id: duplicateCustomerId,
linkId: link.id, programId: link.programId, partnerId: link.partnerId, clickId:
clickEventData.click_id, clickedAt: new Date(clickEventData.timestamp), country
logic, and the conditional sales/saleAmount bits intact.
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
apps/web/lib/actions/partners/create-manual-commission.ts (1)
352-365:Promise.allSettledresults for commission creation are not inspected — failures are silently swallowed.Both the Tinybird event recording (line 353) and commission creation (line 357) use
Promise.allSettledbut never check the settled results. If individual commissions fail (e.g., due to duplicateinvoiceId), the user gets a success response while some commissions were not created.Consider logging or throwing on rejected results:
Proposed enhancement
- await Promise.allSettled( - commissionsToCreate.map((commission) => - createPartnerCommission(commission), - ), - ); + const commissionResults = await Promise.allSettled( + commissionsToCreate.map((commission) => + createPartnerCommission(commission), + ), + ); + + const failedCommissions = commissionResults.filter( + (r) => r.status === "rejected", + ); + if (failedCommissions.length > 0) { + console.error( + `Failed to create ${failedCommissions.length}/${commissionsToCreate.length} commissions:`, + prettyPrint(failedCommissions.map((r) => r.reason)), + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-manual-commission.ts` around lines 352 - 365, Promise.allSettled calls for tbEventsToRecord and commissionsToCreate currently ignore their results (tbRes and the commissions Promise.allSettled) so rejected promises are swallowed; update the code that awaits tbEventsToRecord and commissionsToCreate to inspect the Promise.allSettled results (check each result.status === "rejected"), log detailed errors (including reason and the associated item, e.g., commission.invoiceId) and, for commissionsToCreate, surface failure to the caller by throwing an error or returning a non-success response if any commission creation rejected (use createPartnerCommission and commissionsToCreate to correlate failures); ensure Tinybird failures (tbRes) are at least logged with details instead of being ignored.apps/web/scripts/programs/backfill-reuse-commission.ts (2)
37-48: Hardcoded placeholder IDs — consider a CLI-argument or env-var approach.
link_xxxandcus_xxxare sentinel values that must be hand-edited before each run. If someone forgets,findUniqueOrThrowthrows a generic Prisma "not found" error. A small ergonomic improvement: read fromprocess.argvorprocess.envand fail with a clear message.Example
+const LINK_ID = process.argv[2]; +const CUSTOMER_ID = process.argv[3]; +if (!LINK_ID || !CUSTOMER_ID) { + console.error("Usage: npx tsx backfill-reuse-commission.ts <linkId> <customerId>"); + process.exit(1); +} + const link = await prisma.link.findUniqueOrThrow({ - where: { id: "link_xxx" }, + where: { id: LINK_ID }, }); const customer = await prisma.customer.findUniqueOrThrow({ - where: { id: "cus_xxx" }, + where: { id: CUSTOMER_ID }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/programs/backfill-reuse-commission.ts` around lines 37 - 48, Replace the hardcoded sentinel IDs used in main() (the prisma.link.findUniqueOrThrow call with id "link_xxx" and the prisma.customer.findUniqueOrThrow call with id "cus_xxx") by reading the linkId and customerId from CLI args (process.argv) or env vars (process.env); validate that both values are present and, if not, throw or exit with a clear, descriptive error message before calling Prisma so the runtime error is explicit and actionable. Ensure you update the code paths that reference linkId/customerId to use the variables you read and validated.
231-247: Spreading the full Prisma model intocreateis brittle.
...customercopies every scalar field from the original record (includingcreatedAt,updatedAt, and any future auto-managed columns). While specific fields are overridden afterward, any new column added to theCustomermodel will silently propagate without review. Consider using an explicit pick of the fields you intend to carry over, or at least destructuring out the fields that should not be copied (id,createdAt,updatedAt).Example approach
+ const { id: _id, createdAt: _ca, updatedAt: _ua, ...customerFields } = customer; return await tx.customer.create({ data: { - ...customer, + ...customerFields, id: duplicateCustomerId,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/programs/backfill-reuse-commission.ts` around lines 231 - 247, The current tx.customer.create call spreads the entire customer object (via ...customer), which can unintentionally copy auto-managed or future fields; instead, explicitly pick or filter fields to carry over: destructure the original customer to remove id, createdAt, updatedAt (and any other auto-managed fields) and build a new data object using only the allowed fields plus the overrides (use duplicateCustomerId, link.id, link.programId, link.partnerId, clickEventData values, and conditionally include sales/saleAmount when recordSaleEvents is true) before passing it to tx.customer.create to avoid brittle implicit field propagation.apps/web/ui/modals/add-customer-modal.tsx (1)
146-163:externalIddefaults to Stripe email/ID, which may not match the user's own database identifier.
setValue("externalId", customer.email ?? customer.id, ...)pre-fills the required "External ID" field with the Stripe customer email or Stripe ID. SinceexternalIdis described as "the customer's unique identifier in your database," auto-filling it with a Stripe value could lead to mismatches if the user doesn't notice and correct it. Consider leaving it empty (or adding a visual hint) so the user consciously provides their own ID.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/add-customer-modal.tsx` around lines 146 - 163, The onSelectStripeCustomer handler currently auto-fills the "externalId" with Stripe data via setValue("externalId", customer.email ?? customer.id, ...), which can incorrectly populate the app's own unique identifier; update onSelectStripeCustomer to not pre-fill externalId (use null/empty) or explicitly clear it (setValue("externalId", null, { shouldValidate: true })) and instead surface a UI hint/placeholder informing the user to enter their database's customer ID; keep the other setValue calls (name, email, stripeCustomerId, country) and update any toast/message if needed to reflect that externalId must be provided by the user.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/app/`(ee)/api/customers/search-stripe/route.ts:
- Around line 28-36: The Stripe search string is vulnerable because the
user-controlled variable search is interpolated directly into
stripe.customers.search; sanitize/escape search before building the query by
replacing backslashes and double quotes (e.g., escape "\" -> "\\", "\"" ->
"\\\"") and strip/control characters as needed, then use the escaped value when
constructing the query passed to stripe.customers.search so that the query DSL
cannot be injected or altered.
- Around line 9-11: The stripe client is being put into live mode whenever
VERCEL_ENV is set (causing live mode on non-production Vercel previews); change
the condition in the stripeAppClient call so it only enables { mode: "live" }
when process.env.VERCEL_ENV === "production" (or another explicit production
check), e.g. replace ...(process.env.VERCEL_ENV && { mode: "live" }) with a
guard that checks equality to "production" so stripeAppClient is only created in
live mode in production.
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 49-67: The fetcherStripeInvoices function currently treats any 400
response as "noStripeCustomerId"; change the logic to only set
noStripeCustomerId when the API returns a specific error code in the response
body (e.g., check body?.error?.code === '<SPECIFIC_NO_CUSTOMER_CODE>' or
similar), and for other 400 errors throw with the returned error message. Update
the error-checking branch in fetcherStripeInvoices to safely read
body?.error?.code and body?.error?.message, return { invoices: [],
noStripeCustomerId: true, message } only when the code matches the explicit
sentinel, otherwise throw new Error(body?.error?.message ?? 'Failed to load
invoices').
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 186-187: The code sets lastConversionAt to
stripeCustomerInvoices[0].created while stripeCustomerInvoices is sorted
ascending, so it uses the oldest invoice; change lastConversionAt to use the
most recent invoice instead (e.g., set it from
stripeCustomerInvoices[stripeCustomerInvoices.length - 1].created) while keeping
lastLeadAt as the earliest if intended; update the assignment that references
stripeCustomerInvoices[0] for lastConversionAt to reference the last element of
stripeCustomerInvoices.
- Around line 216-228: The saleEventData currently hardcodes currency: "usd"
which is incorrect for non-USD invoices; update the mapping that builds
saleEventData to use the invoice's actual currency (e.g., invoice.currency)
instead of the literal "usd", and ensure saleEventSchemaTBWithTimestamp's input
validation accepts that value (normalize case if your schema expects
lowercase/ISO codes). Locate the construction in create-manual-commission where
saleEventData is created (references: saleEventData,
saleEventSchemaTBWithTimestamp, generatedClickEvent, nanoid) and replace the
hardcoded string with the invoice field and adjust/extend the schema if needed
to validate multiple currency codes.
- Around line 412-430: In the prisma.customer.update block adjust the increments
and firstSaleAt logic: for sales and saleAmount use the same logic as the link
update (increment sales by saleAmount ? 1 : totalSales and increment saleAmount
by saleAmount ? saleAmount : totalSaleAmount) so manually created non-Stripe
commissions actually update customer totals; and set firstSaleAt to the real
event/invoice timestamp when useExistingEvents is true (e.g., use the
invoice/event createdAt or invoiceTimestamp variable) instead of always new
Date(), falling back to new Date() only when no existing event timestamp is
available.
In `@apps/web/lib/api/customers/get-customer-stripe-invoices.ts`:
- Around line 15-23: stripe.invoices.list(...) is currently called without a
limit so Stripe will return only 10 invoices by default; update the call in
get-customer-stripe-invoices to request all paid invoices by either adding an
explicit limit (e.g., limit: 100) and handling pagination or, better, using
Stripe's auto-paging helpers to collect all pages for the given customer and
stripeConnectId; adjust the logic around the returned data variable to aggregate
invoices from all pages rather than assuming a single-page response.
- Around line 4-6: The current stripe client instantiation uses
process.env.VERCEL_ENV truthiness which is true for "production", "preview", and
"development", causing non-production deployments to use live mode; change the
condition passed to stripeAppClient to explicitly check for production (e.g.,
process.env.VERCEL_ENV === "production" or process.env.NODE_ENV ===
"production") so that stripeAppClient({... mode: "live" }) is only used in true
production. Update the stripe variable creation that calls stripeAppClient to
use this explicit check.
In `@apps/web/scripts/programs/backfill-reuse-commission.ts`:
- Around line 122-140: leadEventTimestamp is only assigned inside the if
(commissionType === "lead") block, so when commissionType === "sale" the
original lead timestamp is never preserved; move the assignment of
leadEventTimestamp = new Date(leadEventData.timestamp + "Z") out of the
commissionType guard (or set it unconditionally wherever leadEventData is
available) so leadEventTimestamp is always populated for later link stats
updates (referencing variables commissionType, leadEventTimestamp, and
leadEventData).
- Around line 317-321: Promise.allSettled on
commissionsToCreate.map(createPartnerCommission) swallows failures; capture its
results (e.g., const results = await Promise.allSettled(...)) and inspect for
any entries with status === 'rejected' or non-OK values from
createPartnerCommission, log the errors with context and abort (throw or
process.exit(1)) instead of continuing to run the downstream
workflow/aggregation steps; update the block that calls Promise.allSettled and
ensure subsequent workflow/aggregation triggering only runs when all creations
succeeded.
- Line 54: tbEventsToRecord is being populated with live promises (from
recordClickZod, recordLeadWithTimestamp, recordSaleWithTimestamp) which run
immediately and can produce orphaned Tinybird events if the subsequent Prisma
transaction (prisma.$transaction) fails; change tbEventsToRecord to store
deferred callables (e.g., () => Promise<unknown>) instead of Promise<unknown>,
push functions that call
recordClickZod/recordLeadWithTimestamp/recordSaleWithTimestamp when you would
currently push the promise, and after the transaction completes successfully
invoke and await Promise.all(tbEventsToRecord.map(fn => fn())) to execute
Tinybird writes only after the DB commit.
- Line 132: The code currently appends "Z" directly to timestamps (e.g.,
createdAt: new Date(leadEventData.timestamp + "Z")) which can produce invalid
dates when timestamps already include timezone info; replace those ad-hoc usages
with a single helper (suggested name: toUTCDate) that: accepts a string ts,
appends "Z" only when ts neither endsWith("Z") nor contains "+" (or other
timezone offset), constructs a Date, validates with isNaN(date.getTime()) and
throws a clear error on invalid input; update all places that currently do
string-concatenation (e.g., assignments referencing createdAt and any usages of
leadEventData.timestamp or other timestamp fields around the existing
occurrences) to call toUTCDate(...) so timestamp parsing is consistent across
the script.
- Around line 91-103: The bug is that when existingClickEvent is undefined the
parsed click event gets timestamp "" which later becomes new Date("") → Invalid
Date; fix by ensuring a valid timestamp is provided when composing
clickEventData (e.g., add timestamp: existingClickEvent?.timestamp ?? new
Date().toISOString() to the merged input before calling
recordClickZodSchema.parse), referencing newClickAttributes, clickEventData,
existingClickEvent, and recordClickZodSchema so the parsed object never contains
an empty string timestamp and downstream code (e.g., where clickedAt is set)
receives a valid date string.
In `@apps/web/ui/modals/add-customer-modal.tsx`:
- Around line 89-106: The modal reset block in the useEffect (triggered when
showModal transitions from false to true) misses resetting stripeSearchLoading,
so reopen can leave the "Search" button stuck; update the reset block inside the
useEffect that checks prevShowModal.current to call
setStripeSearchLoading(false) along with setHasStripeCustomerId,
setShowStripeImport, setStripeSearchEmail, setStripeSearchResults,
setStripeSearchError, and the form reset (reset({ ... })) so all Stripe-related
state including stripeSearchLoading is cleared when the modal opens.
- Around line 291-292: The Stripe dashboard URL currently always uses the live
path; update the logic that builds stripeUrl (where stripeUrl =
`https://dashboard.stripe.com/customers/${customer.id}`) to include the test
prefix when appropriate by checking a livemode flag (e.g., use customer.livemode
or a new field returned by the search API) and constructing
`https://dashboard.stripe.com/${livemode ? '' :
'test/'}customers/${customer.id}`; if the search API does not return livemode,
modify the search endpoint to include a livemode boolean so the UI can decide
which dashboard path to use.
---
Nitpick comments:
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 352-365: Promise.allSettled calls for tbEventsToRecord and
commissionsToCreate currently ignore their results (tbRes and the commissions
Promise.allSettled) so rejected promises are swallowed; update the code that
awaits tbEventsToRecord and commissionsToCreate to inspect the
Promise.allSettled results (check each result.status === "rejected"), log
detailed errors (including reason and the associated item, e.g.,
commission.invoiceId) and, for commissionsToCreate, surface failure to the
caller by throwing an error or returning a non-success response if any
commission creation rejected (use createPartnerCommission and
commissionsToCreate to correlate failures); ensure Tinybird failures (tbRes) are
at least logged with details instead of being ignored.
In `@apps/web/scripts/programs/backfill-reuse-commission.ts`:
- Around line 37-48: Replace the hardcoded sentinel IDs used in main() (the
prisma.link.findUniqueOrThrow call with id "link_xxx" and the
prisma.customer.findUniqueOrThrow call with id "cus_xxx") by reading the linkId
and customerId from CLI args (process.argv) or env vars (process.env); validate
that both values are present and, if not, throw or exit with a clear,
descriptive error message before calling Prisma so the runtime error is explicit
and actionable. Ensure you update the code paths that reference
linkId/customerId to use the variables you read and validated.
- Around line 231-247: The current tx.customer.create call spreads the entire
customer object (via ...customer), which can unintentionally copy auto-managed
or future fields; instead, explicitly pick or filter fields to carry over:
destructure the original customer to remove id, createdAt, updatedAt (and any
other auto-managed fields) and build a new data object using only the allowed
fields plus the overrides (use duplicateCustomerId, link.id, link.programId,
link.partnerId, clickEventData values, and conditionally include
sales/saleAmount when recordSaleEvents is true) before passing it to
tx.customer.create to avoid brittle implicit field propagation.
In `@apps/web/ui/modals/add-customer-modal.tsx`:
- Around line 146-163: The onSelectStripeCustomer handler currently auto-fills
the "externalId" with Stripe data via setValue("externalId", customer.email ??
customer.id, ...), which can incorrectly populate the app's own unique
identifier; update onSelectStripeCustomer to not pre-fill externalId (use
null/empty) or explicitly clear it (setValue("externalId", null, {
shouldValidate: true })) and instead surface a UI hint/placeholder informing the
user to enter their database's customer ID; keep the other setValue calls (name,
email, stripeCustomerId, country) and update any toast/message if needed to
reflect that externalId must be provided by the user.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx (1)
257-304:submitDisabledMessagereturnsfalse(notundefined) when valid — confirmdisabledTooltipaccepts falsy values.Line 292 returns
falseas the "not disabled" sentinel. At line 984, this is passed todisabledTooltip. If the Button component checks truthiness (which is typical),falseworks fine. But mixingstring | false | undefinedin the return type is slightly unusual.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx around lines 257 - 304, submitDisabledMessage currently returns false as the "not disabled" sentinel which mixes types (string | false | undefined); change the final return from false to undefined so the hook consistently returns string | undefined, and update any callers (eg. where submitDisabledMessage is passed to disabledTooltip on the Button) to expect undefined for "no tooltip" (no runtime change if the Button only checks falsiness). Locate the submitDisabledMessage useMemo and replace the return false with return undefined and ensure any typing for disabledTooltip/props accepts undefined instead of false.apps/web/ui/modals/add-customer-modal.tsx (1)
37-47: Potential crash ingetCustomerInitialswhennameandnull.
StripeCustomerSchemadefinesnameasz.string().nullable(). If both arenull, the expressioncustomer.name || customer.email || customer.idresolves tocustomer.id(a string), sorawis safe. However, ifcustomer.idis ever an empty string (unlikely from Stripe, but defensively),raw.slice(0, 2)would return"", andraw.trim().split(...)would produce[""]filtered to[], falling through to"".slice(0, 2)→"". The initials circle would render empty.This is unlikely with real Stripe data but worth noting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/add-customer-modal.tsx` around lines 37 - 47, The getCustomerInitials function can return an empty string if customer.name, customer.email, and customer.id are empty/blank; update getCustomerInitials to defensively ensure a non-empty fallback (e.g., after computing raw from customer.name || customer.email || customer.id, check if raw.trim() is empty and if so use a hardcoded placeholder like "??" or a stable fallback derived from customer.id) and then continue computing initials from that guaranteed non-empty value; modify the function getCustomerInitials (and any helper usage) to apply this guard so the rendered initials never end up empty.
🤖 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/lib/actions/partners/create-manual-commission.ts`:
- Around line 357-366: The commission-creation block uses Promise.allSettled
which silently ignores failures; change the logic around
commissionsToCreate/createPartnerCommission so failures are surfaced and cause
the action to fail (either replace Promise.allSettled with Promise.all on
commissionsToCreate.map(c => createPartnerCommission(c)) to fail-fast, or await
Promise.allSettled and then check the settled results for any rejected entries
and throw a descriptive error including the rejected reasons and the related
commission identifiers); keep Tinybird event recording as allSettled/ignored but
ensure the createPartnerCommission error handling surfaces and prevents
returning success on partial commission creation.
- Around line 431-433: The current assignment of firstSaleAt uses
lastConversionAt (which is supposed to be the latest invoice) and will break
once lastConversionAt is fixed to be the latest invoice; introduce a separate
variable (e.g., earliestInvoiceAt or firstInvoiceAt) that captures the earliest
invoice date in both branches (the useExistingEvents path already has lastLeadAt
as the earliest), set that variable where invoices are computed, and change the
firstSaleAt ternary to use this new earliestInvoiceAt (instead of
lastConversionAt) while keeping lastConversionAt as the latest invoice
timestamp.
- Around line 157-259: The server currently processes useExistingEvents
regardless of commissionType, which can create sale events/commissions even when
commissionType === "lead"; add a guard at the start of the useExistingEvents
branch to validate commissionType === "sale" (or throw a clear error) before
generating saleEventData/recordSaleWithTimestamp and pushing to
commissionsToCreate/totalSaleAmount, so only sale commissions are imported when
useExistingEvents is true.
---
Duplicate comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 45-63: The current fetcherStripeInvoices function treats any 400
response with a message as a missing Stripe customer (setting
noStripeCustomerId), which conflates unrelated client errors; update
fetcherStripeInvoices to distinguish the specific API error (inspect
body.error.code or a specific identifier instead of unconditionally using
res.status === 400 && body?.error?.message) and only set noStripeCustomerId when
that specific error code/message matches the known "no stripe customer id"
condition; for other 400s return invoices:[] with message set to the API message
or throw a descriptive Error so callers can handle non-Stripe-ID 400s
appropriately.
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 189-190: The code sets lastConversionAt from
stripeCustomerInvoices[0] after sorting invoices ascending, so it picks the
oldest invoice; change it to use the most recent invoice (the last element) when
assigning lastConversionAt. Locate the block that assigns lastLeadAt and
lastConversionAt from stripeCustomerInvoices and update lastConversionAt to
reference stripeCustomerInvoices[stripeCustomerInvoices.length - 1] (or the
equivalent last item accessor) while keeping lastLeadAt as currently intended.
- Line 228: The currency is hardcoded to "usd" in the payload — replace the
literal with the actual Stripe invoice currency by reading it from the invoice
object (preferably invoice.currency, falling back to invoice.metadata?.currency)
and normalizing (e.g., toLowerCase()) before using; update the code where
currency: "usd" is set in the create-manual-commission logic (look for the
currency assignment inside the createManualCommission /
createManualCommissionAction block) to use that derived invoiceCurrency variable
and default to "usd" only if no invoice currency is available.
In `@apps/web/lib/api/customers/get-customer-stripe-invoices.ts`:
- Around line 4-6: The current truthiness check on process.env.VERCEL_ENV
incorrectly enables live Stripe for any non-empty value; update the check used
when constructing stripeAppClient so live mode is enabled only when VERCEL_ENV
equals the production value (e.g., "production") — locate the stripeAppClient
instantiation and replace the generic truthy guard (process.env.VERCEL_ENV) with
an explicit equality check (process.env.VERCEL_ENV === "production") or another
explicit production-only predicate so preview/development environments do not
set mode: "live".
In `@apps/web/ui/modals/add-customer-modal.tsx`:
- Around line 90-107: When the modal reopens the code resets many Stripe-related
states but forgets to reset stripeSearchLoading, causing the Search button to
stay in loading state; update the useEffect block (the one that checks if
(showModal && !prevShowModal.current)) to call setStripeSearchLoading(false)
alongside setStripeSearchError(null), setStripeSearchResults(null),
setHasStripeCustomerId(false), setShowStripeImport(false), and
setStripeSearchEmail("") so the loading state is cleared when the modal is
reopened.
- Around line 289-293: The Stripe dashboard link always points to the live
dashboard; update the link construction inside the stripeSearchResults.map so
stripeUrl includes "/test" when the Stripe customer is in test mode (use
customer.livemode to detect this). Replace the current stripeUrl =
`https://dashboard.stripe.com/customers/${customer.id}` with logic that prepends
"/test" when customer.livemode === false (e.g.
`https://dashboard.stripe.com${customer.livemode ? '' :
'/test'}/customers/${customer.id}`) so test-mode workspaces open the test
dashboard.
---
Nitpick comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx:
- Around line 257-304: submitDisabledMessage currently returns false as the "not
disabled" sentinel which mixes types (string | false | undefined); change the
final return from false to undefined so the hook consistently returns string |
undefined, and update any callers (eg. where submitDisabledMessage is passed to
disabledTooltip on the Button) to expect undefined for "no tooltip" (no runtime
change if the Button only checks falsiness). Locate the submitDisabledMessage
useMemo and replace the return false with return undefined and ensure any typing
for disabledTooltip/props accepts undefined instead of false.
In `@apps/web/ui/modals/add-customer-modal.tsx`:
- Around line 37-47: The getCustomerInitials function can return an empty string
if customer.name, customer.email, and customer.id are empty/blank; update
getCustomerInitials to defensively ensure a non-empty fallback (e.g., after
computing raw from customer.name || customer.email || customer.id, check if
raw.trim() is empty and if so use a hardcoded placeholder like "??" or a stable
fallback derived from customer.id) and then continue computing initials from
that guaranteed non-empty value; modify the function getCustomerInitials (and
any helper usage) to apply this guard so the rendered initials never end up
empty.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/actions/partners/create-manual-commission.ts (1)
300-317:⚠️ Potential issue | 🟡 Minor
lastLeadAtis never set in thecommissionType === "sale"branch, causinglink.lastLeadAtto fall back tonew Date().A lead event is always pushed to
tbEventsToRecord(line 298) with timestampfinalLeadEventDate, butlastLeadAtis only assigned inside thecommissionType === "lead"block (line 316). For a sale commission,link.lastLeadAtis therefore updated to the current wall-clock time (line 403) rather than the actual event date.🐛 Proposed fix
+ // Track the lead event timestamp for link stats update (applies to all commission types) + lastLeadAt = new Date(leadEventData.timestamp); + if (commissionType === "lead") { commissionsToCreate.push({ event: "lead" as const, // ... }); - // Track the lead event timestamp for link stats update - lastLeadAt = new Date(leadEventData.timestamp); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/create-manual-commission.ts` around lines 300 - 317, The bug is that lastLeadAt is only set in the commissionType === "lead" branch so when commissionType === "sale" link.lastLeadAt falls back to new Date(); fix by assigning lastLeadAt in the sale branch at the same point you push the lead event into tbEventsToRecord: set lastLeadAt = new Date(finalLeadEventDate) (or new Date(leadEventData.timestamp) if that variable is used there) so the link update uses the actual event timestamp; update the commissionType === "sale" block near where tbEventsToRecord is populated to ensure lastLeadAt is set before link.lastLeadAt is updated.
🤖 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/lib/auth/workspace.ts`:
- Around line 202-205: The Prisma query now returns user.image and
user.defaultWorkspace but the TokenCacheItem Zod schema (tokenCacheItemSchema)
doesn't declare them, causing tokenCache.set() to fail when it calls
tokenCacheItemSchema.parse(token); update the TokenCacheItem shape in
tokenCacheItemSchema to include user.image: z.string().nullable() and
user.defaultWorkspace: z.string().nullable() (keep existing id, name, email,
isMachine and defaultPartnerId/nullish) so the parsed object matches the Prisma
result.
---
Outside diff comments:
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 300-317: The bug is that lastLeadAt is only set in the
commissionType === "lead" branch so when commissionType === "sale"
link.lastLeadAt falls back to new Date(); fix by assigning lastLeadAt in the
sale branch at the same point you push the lead event into tbEventsToRecord: set
lastLeadAt = new Date(finalLeadEventDate) (or new Date(leadEventData.timestamp)
if that variable is used there) so the link update uses the actual event
timestamp; update the commissionType === "sale" block near where
tbEventsToRecord is populated to ensure lastLeadAt is set before link.lastLeadAt
is updated.
---
Duplicate comments:
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 364-373: The commission creation currently uses Promise.allSettled
which hides failures; change the logic around
commissionsToCreate/map(createPartnerCommission) so that any rejected
createPartnerCommission causes the whole operation to surface an error: either
use Promise.all to let the first rejection propagate or run Promise.allSettled
and inspect the results, log each failure (including the commission payload and
error) and then throw an aggregated error if any item failed so the caller can
retry/handle partial commits; refer to createPartnerCommission and
commissionsToCreate to locate and update the failing Promise.allSettled usage.
- Around line 196-197: The code sets lastConversionAt from
stripeCustomerInvoices[0] (the oldest) but it should be the newest: change
assignment of lastConversionAt to use the last element of stripeCustomerInvoices
(e.g., stripeCustomerInvoices[stripeCustomerInvoices.length - 1].createdAt) so
lastConversionAt truly reflects the most recent invoice; also ensure firstSaleAt
computation uses lastLeadAt (which already holds the earliest invoice date)
rather than lastConversionAt, or introduce a distinct firstInvoiceAt variable if
you prefer explicit naming, and update references accordingly (symbols:
lastConversionAt, lastLeadAt, firstSaleAt, stripeCustomerInvoices).
- Line 235: The code currently hardcodes currency: "usd" for Stripe invoice sale
events in the create-manual-commission flow; replace the hardcoded string with
the actual Stripe invoice currency (e.g., use invoice.currency or
event.data.object.currency) and fall back to a sensible default only if that
field is missing (for example workspace.defaultCurrency or throw/skip). Update
the place where the commission object is built (look for currency: "usd" in the
create-manual-commission logic) to use the invoice's currency value, ensuring it
is normalized (lowercase) and validated before storing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements