Group move #3232
Group move #3232
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds multi-condition workflow evaluation, a MoveGroup workflow and server-side partner-move operation, group move UI/API/validation, enriched partnerMetricsUpdated workflow context, centralized ACTION_HANDLERS dispatch, discount auto-provisioning (DB + Stripe + cron), and widespread call-site adaptations. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as Event
participant Dispatcher as executeWorkflows
participant DB as Prisma
participant Evaluator as evaluateWorkflowConditions
participant Executor as executeMoveGroupWorkflow
participant Worker as movePartnersToGroup
Source->>Dispatcher: executeWorkflows(trigger:"partnerMetricsUpdated", reason, identity, metrics)
Dispatcher->>DB: fetch workflows for program & reason
Dispatcher->>Dispatcher: enrich context (identity.groupId, aggregated metrics)
Dispatcher->>Executor: ACTION_HANDLERS.moveGroup.execute(workflow, context)
Executor->>Evaluator: evaluateWorkflowConditions({conditions, attributes})
Evaluator-->>Executor: boolean result
alt conditions satisfied
Executor->>DB: fetch target group & owners
Executor->>Worker: movePartnersToGroup(programId, partnerIds, userId, targetGroup)
Worker->>DB: update programEnrollments (groupId, rewards, discount)
Worker->>Worker: schedule background jobs (remap links, create/delete discount codes, notify partners)
Worker-->>Executor: done
else not satisfied
Executor-->>Dispatcher: no-op
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
… update related schemas. This change includes renaming and restructuring of parameters in the workflow and route files to streamline partner group management.
- Introduced `GroupMoveRule` component to manage rules for moving partners between groups based on specific criteria. - Updated `GroupAdditionalSettings` to include the new `GroupMoveRule` component. - Enhanced `GroupSettingsFormSkeleton` with additional settings rows for group ID and slug. - Refactored `AmountInput` to utilize `InlineBadgePopoverAmountInput` for better input handling. - Added `UserArrowRight` icon for improved UI representation.
- Introduced `upsertGroupMoveWorkflow` to manage group move rules and workflows. - Updated `getGroupOrThrow` to include move rules in the group data. - Enhanced the PATCH route to handle move rules and integrate with the new workflow. - Refactored `GroupMoveRule` component to utilize react-hook-form for better form management. - Updated Zod schemas to include move rules in group definitions and updates. - Improved UI for group additional settings to accommodate new move rule functionality.
… for discount code creation and deletion, and streamline discount code construction logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)
132-153:⚠️ Potential issue | 🟡 MinorWrap workflow and group operations in a transaction for atomicity.
The
upsertGroupMoveRulesoperation and subsequentpartnerGroup.updateat lines 132–153 are not transactional. If a workflow is created but the group update fails, an orphaned workflow record remains in the database and becomes inaccessible due to the@uniqueconstraint onworkflowId. Wrap both operations inprisma.$transaction()to ensure they either both succeed or both fail.
🤖 Fix all issues with AI agents
In `@apps/web/app/`(ee)/api/cron/groups/remap-discount-codes/route.ts:
- Line 7: Replace the non-standard zod import "import * as z from 'zod/v4'" with
the canonical import from the zod package; update the import to use the named
export (z) so code using z.object, z.string, etc., continues to work and follows
project guidelines (search for the import line that currently reads import * as
z from "zod/v4" and change it to import { z } from "zod").
In `@apps/web/lib/api/discounts/delete-discount-code.ts`:
- Around line 40-55: The current loop uses Promise.allSettled over chunkOfCodes
and ignores results, which can silently drop failures from queue.enqueueJSON and
create a DB/queue consistency gap; update the logic in delete-discount-code.ts
to first enqueue each chunk via queue.enqueueJSON (referencing queue.enqueueJSON
and APP_DOMAIN_WITH_NGROK) and inspect the Promise.allSettled results, logging
or rethrowing any rejected entries (or failing the operation) so failed enqueues
are visible and retryable, or move the DB delete to occur only after successful
enqueues for each discount code.
In `@apps/web/lib/api/groups/upsert-group-move-rules.ts`:
- Around line 31-48: The current conditional only deletes an existing workflow
when moveRules?.length === 0 and group.workflowId exists, but if moveRules is an
empty array and there is no existing workflow the code falls through and creates
a new empty workflow; update the early-return logic in the upsert flow (the
block using moveRules?.length === 0 and group.workflowId) to treat any empty
array as a "remove rules" signal: when moveRules?.length === 0 return {
workflowId: null } (and delete existing workflow if group.workflowId is present)
so you never create a new workflow with empty triggerConditions; adjust the
branches that follow the empty-array check (the subsequent if (!moveRules)
branch) accordingly so undefined remains used only for the absent/null moveRules
case.
🧹 Nitpick comments (6)
apps/web/lib/api/groups/move-partners-to-group.ts (2)
53-53: Consider using a distinct variable name for filtered partner IDs.Reassigning the
partnerIdsparameter shadows the original input and can reduce traceability during debugging. A descriptive name likeenrolledPartnerIdswould clarify that this list represents validated enrollments.♻️ Suggested refactor
- partnerIds = programEnrollments.map(({ partnerId }) => partnerId); + const enrolledPartnerIds = programEnrollments.map(({ partnerId }) => partnerId);Then use
enrolledPartnerIdsin subsequent code (lines 58, 81, 96, 103, 111).
90-113: Consider logging failures from background tasks.
Promise.allSettledcaptures rejections but discards them silently. Failed notifications or link recordings could go unnoticed, making debugging harder.♻️ Suggested improvement
- await Promise.allSettled([ + const results = await Promise.allSettled([ qstash.publishJSON({ url: `${APP_DOMAIN_WITH_NGROK}/api/cron/groups/remap-default-links`, body: { programId, groupId: group.id, partnerIds, userId, }, }), triggerDraftBountySubmissionCreation({ programId, partnerIds, }), recordLink(partnerLinks), notifyPartnerGroupChange({ programId, groupId: group.id, partnerIds, }), ]); + + results.forEach((result, index) => { + if (result.status === "rejected") { + console.error( + `Background task ${index} failed in movePartnersToGroup:`, + result.reason, + ); + } + });apps/web/app/(ee)/api/workflows/partner-approved/route.ts (1)
160-223: Add structured workflow logging for auto‑provisioning outcomes.
This step returns silently for several skip cases and usesconsole.error, which makes troubleshooting harder than the other steps. Consider logging start/skip reasons and using the workflow logger for failures.♻️ Suggested logging improvements
await context.run("create-discount-codes", async () => { + logger.info({ + message: "Started executing workflow step 'create-discount-codes'.", + data: input, + }); if (!groupId) { + logger.warn({ + message: `The partner ${partnerId} is not associated with any group.`, + }); return; } const group = await prisma.partnerGroup.findUnique({ where: { id: groupId }, include: { discount: true }, }); if (!group?.discount?.autoProvisionEnabledAt) { + logger.info({ + message: `Discount auto-provisioning not enabled for group ${groupId}.`, + }); return; } const workspace = await prisma.project.findUniqueOrThrow({ where: { id: program.workspaceId }, select: { stripeConnectId: true }, }); if (!workspace.stripeConnectId) { + logger.warn({ + message: `Workspace ${program.workspaceId} is not connected to Stripe.`, + }); return; } const partnerLinks = await prisma.link.findMany({ where: { programId, partnerId, partnerGroupDefaultLinkId: { not: null }, discountCode: { is: null }, }, select: { id: true }, }); if (partnerLinks.length === 0) { + logger.info({ + message: `No eligible links found for partner ${partnerId}.`, + }); return; } for (const link of partnerLinks) { try { await createDiscountCode({ stripeConnectId: workspace.stripeConnectId, partner, link, discount: group.discount, }); } catch (error) { - console.error( - `Failed to create discount code for link ${link.id}:`, - error, - ); + logger.error({ + message: `Failed to create discount code for link ${link.id}.`, + error, + }); } } });apps/web/lib/api/groups/upsert-group-move-rules.ts (1)
9-10: Remove.tsextensions from import paths.TypeScript module resolution typically doesn't require explicit
.tsextensions in import paths. Including them can cause issues with some bundlers and is inconsistent with common TypeScript conventions.♻️ Proposed fix
-import { findGroupsWithMatchingRules } from "./find-groups-with-matching-rules.ts"; -import { getGroupMoveRules } from "./get-group-move-rules.ts"; +import { findGroupsWithMatchingRules } from "./find-groups-with-matching-rules"; +import { getGroupMoveRules } from "./get-group-move-rules";apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)
438-440: Inefficient: callingdeleteDiscountCodesseparately for each discount code.
deleteDiscountCodesaccepts an array and batches the database delete and Stripe queue operations. Calling it once per discount code creates unnecessary overhead.♻️ Proposed fix
- ...discountCodesToDelete.map((discountCode) => - deleteDiscountCodes(discountCode), - ), + discountCodesToDelete.length > 0 && + deleteDiscountCodes(discountCodesToDelete),apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)
139-148: Sequential discount code creation may be slow for large partner sets.Creating discount codes one at a time in a for-loop can be slow and increases the cron job duration. Consider using
Promise.allor batched parallel execution (e.g., withp-limitor chunking) to improve throughput.♻️ Proposed improvement using Promise.all
if (workspace.stripeConnectId) { - for (const { link, partner } of partners) { - await createDiscountCode({ + await Promise.all( + partners.map(({ link, partner }) => + createDiscountCode({ stripeConnectId: workspace.stripeConnectId, partner, link, discount: group.discount, - }); - } + }), + ), + ); }
| if (moveRules?.length === 0 && group.workflowId) { | ||
| await prisma.workflow.delete({ | ||
| where: { | ||
| id: group.workflowId, | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| workflowId: null, | ||
| }; | ||
| } | ||
|
|
||
| // Do nothing if no move rule is provided | ||
| if (!moveRules) { | ||
| return { | ||
| workflowId: undefined, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Potential edge case: empty moveRules array without existing workflow.
If moveRules is an empty array ([]) and the group has no existing workflowId, the function will proceed to create a new workflow with empty triggerConditions. This may not be the intended behavior—typically an empty array signals "remove rules" rather than "create workflow with no rules."
Consider returning early for empty arrays regardless of existing workflow state:
🛡️ Proposed fix
if (moveRules?.length === 0 && group.workflowId) {
await prisma.workflow.delete({
where: {
id: group.workflowId,
},
});
return {
workflowId: null,
};
}
+ // Return early if moveRules is an empty array (no workflow to create)
+ if (moveRules?.length === 0) {
+ return {
+ workflowId: undefined,
+ };
+ }
+
// Do nothing if no move rule is provided
if (!moveRules) {
return {
workflowId: undefined,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (moveRules?.length === 0 && group.workflowId) { | |
| await prisma.workflow.delete({ | |
| where: { | |
| id: group.workflowId, | |
| }, | |
| }); | |
| return { | |
| workflowId: null, | |
| }; | |
| } | |
| // Do nothing if no move rule is provided | |
| if (!moveRules) { | |
| return { | |
| workflowId: undefined, | |
| }; | |
| } | |
| if (moveRules?.length === 0 && group.workflowId) { | |
| await prisma.workflow.delete({ | |
| where: { | |
| id: group.workflowId, | |
| }, | |
| }); | |
| return { | |
| workflowId: null, | |
| }; | |
| } | |
| // Return early if moveRules is an empty array (no workflow to create) | |
| if (moveRules?.length === 0) { | |
| return { | |
| workflowId: undefined, | |
| }; | |
| } | |
| // Do nothing if no move rule is provided | |
| if (!moveRules) { | |
| return { | |
| workflowId: undefined, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/lib/api/groups/upsert-group-move-rules.ts` around lines 31 - 48, The
current conditional only deletes an existing workflow when moveRules?.length ===
0 and group.workflowId exists, but if moveRules is an empty array and there is
no existing workflow the code falls through and creates a new empty workflow;
update the early-return logic in the upsert flow (the block using
moveRules?.length === 0 and group.workflowId) to treat any empty array as a
"remove rules" signal: when moveRules?.length === 0 return { workflowId: null }
(and delete existing workflow if group.workflowId is present) so you never
create a new workflow with empty triggerConditions; adjust the branches that
follow the empty-array check (the subsequent if (!moveRules) branch) accordingly
so undefined remains used only for the absent/null moveRules case.
|
@coderabbitai, full review please. Are there any breaking changes? |
|
Breaking Changes1.
|
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/api/workflows/execute-workflows.ts (1)
130-142:⚠️ Potential issue | 🟠 MajorGate the groupId check only for workflows that require it.
Lines 137-142 block all workflows when
groupIdis missing. However,executeSendCampaignWorkflowdoesn't require the partner to have agroupIdin context—it uses campaign group associations instead. OnlyAwardBountyandMoveGroupactually check and usegroupIdfrom the context, so this is a functional regression that prevents valid campaigns from executing.Consider gating the check to only
AwardBountyandMoveGroupworkflows:🔧 Suggested adjustment
- if (!programEnrollment.groupId) { + const requiresGroupId = workflows.some((workflow) => { + const parsed = parseWorkflowConfig(workflow); + return [ + WORKFLOW_ACTION_TYPES.AwardBounty, + WORKFLOW_ACTION_TYPES.MoveGroup, + ].includes(parsed.action.type); + }); + + if (requiresGroupId && !programEnrollment.groupId) { console.error( `Partner ${partnerId} is not enrolled in a group in program ${programId}.`, ); return; }
🤖 Fix all issues with AI agents
In `@apps/web/lib/api/workflows/execute-workflows.ts`:
- Around line 66-88: The pre-filtering and commission-check currently call
parseWorkflowConfig directly which can throw and abort processing; wrap calls to
parseWorkflowConfig inside try/catch blocks (for the workflows.filter pre-filter
and the workflows.some commission check) and if parse fails, log the error and
skip that workflow (i.e., treat it as not matching and not requiring
commissions) so a single bad config doesn't stop execution; reference
parseWorkflowConfig, workflows, REASON_TO_ATTRIBUTES, shouldFetchCommissions,
trigger and reason when locating the spots to guard and add the logging/skip
behavior.
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 (3)
packages/prisma/schema/workflow.prisma (1)
1-10:⚠️ Potential issue | 🟡 MinorMigration script incomplete —
clickRecordednot handled.The migration script at
apps/web/scripts/migrations/migrate-workflow-triggers.tsmigratesleadRecorded,saleRecorded, andcommissionEarnedtopartnerMetricsUpdated, but does not handleclickRecorded. WhileclickRecordedappears unused in the codebase, the TODO comment indicates it should be removed after migrations. Add migration logic forclickRecordedbefore removing it from the enum to prevent schema inconsistencies.apps/web/lib/zod/schemas/campaigns.ts (1)
115-122:⚠️ Potential issue | 🟡 MinorInconsistent schema usage for
triggerConditionin query vs update.
getCampaignsQuerySchemausesworkflowConditionSchema(line 120) for parsingtriggerCondition, whileCampaignSchemaandupdateCampaignSchemause the newcampaignTriggerConditionSchema. This could cause issues if a query includes operators or value types that the stricter campaign schema doesn't support.Consider using
campaignTriggerConditionSchemahere for consistency:Suggested fix
triggerCondition: z .string() .pipe( z.preprocess( (input: string) => JSON.parse(input), - workflowConditionSchema, + campaignTriggerConditionSchema, ), ) .optional(),apps/web/lib/api/workflows/execute-workflows.ts (1)
137-142:⚠️ Potential issue | 🟠 MajorOnly require
groupIdforMoveGroupworkflows; other workflows should not be blocked by a missing group assignment.The early return at lines 137-142 prevents all workflows from executing if a partner lacks a group assignment. However,
groupIdis nullable in theProgramEnrollmentmodel and is only required forMoveGroupworkflows.AwardBountyworkflows should execute regardless of group assignment. Either gate the check to onlyMoveGroupor allow workflows to handle missinggroupIdindividually (similar to howexecute-move-group-workflowalready does on line 29).
🤖 Fix all issues with AI agents
In `@apps/web/app/`(ee)/api/cron/discount-codes/delete/route.ts:
- Around line 27-34: The success message always reports the discount was
disabled even though disableStripeDiscountCode(code, stripeConnectId) can return
undefined; update the route to branch on the function's return value (the result
of disableStripeDiscountCode) and only call logAndRespond with the success
message when a non-undefined result is returned, otherwise either throw an error
or call logAndRespond with a distinct "not found" or failure message that
includes code and workspace.stripeConnectId so logs reflect the real Stripe
update outcome.
In `@apps/web/lib/api/groups/find-groups-with-matching-rules.ts`:
- Around line 50-59: The current early return in
find-groups-with-matching-rules.ts assumes that if sharedAttributes.length === 0
the rule sets cannot conflict, but disjoint attributes can still be
simultaneously satisfied (e.g., leads ≥ X and conversions ≥ Y); update the logic
where rules1ByAttribute and rules2ByAttribute are compared so you do not
automatically return false for sharedAttributes.length === 0 — either treat
disjoint attribute sets as overlapping by returning true in that branch, or
implement a satisfiability check that combines independent attribute constraints
from rulesForAttribute / rules1ByAttribute and rules2ByAttribute to determine if
there exists an assignment satisfying both rule sets, and replace the return
false with the chosen behavior.
In `@apps/web/lib/api/groups/upsert-group-move-rules.ts`:
- Around line 9-10: Update the two local imports so they match project
convention by removing the ".ts" extension: change the import paths that
reference findGroupsWithMatchingRules and getGroupMoveRules to import from
"./find-groups-with-matching-rules" and "./get-group-move-rules" respectively;
ensure only the extension is removed and identifiers
(findGroupsWithMatchingRules, getGroupMoveRules) remain unchanged.
🧹 Nitpick comments (12)
apps/web/ui/shared/inline-badge-popover.tsx (2)
339-350: Money input handlers applied unconditionally for all input types.The
handleMoneyKeyDownandhandleMoneyInputChangehandlers are invoked regardless of thetypeprop. Fortype="number"inputs that aren't currency/percentage values, these handlers may impose unwanted formatting constraints (e.g., restricting decimal places or allowed characters).Consider conditionally applying these handlers only for currency/percentage types:
♻️ Proposed fix
onKeyDown={(e) => { if (e.key === "Enter") { e.preventDefault(); setIsOpen(false); } - handleMoneyKeyDown(e); + if (type !== "number") { + handleMoneyKeyDown(e); + } onKeyDown?.(e); }} onChange={(e) => { - handleMoneyInputChange(e); + if (type !== "number") { + handleMoneyInputChange(e); + } onChange?.(e); }}
317-360: Consider addingdisplayNamefor better debugging.Components created with
forwardRefbenefit from an explicitdisplayNamefor React DevTools and error messages. This is consistent withInlineBadgePopoverInputwhich also lacks one.✨ Suggested improvement
}); + +InlineBadgePopoverAmountInput.displayName = "InlineBadgePopoverAmountInput";apps/web/lib/integrations/shopify/create-sale.ts (1)
164-166: Prefer enum usage for workflow triggers (if still exported).Using the enum keeps compile-time safety and avoids typos during future refactors.
♻️ Suggested change
-import { prisma } from "@dub/prisma"; +import { WorkflowTrigger, prisma } from "@dub/prisma"; ... - trigger: "partnerMetricsUpdated", + trigger: WorkflowTrigger.partnerMetricsUpdated,apps/web/scripts/migrations/migrate-workflow-triggers.ts (1)
4-19: Ensure the migration exits cleanly by disconnecting Prisma and surfacing failures.Without a catch/finally, failures can be swallowed and Prisma connections may keep the process alive.
🧹 Suggested cleanup pattern
async function main() { const workflows = await prisma.workflow.updateMany({ where: { trigger: { in: ["leadRecorded", "saleRecorded", "commissionEarned"], }, }, data: { trigger: "partnerMetricsUpdated", }, }); console.log(`Updated ${workflows.count} workflows.`); } -main(); +main() + .catch((error) => { + console.error("Migration failed:", error); + process.exitCode = 1; + }) + .finally(async () => { + await prisma.$disconnect(); + });apps/web/lib/api/groups/get-group-or-throw.ts (1)
26-35: Optimize the workflow fetch to only includetriggerConditionsand remove the full object from the response.The function currently fetches the entire workflow object but only uses
triggerConditionsformoveRules. Removeworkflowfrom the return shape to keep the payload minimal.🔧 Suggested optimization
const group = await prisma.partnerGroup.findUnique({ // ... include: { clickReward: includeExpandedFields, leadReward: includeExpandedFields, saleReward: includeExpandedFields, discount: includeExpandedFields, utmTemplate: includeExpandedFields, partnerGroupDefaultLinks: includeExpandedFields, program: includeExpandedFields, - workflow: includeExpandedFields, + workflow: includeExpandedFields + ? { select: { triggerConditions: true } } + : false, }, }); - return { - ...group, - moveRules: group.workflow?.triggerConditions, - }; + const { workflow, ...rest } = group; + return { + ...rest, + moveRules: workflow?.triggerConditions, + };packages/email/src/templates/partner-group-changed.tsx (1)
97-108: Consider using a more stable key for list items.Using
labelas a React key could cause issues if two rewards happen to have the same label text. Consider using the index or combining index with label for uniqueness.♻️ Suggested change
- {rewards.map((reward) => ( - <Row key={reward.label} className="mb-0 mt-2"> + {rewards.map((reward, index) => ( + <Row key={`reward-${index}`} className="mb-0 mt-2">Apply the same pattern for the bounties list at lines 119-130.
apps/web/lib/stripe/create-stripe-discount-code.ts (1)
66-80: The final throw statement is unreachable but serves as a defensive safeguard.The while loop will always either return successfully or throw within the catch block when
attempt >= MAX_ATTEMPTS. The throw at line 80 can never execute. While this is dead code, it acts as a safety net if the loop logic is ever modified.♻️ Optional: Remove unreachable code
currentCode = newCode; } } - - throw new Error("Failed to create Stripe discount code."); }Alternatively, keep it as a defensive assertion with
// Should never reach herecomment.apps/web/lib/actions/partners/bulk-deactivate-partners.ts (1)
94-96: Consider simplifying the redundant identity mapping.The
.map((dc) => dc)is an identity function and can be removed for cleaner code:♻️ Suggested simplification
- const allDiscountCodes = programEnrollments.flatMap((pe) => - pe.discountCodes.map((dc) => dc), - ); + const allDiscountCodes = programEnrollments.flatMap( + (pe) => pe.discountCodes, + );apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)
140-147: Sequential discount code creation may be slow for large batches.The loop creates discount codes one at a time. For partners with many links, this could be slow. Consider parallelizing with
Promise.allorPromise.allSettled.♻️ Suggested refactor
- if (workspace.stripeConnectId) { - for (const { link, partner } of partners) { - await createDiscountCode({ - stripeConnectId: workspace.stripeConnectId, - partner, - link, - discount: group.discount, - }); - } - } + if (workspace.stripeConnectId) { + await Promise.allSettled( + partners.map(({ link, partner }) => + createDiscountCode({ + stripeConnectId: workspace.stripeConnectId, + partner, + link, + discount: group.discount, + }), + ), + ); + }apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (1)
15-15: Consider using standard zod import.Similar to other files in this PR, this uses
import * as z from "zod/v4"instead of the standardimport { z } from "zod". Per project guidelines, standard schemas should import from thezodpackage directly.apps/web/lib/api/groups/validate-group-move-rules.ts (2)
17-19: Minor: Redundant null check.
rule.value == null(loose equality) already covers bothnullandundefinedcases, making|| rule.value === undefinedredundant.Suggested simplification
// Check if value is set - if (rule.value == null || rule.value === undefined) { + if (rule.value == null) { throw new Error(`Rule ${i + 1}: Please enter a value.`); }
41-41: Minor: Same redundant pattern for min/max checks.Similar to the earlier check,
min == nullandmax == nullalready coverundefined.Suggested simplification
- if (min == null || min === undefined || isNaN(min) || min <= 0) { + if (min == null || isNaN(min) || min <= 0) { throw new Error( `Rule ${i + 1}: Please enter a minimum value greater than 0.`, ); } - if (max == null || max === undefined || isNaN(max) || max <= 0) { + if (max == null || isNaN(max) || max <= 0) { throw new Error( `Rule ${i + 1}: Please enter a maximum value (limit) greater than 0.`, ); }Also applies to: 47-47
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/web/lib/api/workflows/execute-move-group-workflow.ts`:
- Around line 56-62: The attributes object currently defaults missing metrics to
0 which can falsely satisfy conditions; update the assignments in
execute-move-group-workflow.ts so each field uses null when the metric is
missing instead of 0 (e.g., replace metrics?.aggregated?.leads ?? 0 with
metrics?.aggregated?.leads ?? null for totalLeads, same for totalConversions,
totalSaleAmount and totalCommissions), ensuring the
Partial<Record<WorkflowConditionAttribute, number | null>> type remains correct
and downstream evaluators treat null as "missing" rather than a numeric zero.
- Around line 36-47: Replace the prisma.programEnrollment.findUniqueOrThrow call
with prisma.programEnrollment.findUnique and add an explicit null check for the
returned programEnrollment; if it is null, gracefully skip/return from
executeMoveGroupWorkflow (matching patterns used in
execute-send-campaign-workflow.ts and execute-workflows.ts) so the flow
continues without throwing. Specifically, update the fetch that assigns
programEnrollment to use prisma.programEnrollment.findUnique({...}) and then: if
(!programEnrollment) return (or otherwise skip processing), ensuring subsequent
code that references programEnrollment.groupId is guarded by this check.
- Around line 80-92: The fetch for the target group uses
prisma.partnerGroup.findUnique({ where: { id: newGroupId } }) without scoping to
the program, which can allow moving a partner into a group from another program;
update the query that retrieves newGroup (the prisma.partnerGroup call using
newGroupId) to include the program constraint (e.g., where: { id: newGroupId,
programId: <programId variable> } or switch to findFirst with both id and
programId) so the lookup enforces the workflow's program context before
proceeding.
…e discount code creation, enhance logging messages, and remove unnecessary partner and link selections.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/app/`(ee)/api/cron/groups/remap-discount-codes/route.ts:
- Around line 82-98: The update currently uses group.discount?.id which can
become undefined; decide whether you want to clear the discount or skip updating
when the group has no discount and implement the corresponding guard: either
wrap the prisma.discountCode.updateMany call (the block that uses
discountCodesToUpdate and prisma.discountCode.updateMany) in an if
(group.discount) check to skip updates when group.discount is absent, or
explicitly set data.discountId to null when clearing is intended (e.g., set
discountId to group.discount ? group.discount.id : null) so Prisma receives null
rather than undefined.
- Around line 151-158: The loop blindly dereferences link.programEnrollment with
a non-null assertion in the remap-discount-codes handler; update the loop that
calls createDiscountCode so it either filters out links missing
programEnrollment before iterating or performs a null-check inside the loop and
skips (or logs) those entries, ensuring you pass a valid partner to
createDiscountCode; reference the link.programEnrollment property and the
createDiscountCode call in your change and preserve workspace.stripeConnectId,
link, and group.discount when invoking createDiscountCode.
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/workflows/partner-approved/route.ts (2)
160-227: Missing workflow logger usage in new step.Other steps in this workflow consistently use
logger.infoandlogger.errorfor observability, but this new step only usesconsole.erroron failure. Consider adding logger calls for consistency and better debugging:// Step 2: Auto-provision discount code if enabled await context.run("create-discount-codes", async () => { + logger.info({ + message: "Started executing workflow step 'create-discount-codes'.", + data: input, + }); + if (!groupId) { return; }Also, the silent error handling (lines 220-225) means discount code creation failures won't surface in workflow logs. If this is intentional for resilience, consider at minimum logging these failures via the workflow logger so they're discoverable.
179-186: Consider consolidating workspace queries.The workspace is queried three times in this workflow with different field selections (Step 1:
id, plan; Step 2:stripeConnectId; Step 4:id, webhookEnabled).For a minor optimization, you could fetch all required workspace fields once before the steps begin:
const workspace = await prisma.project.findUniqueOrThrow({ where: { id: program.workspaceId }, select: { id: true, plan: true, stripeConnectId: true, webhookEnabled: true, }, });Then pass the workspace object to each step. This reduces database round-trips, though the current approach is still correct.
| // Update the discount codes to use the new discount if they are equivalent | ||
| if (discountCodesToUpdate.length > 0) { | ||
| console.log( | ||
| `Found ${discountCodesToUpdate.length} discount codes equivalent to the new group's discount. Updating them.`, | ||
| ); | ||
|
|
||
| const programEnrollments = await prisma.programEnrollment.findMany({ | ||
| await prisma.discountCode.updateMany({ | ||
| where: { | ||
| partnerId: { | ||
| in: partnerIds, | ||
| id: { | ||
| in: discountCodesToUpdate.map(({ id }) => id), | ||
| }, | ||
| programId, | ||
| }, | ||
| include: { | ||
| discountCodes: { | ||
| include: { | ||
| discount: true, | ||
| }, | ||
| }, | ||
| data: { | ||
| discountId: group.discount?.id, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Guard against null group.discount before updating.
If group.discount is null, the discountId will be set to undefined, which Prisma may ignore in updateMany (leaving the old value). If the intent is to clear the discount association when the group has no discount, use null explicitly. Otherwise, skip the update when group.discount is absent.
🛡️ Proposed guard
// Update the discount codes to use the new discount if they are equivalent
- if (discountCodesToUpdate.length > 0) {
+ if (discountCodesToUpdate.length > 0 && group.discount) {
console.log(
`Found ${discountCodesToUpdate.length} discount codes equivalent to the new group's discount. Updating them.`,
);
await prisma.discountCode.updateMany({
where: {
id: {
in: discountCodesToUpdate.map(({ id }) => id),
},
},
data: {
- discountId: group.discount?.id,
+ discountId: group.discount.id,
},
});
}🤖 Prompt for AI Agents
In `@apps/web/app/`(ee)/api/cron/groups/remap-discount-codes/route.ts around lines
82 - 98, The update currently uses group.discount?.id which can become
undefined; decide whether you want to clear the discount or skip updating when
the group has no discount and implement the corresponding guard: either wrap the
prisma.discountCode.updateMany call (the block that uses discountCodesToUpdate
and prisma.discountCode.updateMany) in an if (group.discount) check to skip
updates when group.discount is absent, or explicitly set data.discountId to null
when clearing is intended (e.g., set discountId to group.discount ?
group.discount.id : null) so Prisma receives null rather than undefined.
| for (const link of links) { | ||
| await createDiscountCode({ | ||
| stripeConnectId: workspace.stripeConnectId, | ||
| partner: link.programEnrollment!.partner, | ||
| link, | ||
| discount: group.discount, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Non-null assertion on programEnrollment could fail.
Line 154 uses link.programEnrollment!.partner, but the type system doesn't guarantee programEnrollment is non-null. If data inconsistency exists (a link without an enrollment), this will throw at runtime. Consider filtering out links without enrollments or adding a null check.
🛡️ Proposed safeguard
for (const link of links) {
+ if (!link.programEnrollment) {
+ console.warn(`Link ${link.id} has no programEnrollment, skipping.`);
+ continue;
+ }
+
await createDiscountCode({
stripeConnectId: workspace.stripeConnectId,
- partner: link.programEnrollment!.partner,
+ partner: link.programEnrollment.partner,
link,
discount: group.discount,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const link of links) { | |
| await createDiscountCode({ | |
| stripeConnectId: workspace.stripeConnectId, | |
| partner: link.programEnrollment!.partner, | |
| link, | |
| discount: group.discount, | |
| }); | |
| } | |
| for (const link of links) { | |
| if (!link.programEnrollment) { | |
| console.warn(`Link ${link.id} has no programEnrollment, skipping.`); | |
| continue; | |
| } | |
| await createDiscountCode({ | |
| stripeConnectId: workspace.stripeConnectId, | |
| partner: link.programEnrollment.partner, | |
| link, | |
| discount: group.discount, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/app/`(ee)/api/cron/groups/remap-discount-codes/route.ts around lines
151 - 158, The loop blindly dereferences link.programEnrollment with a non-null
assertion in the remap-discount-codes handler; update the loop that calls
createDiscountCode so it either filters out links missing programEnrollment
before iterating or performs a null-check inside the loop and skips (or logs)
those entries, ensuring you pass a valid partner to createDiscountCode;
reference the link.programEnrollment property and the createDiscountCode call in
your change and preserve workspace.stripeConnectId, link, and group.discount
when invoking createDiscountCode.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests