Shopify discount#3804
Merged
Merged
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:
📝 WalkthroughWalkthroughRefactors discount flows from Stripe-only to a provider-based system (Stripe + Shopify): adds provider registry and implementations, provider-aware create/delete flows and errors, Shopify Admin GraphQL client, Prisma enum/field and schema updates, webhook discount-code attribution, and updates API routes, cron jobs, UI, and types to use providers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Admin/Client
participant API as API Route
participant Registry as getDiscountProvider
participant Provider as Discount Provider
participant External as External System (Stripe/Shopify)
participant DB as Database
Client->>API: POST /api/discount-codes { workspace, partner, link, discount, code }
API->>Registry: getDiscountProvider(discount.provider)
Registry-->>API: Provider instance
API->>Provider: createDiscountCode({ workspace, partner, link, discount, code })
Provider->>Provider: assertDiscountIntegrationAvailable(workspace)
alt Integration available
Provider->>External: create code (REST/GraphQL)
External-->>Provider: success / retry on duplicate
Provider->>DB: persist discountCode
DB-->>Provider: persisted record
Provider-->>API: discount code created
API-->>Client: 200 OK
else Integration missing
Provider-->>API: DiscountIntegrationNotAvailableError
API-->>Client: logged skip / early return
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
…ted utility functions
…n logic - Introduced a new discount provider abstraction to support multiple discount providers (Stripe and Shopify). - Refactored the `createDiscountAction` to utilize the new provider system for coupon management. - Updated the discount deletion logic to use a batch job enqueueing system. - Enhanced the discount schema to include a provider field for better flexibility. - Cleaned up unused imports and streamlined the discount-related utility functions.
- Removed the `shopifyStoreId` from the credentials in the PATCH route. - Updated the `createDiscountAction` to include a `provider` field and handle null coupon IDs. - Introduced a new `shopifyAdminGraphql` function for better error handling in Shopify API calls. - Added integration credentials schema for Shopify, ensuring proper access token management. - Enhanced the discount creation logic to support one-time use discounts and improved validation. - Updated the UI to allow selection between Stripe and Shopify as discount providers.
- Removed direct dependencies on Stripe integration settings in discount code creation logic. - Introduced a unified discount provider system to streamline discount code generation for both Stripe and Shopify. - Enhanced error handling for workspace integration checks and improved validation for discount creation. - Updated the `createDiscountCode` function to accept workspace details directly, simplifying the parameters. - Cleaned up unused imports and optimized the discount-related utility functions.
- Updated the partner API to include discount information in the response. - Refactored the PartnerLinks component to utilize the extended partner schema with discount details. - Improved error handling for discount creation based on partner discount status and workspace integration. - Cleaned up and optimized discount-related UI components and constants for better maintainability.
- Updated the discount code creation and deletion logic to utilize a unified discount provider system for both Stripe and Shopify. - Enhanced the input schema to include a provider field for better flexibility in discount operations. - Removed deprecated functions related to Stripe discount code management and streamlined the discount-related utility functions. - Improved error handling and validation for discount code operations, ensuring better integration with workspace settings.
- Updated the discount code logic across multiple routes to include discount provider details. - Enhanced the `deleteDiscountCodes` function to accept an array of discount codes with associated provider information. - Streamlined the discount code creation process by directly passing workspace details. - Improved the API responses for partners and links to include nested discount provider data for better integration.
- Added try-catch block to handle specific error messages related to Stripe and Shopify connections during discount code disabling. - Updated the `deleteDiscountCodes` function to filter out orphaned discount codes without provider information, improving cleanup logic. - Enhanced logging to provide clearer feedback on skipped discount codes due to missing provider relations.
- Updated the `createDiscountAction` to fetch existing coupons if a `couponId` is provided, enhancing flexibility in discount management. - Removed deprecated `getOrCreateCoupon` methods from Stripe and Shopify discount providers, streamlining the coupon handling process. - Improved error handling and validation for discount creation across different providers.
- Introduced `DiscountIntegrationNotAvailableError` to standardize error handling for missing discount integrations. - Updated discount code creation and deletion routes to utilize the new error class for improved clarity in error responses. - Refactored discount provider logic to assert integration availability before proceeding with discount operations, enhancing robustness.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/lib/integrations/shopify/create-lead.ts (2)
156-173: Redundantprisma.customer.update—programId/partnerIdare already set at create time.Now that the customer is created with
programId: partnerLink.programIdandpartnerId: partnerLink.partnerIdat lines 77–78, the subsequentprisma.customer.updateinsidePromise.allSettledwrites the same values back to the same row (sincelinkandpartnerLinkreference the same Link record). This is a wasted write per lead.Consider dropping just the
prisma.customer.updatewhile keeping thesyncPartnerLinksStatscall:♻️ Proposed simplification
...(link.programId && link.partnerId ? [ syncPartnerLinksStats({ partnerId: link.partnerId, programId: link.programId, eventType: "lead", }), - prisma.customer.update({ - where: { - id: customer.id, - }, - data: { - programId: link.programId, - partnerId: link.partnerId, - }, - }), ] : []),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/shopify/create-lead.ts` around lines 156 - 173, The code performs a redundant write: after creating the customer with programId/partnerId set from partnerLink, the Promise.allSettled branch still calls prisma.customer.update to set the same programId/partnerId based on link (which points to the same Link), causing an unnecessary DB write; remove the prisma.customer.update call from the array and keep syncPartnerLinksStats({ partnerId: link.partnerId, programId: link.programId, eventType: "lead" }) so Promise.allSettled only runs syncPartnerLinksStats (and any other non-redundant actions) for the new customer referenced by customer and link/partnerLink.
51-67: Optional: collapse the two link queries into one.
partnerLinkis fetched here just to pullprogramId/partnerIdfor the customer create, then a second roundtrip updates the same record at lines 101–112 (whose resultlinkalready contains those fields plusincludeTags). You could move the linkupdateto run before the customercreateand reuse it, eliminating one DB roundtrip per lead — at the cost of incrementingleads/lastLeadAtslightly earlier in the flow (and relying onupdatethrowingP2025for the missing-link case, or pre-checking with a singlefindUniqueand dropping the duplicate read).Not blocking — current code is correct, just an extra query on a hot path. Same observation applies to
create-sale.ts, which fetches the link insidePromise.allalready; matching that pattern here would also be reasonable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/shopify/create-lead.ts` around lines 51 - 67, Collapse the two DB roundtrips by replacing the separate partnerLink findUnique and the later prisma.link.update with a single prisma.link.update call run before creating the customer and reuse its result (the updated "link" that includes programId, partnerId, includeTags, leads, lastLeadAt); if you prefer to preserve the current not-found behavior, either pre-check with one findUnique and then update using that same record, or run the update and catch Prisma P2025 to throw the existing DubApiError for the missing link. Target the existing identifiers partnerLink, prisma.link.update, and the later code that reads link.includeTags / link.programId so they consume the single updated link object.apps/web/lib/actions/partners/create-discount.ts (1)
51-73: Consider an exhaustive switch onproviderto future-proof against new providers.The current
if/else ifchain silently no-ops for any futureDiscountProviderenum value (no provider-side hook is invoked, no error is raised, and the discount row is still created). Today onlystripeandshopifyexist, so this isn't a bug — but switching to an exhaustiveswitchwith anever-typed default makes TypeScript flag the gap when a new provider is added.♻️ Suggested refactor
- const discountProvider = getDiscountProvider(provider); - - let coupon: (DubDiscountAttributes & { id: string }) | null = null; - - // Fetch existing coupon if couponId is provided otherwise create a new coupon on the discount provider - if (provider === DiscountProvider.stripe) { - if (couponId) { - coupon = await discountProvider.getCoupon({ - couponId, - workspace, - }); - } else { - coupon = await discountProvider.createCoupon({ - workspace, - group, - data: parsedInput, - }); - } - } else if (provider === DiscountProvider.shopify) { - await discountProvider.assertDiscountIntegrationAvailable({ - workspace, - }); - } + const discountProvider = getDiscountProvider(provider); + + let coupon: (DubDiscountAttributes & { id: string }) | null = null; + + switch (provider) { + case DiscountProvider.stripe: + // Fetch existing coupon if couponId is provided, otherwise create a new one + coupon = couponId + ? await discountProvider.getCoupon({ couponId, workspace }) + : await discountProvider.createCoupon({ + workspace, + group, + data: parsedInput, + }); + break; + case DiscountProvider.shopify: + await discountProvider.assertDiscountIntegrationAvailable({ + workspace, + }); + break; + default: { + const _exhaustive: never = provider; + throw new Error(`Unhandled discount provider: ${_exhaustive}`); + } + }🤖 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 51 - 73, Replace the current if/else-if chain that checks provider with an exhaustive switch on the DiscountProvider enum: call getCoupon/createCoupon for DiscountProvider.stripe (using discountProvider, couponId, workspace, group, parsedInput), call assertDiscountIntegrationAvailable for DiscountProvider.shopify, and include a default case typed as never to force a compile-time error when a new DiscountProvider value is added; this ensures the getDiscountProvider/discountProvider logic and coupon handling (coupon variable) are always handled for every enum member.
🤖 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/integrations/shopify/process-order.ts`:
- Around line 137-226: The createPartnerCommission call inside the waitUntil
IIFE can throw and short-circuit the rest of the IIFE so
sendWorkspaceWebhook/sendPartnerPostback never run; wrap the
createPartnerCommission(...) invocation in a try/catch (or use
Promise.resolve(...).catch(...)) inside the IIFE so any error is caught,
optionally log the failure, leave createdCommission as undefined on error, and
then continue to the subsequent Promise.allSettled that calls
sendWorkspaceWebhook and sendPartnerPostback (references: the waitUntil IIFE,
createPartnerCommission, createdCommission, sendWorkspaceWebhook,
sendPartnerPostback, and linkUpdated).
- Around line 92-117: The billing_address schema needs a new country_code field
and the code should use that ISO‑2 code for lookups and storage: add
country_code to orderSchema.billing_address, change the continent lookup to use
COUNTRIES_TO_CONTINENTS[billingAddress.country_code] (falling back to "Unknown"
if the lookup fails), and when creating the Customer in prisma.customer.create
set customer.country to billingAddress.country_code instead of the full name;
update usages around recordFakeClick, billingAddress, COUNTRIES_TO_CONTINENTS,
and prisma.customer.create to reflect this change.
---
Nitpick comments:
In `@apps/web/lib/actions/partners/create-discount.ts`:
- Around line 51-73: Replace the current if/else-if chain that checks provider
with an exhaustive switch on the DiscountProvider enum: call
getCoupon/createCoupon for DiscountProvider.stripe (using discountProvider,
couponId, workspace, group, parsedInput), call
assertDiscountIntegrationAvailable for DiscountProvider.shopify, and include a
default case typed as never to force a compile-time error when a new
DiscountProvider value is added; this ensures the
getDiscountProvider/discountProvider logic and coupon handling (coupon variable)
are always handled for every enum member.
In `@apps/web/lib/integrations/shopify/create-lead.ts`:
- Around line 156-173: The code performs a redundant write: after creating the
customer with programId/partnerId set from partnerLink, the Promise.allSettled
branch still calls prisma.customer.update to set the same programId/partnerId
based on link (which points to the same Link), causing an unnecessary DB write;
remove the prisma.customer.update call from the array and keep
syncPartnerLinksStats({ partnerId: link.partnerId, programId: link.programId,
eventType: "lead" }) so Promise.allSettled only runs syncPartnerLinksStats (and
any other non-redundant actions) for the new customer referenced by customer and
link/partnerLink.
- Around line 51-67: Collapse the two DB roundtrips by replacing the separate
partnerLink findUnique and the later prisma.link.update with a single
prisma.link.update call run before creating the customer and reuse its result
(the updated "link" that includes programId, partnerId, includeTags, leads,
lastLeadAt); if you prefer to preserve the current not-found behavior, either
pre-check with one findUnique and then update using that same record, or run the
update and catch Prisma P2025 to throw the existing DubApiError for the missing
link. Target the existing identifiers partnerLink, prisma.link.update, and the
later code that reads link.includeTags / link.programId so they consume the
single updated link object.
🪄 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: 5b94676a-ed7c-4679-80da-6141ab5622b3
📒 Files selected for processing (4)
apps/web/app/(ee)/api/shopify/integration/webhook/orders-paid.tsapps/web/lib/actions/partners/create-discount.tsapps/web/lib/integrations/shopify/create-lead.tsapps/web/lib/integrations/shopify/process-order.ts
Collaborator
Author
|
@coderabbitai full review please |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
apps/web/.env.example (1)
195-195:⚠️ Potential issue | 🟡 MinorMissing trailing newline (regression).
The file is missing a trailing newline, which violates the POSIX standard and triggers a dotenv-linter warning. This was previously addressed but appears to have regressed.
🧹 Add trailing newline
VERIFF_SHARED_SECRET= +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/.env.example` at line 195, The .env example is missing a trailing newline causing dotenv-linter warnings; update the file so the final line (the VERIFF_SHARED_SECRET= entry) is followed by a newline character at EOF so the file ends with a newline (i.e., add a single blank line after the existing VERIFF_SHARED_SECRET= line).apps/web/lib/discounts/discount-provider-shopify.ts (1)
78-84:⚠️ Potential issue | 🔴 CriticalGrammatically broken error message still present;
createCouponshould likely be a no-op.The error string
"Shopify does not this method."is missing a verb (e.g., "support"). More importantly, sinceadd-edit-discount-sheet.tsxclearscouponIdfor Shopify,createDiscountActionfalls through tocreateCouponand these throws break the create flow end-to-end. Either returnnull(no-op) forcreateCouponor have callers branch on provider so this stub is never invoked.🐛 Suggested fix
const getCoupon = async () => { - throw new Error("Shopify does not this method."); + throw new Error("Shopify provider does not support getCoupon."); }; const createCoupon = async () => { - throw new Error("Shopify does not this method."); + // Shopify creates discount codes per-partner; no global coupon to create. + return null; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/discounts/discount-provider-shopify.ts` around lines 78 - 84, The error messages in getCoupon and createCoupon are grammatically incorrect and createCoupon should not throw (it breaks the Shopify create flow); update the thrown text to a correct message like "Shopify does not support this method." and change createCoupon to be a no-op that returns null (or a resolved Promise<null>) so callers (e.g., createDiscountAction) can continue without error; keep getCoupon behavior as-is or similarly adjust if callers expect null instead of an exception.
🧹 Nitpick comments (9)
apps/web/lib/integrations/shopify/create-lead.ts (1)
156-173: Redundantcustomer.update—programId/partnerIdare already set at creation.Since the customer is now created with
programId: partnerLink.programIdandpartnerId: partnerLink.partnerId(lines 77‑78), andpartnerLinkandlinkare the same DB row (both fetched bylinkId), the subsequentprisma.customer.updatewritinglink.programId/link.partnerIdis a no-op. Consider dropping it from thePromise.allSettledto save a write per Shopify lead.♻️ Proposed simplification
...(link.programId && link.partnerId ? [ syncPartnerLinksStats({ partnerId: link.partnerId, programId: link.programId, eventType: "lead", }), - prisma.customer.update({ - where: { - id: customer.id, - }, - data: { - programId: link.programId, - partnerId: link.partnerId, - }, - }), ] : []),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/shopify/create-lead.ts` around lines 156 - 173, Remove the redundant write: the prisma.customer.update call inside the conditional Promise.allSettled block is a no-op because the customer is created with programId/partnerId from partnerLink (same DB row as link); delete the prisma.customer.update({...}) entry and keep only syncPartnerLinksStats({...}) when link.programId && link.partnerId are present, ensuring you remove references to the update call in create-lead.ts (look for the prisma.customer.update invocation and the surrounding conditional that spreads its array).apps/web/lib/integrations/shopify/admin-graphql.ts (2)
3-11: MisleadinguserErrorsfield name; also setthis.nameon the custom error.In the Shopify Admin GraphQL API,
userErrorsis a well-known concept that specifically refers to mutation-level structured errors returned insidedata.<mutationName>.userErrors(e.g.,discountCodeBasicCreate.userErrors). Here, this property is populated fromjson.errors(top-level GraphQL transport/validation errors), not from mutationuserErrors. Callers readingerr.userErrorswill be confused. Consider renaming toerrors(orgraphqlErrors).Additionally,
this.nameisn't set, so stack traces andinstanceofdebugging will show a generic"Error".♻️ Proposed change
export class ShopifyAdminGraphqlError extends Error { constructor( public code: string, message: string, - public userErrors?: unknown, + public errors?: unknown, ) { super(message); + this.name = "ShopifyAdminGraphqlError"; } }And update the throw site at line 49–53 to pass
json.errorsto the renamed parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/shopify/admin-graphql.ts` around lines 3 - 11, Rename the misleading property on the ShopifyAdminGraphqlError class from userErrors to a clearer name like errors or graphqlErrors and update the constructor signature accordingly (constructor(public code: string, message: string, public errors?: unknown)), also set this.name = 'ShopifyAdminGraphqlError' inside the constructor so stack traces and instanceof work; then update the throw site(s) that call new ShopifyAdminGraphqlError(...) (the place that currently passes json.errors) to pass json.errors into the renamed errors/graphqlErrors parameter.
13-14: TODO: send errors to Axiom.Tracking this so it's not forgotten — the helper is the central choke point for Shopify Admin API failures, so wiring it into Axiom (or whatever observability layer the repo uses) will be valuable for production debugging.
Want me to open a follow-up issue to track adding Axiom logging here, or sketch the wiring once the existing Axiom helper module is identified?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/shopify/admin-graphql.ts` around lines 13 - 14, The TODO "Send the error to Axiom" marks the central Shopify Admin API helper where failures are currently only logged locally; update that helper to call the project's Axiom logging helper (import the existing Axiom client/utility, e.g., axiom or axiomLogger) inside the catch paths of the helper (where the TODO sits) and send a structured event including the error stack/message, the GraphQL operation/name, the shop or store identifier, request payload/variables (redacting secrets), HTTP status/response info, and any retry metadata; keep the existing processLogger.error fallback so local logs remain if Axiom fails.apps/web/app/(ee)/api/cron/partners/deactivate/route.ts (1)
58-60: Drop the redundant identity.map.
discountCodes.map((dc) => dc)is a no-op and can be removed.♻️ Proposed simplification
- const discountCodes = programEnrollments.flatMap(({ discountCodes }) => - discountCodes.map((dc) => dc), - ); + const discountCodes = programEnrollments.flatMap( + ({ discountCodes }) => discountCodes, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/deactivate/route.ts around lines 58 - 60, The current construction creates discountCodes via programEnrollments.flatMap(({ discountCodes }) => discountCodes.map((dc) => dc)) which uses an identity .map that does nothing; replace that expression with programEnrollments.flatMap(({ discountCodes }) => discountCodes) to directly flatten the arrays (variable: discountCodes, source: programEnrollments) and remove the redundant .map call.apps/web/lib/discounts/discount-provider.ts (1)
5-18: Enum values match the registry keys—no mismatch detected.The Prisma
DiscountProviderenum (packages/prisma/schema/discount.prisma) definesstripeandshopifyas unquoted enum values, which correspond to the lowercase string values"stripe"and"shopify". These match the registry keys exactly, so the lookup will not throw a runtime error for valid enum values.♻️ Optional: enforce exhaustiveness at compile time
Adding a
Record<DiscountProvider, …>type annotation would catch at compile time if a new enum value is added without updating the registry—recommended as a safeguard for future maintainability.-const discountProviders = { +const discountProviders: Record<DiscountProvider, typeof stripeDiscountProvider | typeof shopifyDiscountProvider> = { stripe: stripeDiscountProvider, shopify: shopifyDiscountProvider, -} as const; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/discounts/discount-provider.ts` around lines 5 - 18, The registry object discountProviders is currently untyped which misses a compile-time check against the Prisma enum; update its declaration to be typed as a Record<DiscountProvider, typeof stripeDiscountProvider | typeof shopifyDiscountProvider> (or a common provider interface type) so the TS compiler enforces exhaustiveness when DiscountProvider changes, then keep getDiscountProvider and its runtime lookup as-is to preserve the runtime error for unknown keys.apps/web/lib/discounts/delete-discount-code.ts (1)
48-58: Orphan-skip logic is a sensible compromise.Skipping external-provider cleanup when the
discountrelation is null (and surfacing it as aconsole.warnwith the count) is a reasonable approach given the local rows are still removed. If you want stronger ops visibility, consider routing this through your structured logger / error sink rather thanconsole.warnso it's queryable in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/discounts/delete-discount-code.ts` around lines 48 - 58, The console.warn call that logs orphanedCount in delete-discount-code.ts should use the app's structured logger instead of console to make the event queryable; locate the codesWithProvider and orphanedCount logic and replace the console.warn with a structured logger.warn (or logger.warn) call that includes a clear message and metadata { orphanedCount, context: 'deleteDiscountCodes' } and, if no logger is in scope, accept/inject the existing application logger into the deleteDiscountCodes function so you can emit a structured warning rather than using console.warn.apps/web/lib/integrations/shopify/schema.ts (1)
22-26: Makediscount_codesoptional to handle missing field in Shopify webhook payloads.Shopify's Admin API orders/paid webhook can omit the
discount_codesfield when no discount codes are applied. The current schema requires it as a non-optional array, which would cause parsing to fail if the field is absent. Use.optional().default([])to safely handle both cases—omitted or empty array—without losing validation for present values.♻️ Suggested change
- discount_codes: z.array( - z.object({ - code: z.string().describe("The code of the discount."), - }), - ), + discount_codes: z + .array( + z.object({ + code: z.string().describe("The code of the discount."), + }), + ) + .optional() + .default([]),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/integrations/shopify/schema.ts` around lines 22 - 26, The schema currently declares discount_codes as a required array which breaks when Shopify omits that field; update the discount_codes definition in apps/web/lib/integrations/shopify/schema.ts (the z.array(z.object({...})) for discount_codes) to be optional with a default empty array by chaining .optional().default([]) on the z.array so absent or empty discount_codes are handled safely while preserving validation for present entries.apps/web/ui/partners/discounts/add-edit-discount-sheet.tsx (1)
273-301: Consider extracting the inline IIFE for readability.The IIFE wrapping
register("provider", ...)to splice in customonChangeside-effects is functional but a bit hard to read inline within JSX. Extracting it into aproviderRegistrationconst above the JSX would improve readability without changing behavior.♻️ Suggested refactor
+ const providerRegistration = (() => { + const { onChange: onProviderChange, ...rest } = register("provider", { + disabled: Boolean(discount), + }); + return { + ...rest, + onChange: (e: ChangeEvent<HTMLSelectElement>) => { + onProviderChange(e); + if (e.target.value === DiscountProvider.shopify) { + setUseExistingCoupon(false); + setUseStripeTestCouponId(false); + setValue("couponId", ""); + setValue("couponTestId", ""); + setValue("maxDuration", 0); + } else { + setUseExistingCoupon(false); + setUseStripeTestCouponId(false); + setValue( + "maxDuration", + defaultValuesSource.maxDuration === null + ? Infinity + : defaultValuesSource.maxDuration, + ); + } + }, + }; + })();Then in the JSX:
<select ... {...providerRegistration}>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/partners/discounts/add-edit-discount-sheet.tsx` around lines 273 - 301, The inline IIFE that calls register("provider", { disabled: Boolean(discount) }) and injects a custom onChange with side-effects is hard to read in JSX; extract it into a top-level const (e.g., providerRegistration) above the JSX, assign const { onChange: onProviderChange, ...rest } = register("provider", { disabled: Boolean(discount) }); then create onChange that calls onProviderChange(e) and performs the same branches (checking e.target.value === DiscountProvider.shopify, calling setUseExistingCoupon(false), setUseStripeTestCouponId(false), setValue("couponId",""), setValue("couponTestId",""), setValue("maxDuration", 0) or the else branch using defaultValuesSource.maxDuration), and spread providerRegistration into the <select> in place of the IIFE.apps/web/lib/discounts/discount-provider-stripe.ts (1)
19-51: Duplicate "missing Stripe integration" error message.
requireInstalledIntegrationthrows the sameSTRIPE_CONNECTION_REQUIREDmessage for two distinct conditions (nostripeConnectIdvs. noinstalledIntegrationrow). Consider extracting the message into a constant, or differentiating the two paths so debugging which precondition failed is easier in logs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/discounts/discount-provider-stripe.ts` around lines 19 - 51, The function requireInstalledIntegration currently throws the identical STRIPE_CONNECTION_REQUIRED error for two different failure cases; change this by introducing distinct error messages (or a shared constants file) and throw a more specific message for each path: one when workspace.stripeConnectId is missing (e.g., "STRIPE_ACCOUNT_NOT_LINKED") and another when prisma.installedIntegration lookup returns null (e.g., "STRIPE_APP_NOT_INSTALLED"), keeping use of DiscountIntegrationNotAvailableError; update the thrown message strings where requireInstalledIntegration, STRIPE_INTEGRATION_ID, prisma.installedIntegration, and stripeIntegrationSettingsSchema are referenced so logs and debugging can distinguish the two failure reasons.
🤖 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/shopify/integration/callback/route.ts:
- Around line 43-53: The call to installIntegration is persisting plaintext
credentials (credentials.accessToken = body.accessToken); add a clear TODO
comment immediately above this installIntegration invocation noting that
encryption is intentionally deferred and must be implemented in the follow-up
PR, reference the planned encryption work and the unique symbols
(installIntegration, SHOPIFY_INTEGRATION_ID, body.accessToken, credentials) so
reviewers can find and track this hotspot, and if desirable add a short runtime
assertion or guard (e.g., a feature-flag check) to ensure the unencrypted path
cannot be enabled in production until the encryption PR is merged.
- Around line 56-68: The empty catch in the disconnect branch (inside the route
handling block where body.action === "disconnect") swallows all errors from
prisma.installedIntegration.delete; change this to an idempotent deleteMany call
on prisma.installedIntegration.deleteMany with the same filter (userId,
integrationId, projectId) or, if you must keep delete, catch only
Prisma.PrismaClientKnownRequestError and rethrow unless error.code === 'P2025'
so only "record not found" is ignored; update the block around
prisma.installedIntegration.delete to use
prisma.installedIntegration.deleteMany(...) or a narrow catch checking Prisma
error code.
In `@apps/web/lib/discounts/discount-provider-stripe.ts`:
- Around line 22-41: The user-facing error message used in the two
DiscountIntegrationNotAvailableError throws is awkward ("...to create
discount."); update both occurrences so the message reads "...to create a
discount." (search for the throws near workspace.stripeConnectId and the
installation check around
prisma.installedIntegration.findFirst/STRIPE_INTEGRATION_ID) and keep the rest
of the message text unchanged so both error instances are consistent.
In `@apps/web/lib/integrations/shopify/admin-graphql.ts`:
- Around line 27-44: The fetch call that posts to Shopify in admin-graphql.ts
needs a request timeout and must include the response body in non-OK errors: add
an AbortSignal (e.g., use AbortSignal.timeout(TIMEOUT_MS) or create an
AbortController and pass signal into the fetch options) when calling fetch so
the request can't hang indefinitely, and on the error branch that throws
ShopifyAdminGraphqlError, read the response body (preferably try response.json()
then fallback to response.text()) and include that payload in the error
message/details so the thrown ShopifyAdminGraphqlError contains the HTTP status,
statusText and the response body for debugging; update the fetch invocation and
the error-handling block around response and response.ok accordingly
(references: the fetch call variable response and the ShopifyAdminGraphqlError
thrown).
In `@apps/web/lib/integrations/shopify/process-order.ts`:
- Around line 196-224: The payloads are inconsistent: sendWorkspaceWebhook
explicitly sets metadata: null but sendPartnerPostback spreads ...leadEvent
which currently contains metadata: "" — fix by ensuring both consumers receive
null; either initialize leadEvent.metadata = null where leadEvent is constructed
or explicitly override metadata: null in the sendPartnerPostback data object
(inside the sendPartnerPostback call) so the postback uses metadata: null
instead of the empty string.
In `@apps/web/ui/partners/discounts/add-edit-discount-sheet.tsx`:
- Around line 267-308: The label and select for the Discount provider are not
associated for accessibility; add a matching id on the select and htmlFor on the
label (e.g., use "provider" or another unique id) so clicking the label focuses
the select and screen readers link them; keep the existing register("provider",
...) spread and onChange logic (symbols: register, DiscountProvider,
setUseExistingCoupon, setUseStripeTestCouponId, setValue, defaultValuesSource) —
either add the id attribute before/after spreading the register props or include
it in the props returned from register so the id and htmlFor match.
---
Duplicate comments:
In `@apps/web/.env.example`:
- Line 195: The .env example is missing a trailing newline causing dotenv-linter
warnings; update the file so the final line (the VERIFF_SHARED_SECRET= entry) is
followed by a newline character at EOF so the file ends with a newline (i.e.,
add a single blank line after the existing VERIFF_SHARED_SECRET= line).
In `@apps/web/lib/discounts/discount-provider-shopify.ts`:
- Around line 78-84: The error messages in getCoupon and createCoupon are
grammatically incorrect and createCoupon should not throw (it breaks the Shopify
create flow); update the thrown text to a correct message like "Shopify does not
support this method." and change createCoupon to be a no-op that returns null
(or a resolved Promise<null>) so callers (e.g., createDiscountAction) can
continue without error; keep getCoupon behavior as-is or similarly adjust if
callers expect null instead of an exception.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/partners/deactivate/route.ts:
- Around line 58-60: The current construction creates discountCodes via
programEnrollments.flatMap(({ discountCodes }) => discountCodes.map((dc) => dc))
which uses an identity .map that does nothing; replace that expression with
programEnrollments.flatMap(({ discountCodes }) => discountCodes) to directly
flatten the arrays (variable: discountCodes, source: programEnrollments) and
remove the redundant .map call.
In `@apps/web/lib/discounts/delete-discount-code.ts`:
- Around line 48-58: The console.warn call that logs orphanedCount in
delete-discount-code.ts should use the app's structured logger instead of
console to make the event queryable; locate the codesWithProvider and
orphanedCount logic and replace the console.warn with a structured logger.warn
(or logger.warn) call that includes a clear message and metadata {
orphanedCount, context: 'deleteDiscountCodes' } and, if no logger is in scope,
accept/inject the existing application logger into the deleteDiscountCodes
function so you can emit a structured warning rather than using console.warn.
In `@apps/web/lib/discounts/discount-provider-stripe.ts`:
- Around line 19-51: The function requireInstalledIntegration currently throws
the identical STRIPE_CONNECTION_REQUIRED error for two different failure cases;
change this by introducing distinct error messages (or a shared constants file)
and throw a more specific message for each path: one when
workspace.stripeConnectId is missing (e.g., "STRIPE_ACCOUNT_NOT_LINKED") and
another when prisma.installedIntegration lookup returns null (e.g.,
"STRIPE_APP_NOT_INSTALLED"), keeping use of
DiscountIntegrationNotAvailableError; update the thrown message strings where
requireInstalledIntegration, STRIPE_INTEGRATION_ID, prisma.installedIntegration,
and stripeIntegrationSettingsSchema are referenced so logs and debugging can
distinguish the two failure reasons.
In `@apps/web/lib/discounts/discount-provider.ts`:
- Around line 5-18: The registry object discountProviders is currently untyped
which misses a compile-time check against the Prisma enum; update its
declaration to be typed as a Record<DiscountProvider, typeof
stripeDiscountProvider | typeof shopifyDiscountProvider> (or a common provider
interface type) so the TS compiler enforces exhaustiveness when DiscountProvider
changes, then keep getDiscountProvider and its runtime lookup as-is to preserve
the runtime error for unknown keys.
In `@apps/web/lib/integrations/shopify/admin-graphql.ts`:
- Around line 3-11: Rename the misleading property on the
ShopifyAdminGraphqlError class from userErrors to a clearer name like errors or
graphqlErrors and update the constructor signature accordingly
(constructor(public code: string, message: string, public errors?: unknown)),
also set this.name = 'ShopifyAdminGraphqlError' inside the constructor so stack
traces and instanceof work; then update the throw site(s) that call new
ShopifyAdminGraphqlError(...) (the place that currently passes json.errors) to
pass json.errors into the renamed errors/graphqlErrors parameter.
- Around line 13-14: The TODO "Send the error to Axiom" marks the central
Shopify Admin API helper where failures are currently only logged locally;
update that helper to call the project's Axiom logging helper (import the
existing Axiom client/utility, e.g., axiom or axiomLogger) inside the catch
paths of the helper (where the TODO sits) and send a structured event including
the error stack/message, the GraphQL operation/name, the shop or store
identifier, request payload/variables (redacting secrets), HTTP status/response
info, and any retry metadata; keep the existing processLogger.error fallback so
local logs remain if Axiom fails.
In `@apps/web/lib/integrations/shopify/create-lead.ts`:
- Around line 156-173: Remove the redundant write: the prisma.customer.update
call inside the conditional Promise.allSettled block is a no-op because the
customer is created with programId/partnerId from partnerLink (same DB row as
link); delete the prisma.customer.update({...}) entry and keep only
syncPartnerLinksStats({...}) when link.programId && link.partnerId are present,
ensuring you remove references to the update call in create-lead.ts (look for
the prisma.customer.update invocation and the surrounding conditional that
spreads its array).
In `@apps/web/lib/integrations/shopify/schema.ts`:
- Around line 22-26: The schema currently declares discount_codes as a required
array which breaks when Shopify omits that field; update the discount_codes
definition in apps/web/lib/integrations/shopify/schema.ts (the
z.array(z.object({...})) for discount_codes) to be optional with a default empty
array by chaining .optional().default([]) on the z.array so absent or empty
discount_codes are handled safely while preserving validation for present
entries.
In `@apps/web/ui/partners/discounts/add-edit-discount-sheet.tsx`:
- Around line 273-301: The inline IIFE that calls register("provider", {
disabled: Boolean(discount) }) and injects a custom onChange with side-effects
is hard to read in JSX; extract it into a top-level const (e.g.,
providerRegistration) above the JSX, assign const { onChange: onProviderChange,
...rest } = register("provider", { disabled: Boolean(discount) }); then create
onChange that calls onProviderChange(e) and performs the same branches (checking
e.target.value === DiscountProvider.shopify, calling
setUseExistingCoupon(false), setUseStripeTestCouponId(false),
setValue("couponId",""), setValue("couponTestId",""), setValue("maxDuration", 0)
or the else branch using defaultValuesSource.maxDuration), and spread
providerRegistration into the <select> in place of the IIFE.
🪄 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: 51dd5074-aaa2-4317-956f-327423a4b20a
📒 Files selected for processing (47)
apps/web/.env.exampleapps/web/app/(ee)/api/cron/discount-codes/create/queue-batches/route.tsapps/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/cron/partners/ban/route.tsapps/web/app/(ee)/api/cron/partners/deactivate/route.tsapps/web/app/(ee)/api/discount-codes/[discountCodeId]/route.tsapps/web/app/(ee)/api/discount-codes/route.tsapps/web/app/(ee)/api/partners/[partnerId]/route.tsapps/web/app/(ee)/api/shopify/integration/callback/route.tsapps/web/app/(ee)/api/shopify/integration/webhook/orders-paid.tsapps/web/app/(ee)/api/shopify/integration/webhook/route.tsapps/web/app/(ee)/api/workflows/partner-approved/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/[partnerId]/links/page.tsxapps/web/lib/actions/partners/accept-program-invite.tsapps/web/lib/actions/partners/create-discount.tsapps/web/lib/actions/partners/delete-discount.tsapps/web/lib/api/discounts/delete-discount-code.tsapps/web/lib/api/links/delete-link.tsapps/web/lib/api/partner-profile/get-partner-for-program.tsapps/web/lib/api/partners/bulk-delete-partners.tsapps/web/lib/cron/enqueue-batch-jobs.tsapps/web/lib/discounts/construct-discount-code.tsapps/web/lib/discounts/create-discount-code.tsapps/web/lib/discounts/delete-discount-code.tsapps/web/lib/discounts/discount-error.tsapps/web/lib/discounts/discount-provider-shopify.tsapps/web/lib/discounts/discount-provider-stripe.tsapps/web/lib/discounts/discount-provider.tsapps/web/lib/discounts/generate-discount-code-for-partner.tsapps/web/lib/discounts/is-discount-equivalent.tsapps/web/lib/integrations/shopify/admin-graphql.tsapps/web/lib/integrations/shopify/create-lead.tsapps/web/lib/integrations/shopify/process-order.tsapps/web/lib/integrations/shopify/schema.tsapps/web/lib/stripe/coupon-discount-converter.tsapps/web/lib/stripe/create-stripe-discount-code.tsapps/web/lib/stripe/disable-stripe-discount-code.tsapps/web/lib/zod/schemas/discount.tsapps/web/lib/zod/schemas/partners.tsapps/web/lib/zod/schemas/workspaces.tsapps/web/ui/modals/add-discount-code-modal.tsxapps/web/ui/partners/constants.tsapps/web/ui/partners/discounts/add-edit-discount-sheet.tsxpackages/prisma/client.tspackages/prisma/schema/discount.prisma
💤 Files with no reviewable changes (3)
- apps/web/lib/stripe/disable-stripe-discount-code.ts
- apps/web/lib/api/discounts/delete-discount-code.ts
- apps/web/lib/stripe/create-stripe-discount-code.ts
Collaborator
Author
|
@coderabbitai any breaking changes? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5540
🔴 Public-facing API
🟡 Internal cron/queue payloads
🟡 Database migration required
⚪ Internal library (all call sites updated within the PR)
TL;DR: The only breaking change that could affect external consumers is the new required 🧠 Learnings used |
steven-tey
approved these changes
Apr 27, 2026
This was referenced May 29, 2026
Merged
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
2 participants
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
Bug Fixes