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 action and execution path, consolidated "partnerMetricsUpdated" trigger with richer identity/metrics payloads, group move rule UI/API/service surface, Prisma relation PartnerGroup.workflowId, migration, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Webhook as Webhook (Stripe/Shopify/Conversion)
participant Execute as executeWorkflows
participant DB as Prisma
participant Engine as Workflow Engine (ACTION_HANDLERS)
participant Eval as evaluateWorkflowConditions
participant Move as movePartnersToGroup
Webhook->>Execute: executeWorkflows({ trigger: "partnerMetricsUpdated", reason, identity, metrics })
Execute->>DB: fetch program enrollment & aggregated metrics
Execute->>Engine: iterate filtered workflows
Engine->>Eval: evaluateWorkflowConditions({ conditions, attributes: metrics.aggregated })
alt conditions met
Engine->>Move: movePartnersToGroup(programId, partnerId, userId, targetGroup)
Move->>DB: update programEnrollments (group/rewards)
Move-->>Engine: schedule background tasks (link remap, bounties)
else not met
Engine-->>Execute: skip workflow
end
sequenceDiagram
actor User
participant UI as Group Settings UI
participant API as PATCH /groups/{id}
participant Upsert as upsertGroupMoveRules
participant Detect as findGroupsWithMatchingRules
participant DB as Prisma
User->>UI: Configure move rules and Save
UI->>API: PATCH with moveRules
API->>Upsert: upsertGroupMoveRules({ group, moveRules, workspace })
Upsert->>Detect: findGroupsWithMatchingRules(existingGroups, moveRules)
alt conflicts
Detect-->>Upsert: conflicting groups
Upsert-->>API: throw bad_request
API-->>UI: return error
else no conflicts
Upsert->>DB: create or update Workflow (triggerConditions, actions)
DB-->>Upsert: workflowId
Upsert->>DB: update PartnerGroup.workflowId
Upsert-->>API: { workflowId }
API-->>UI: success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (3)
46-89: Cache invalidation inconsistency persists.Lines 59 and 82 still use
mutatePrefix(\/api/groups/${group.slug}`)while the API calls usegroup.id. This mismatch will prevent proper cache invalidation. Update both to usegroup.id` for consistency with the API endpoint and with the pattern used at line 142.
108-116: Unsafe error property access without type guard.Line 113 accesses
error.messagedirectly without verifying thaterroris anErrorobject. IfvalidateGroupMoveRulesthrows a non-Error value, this could cause a runtime error or displayundefined.🔎 Proposed fix with type guard
if (data.moveRules && data.moveRules.length > 0) { try { validateGroupMoveRules(data.moveRules); } catch (error) { - toast.error(error.message); + toast.error(error instanceof Error ? error.message : String(error)); return; }
268-285: Placeholder documentation link.The "Learn more" link at lines 274-280 points to
https://dub.co/help, which appears to be a generic help page rather than documentation specific to group move rules. Consider updating this to point to specific documentation when available, or use a named constant to make future updates easier.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/find-groups-with-matching-rules.ts.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/find-groups-with-matching-rules.ts.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T01:05:43.266Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx:432-457
Timestamp: 2025-10-15T01:05:43.266Z
Learning: In apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx, defer refactoring the custom MenuItem component (lines 432-457) to use the shared dub/ui MenuItem component to a future PR, as requested by steven-tey.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
🧬 Code graph analysis (1)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (5)
apps/web/lib/zod/schemas/groups.ts (1)
updateGroupSchema(130-153)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/validate-group-move-rules.ts (1)
validateGroupMoveRules(3-61)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/find-groups-with-matching-rules.ts.ts (1)
findGroupsWithMatchingRules(3-25)apps/web/lib/constants/payouts.ts (1)
PAYOUT_HOLDING_PERIOD_DAYS(9-9)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-move-rules.tsx (1)
GroupMoveRules(43-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (1)
167-242: Well-structured form implementation.The form architecture follows React Hook Form best practices:
- ✓
FormProviderproperly wraps the form context- ✓
handleSubmitcorrectly wires validation and submission- ✓ Save button appropriately disabled when form is pristine (
!isDirty)- ✓ Clean integration of the
GroupMoveRulescomponent within the form flow- ✓ Form reset at line 143 correctly updates state after successful save
...co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (3)
113-113: Unsafe error property access without type guard.This issue was previously flagged and remains unaddressed. The
error.messageaccess at line 113 is unsafe ifvalidateGroupMoveRulesthrows a non-Error value.
35-35: Loading state not checked before duplicate validation.This issue was previously flagged and remains unaddressed. The duplicate check can be bypassed if
groupsdata hasn't finished loading, as theloadingstate fromuseGroupsRules()is not extracted or checked.Also applies to: 117-133
275-281: Placeholder link for documentation.This issue was previously flagged and remains unaddressed. The "Learn more" link still points to a generic help page rather than specific group move rules documentation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(ee)/api/groups/rules/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/find-groups-with-matching-rules.ts.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsxapps/web/lib/swr/use-groups-rules.tsapps/web/lib/zod/schemas/groups.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/find-groups-with-matching-rules.ts.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
📚 Learning: 2025-11-24T08:55:31.332Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3089
File: apps/web/app/(ee)/api/fraud-rules/route.ts:71-87
Timestamp: 2025-11-24T08:55:31.332Z
Learning: In apps/web/app/(ee)/api/fraud-rules/route.ts, fraud rules cannot be created in a disabled state. When using prisma.fraudRule.upsert, the create branch intentionally omits the disabledAt field (defaulting to null, meaning enabled), while the update branch allows toggling enabled/disabled state via the disabledAt field. This is a business logic constraint.
Applied to files:
apps/web/app/(ee)/api/groups/rules/route.ts
📚 Learning: 2025-08-26T15:05:55.081Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2736
File: apps/web/lib/swr/use-bounty.ts:11-16
Timestamp: 2025-08-26T15:05:55.081Z
Learning: In the Dub codebase, workspace authentication and route structures prevent endless loading states when workspaceId or similar route parameters are missing, so gating SWR loading states on parameter availability is often unnecessary.
Applied to files:
apps/web/lib/swr/use-groups-rules.ts
📚 Learning: 2025-07-30T15:25:13.936Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2673
File: apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx:56-66
Timestamp: 2025-07-30T15:25:13.936Z
Learning: In apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition objects to allow users to add empty/unconfigured condition fields without type errors, while submission validation uses strict schemas to ensure data integrity. This two-stage validation pattern improves UX by allowing progressive completion of complex forms.
Applied to files:
apps/web/lib/zod/schemas/groups.ts
📚 Learning: 2025-12-18T16:55:57.098Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:57.098Z
Learning: In the bounty system (e.g., apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts), bounties cannot be created with workflow attributes `partnerEnrolledDays` and `partnerJoined`. Bounty workflows only support attributes available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions, which is a subset of the general WORKFLOW_ATTRIBUTES schema.
Applied to files:
apps/web/lib/zod/schemas/groups.ts
📚 Learning: 2025-08-16T11:14:00.667Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2754
File: apps/web/lib/partnerstack/schemas.ts:47-52
Timestamp: 2025-08-16T11:14:00.667Z
Learning: The PartnerStack API always includes the `group` field in partner responses, so the schema should use `.nullable()` rather than `.nullish()` since the field is never omitted/undefined.
Applied to files:
apps/web/lib/zod/schemas/groups.ts
📚 Learning: 2025-10-15T01:05:43.266Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx:432-457
Timestamp: 2025-10-15T01:05:43.266Z
Learning: In apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx, defer refactoring the custom MenuItem component (lines 432-457) to use the shared dub/ui MenuItem component to a future PR, as requested by steven-tey.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
📚 Learning: 2025-10-15T01:52:37.048Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx:270-303
Timestamp: 2025-10-15T01:52:37.048Z
Learning: In React components with dropdowns or form controls that show modals for confirmation (e.g., role selection, delete confirmation), local state should be reverted to match the prop value when the modal is cancelled. This prevents the UI from showing an unconfirmed change. The solution is to either: (1) pass an onClose callback to the modal that resets the local state, or (2) observe modal visibility state and reset on close. Example context: RoleCell component in apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx where role dropdown should revert to user.role when UpdateUserModal is cancelled.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
🧬 Code graph analysis (4)
apps/web/app/(ee)/api/groups/rules/route.ts (3)
apps/web/lib/auth/workspace.ts (1)
withWorkspace(57-491)packages/prisma/index.ts (1)
prisma(16-16)apps/web/lib/zod/schemas/groups.ts (1)
groupRulesSchema(190-192)
apps/web/lib/swr/use-groups-rules.ts (2)
apps/web/lib/zod/schemas/groups.ts (1)
groupRulesSchema(190-192)apps/web/lib/swr/use-workspace.ts (1)
useWorkspace(7-48)
apps/web/lib/zod/schemas/groups.ts (1)
apps/web/lib/zod/schemas/workflows.ts (1)
workflowConditionSchema(83-93)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (6)
apps/web/lib/zod/schemas/groups.ts (1)
updateGroupSchema(130-153)apps/web/lib/swr/use-groups-rules.ts (1)
useGroupsRules(9-25)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/validate-group-move-rules.ts (1)
validateGroupMoveRules(3-61)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/find-groups-with-matching-rules.ts.ts (1)
findGroupsWithMatchingRules(5-32)apps/web/lib/constants/payouts.ts (1)
PAYOUT_HOLDING_PERIOD_DAYS(9-9)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-move-rules.tsx (1)
GroupMoveRules(43-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (1)
96-148: LGTM on the form implementation!The form setup with react-hook-form, validation flow, duplicate checking logic, and cache invalidation strategy are all well-structured. The fix to reset the form state after successful submission (line 144) properly addresses the previous concern about the "Save changes" button remaining enabled.
apps/web/lib/swr/use-groups-rules.ts (1)
1-25: LGTM!This SWR hook follows the established patterns in the codebase. The conditional key based on
workspaceId, the use ofkeepPreviousData: trueto prevent UI flicker during revalidation, and the return structure are all appropriate. Based on learnings, workspace authentication in the route structure prevents endless loading states, so the implementation is complete.apps/web/lib/zod/schemas/groups.ts (1)
15-15: LGTM on the schema integration!The
moveRulesintegration is well-structured across the schemas:
- Line 74:
GroupSchemauses.nullish().default(null)appropriately, asnullis a valid persisted value.- Line 152:
updateGroupSchemauses.optional()correctly, allowing the field to be omitted during updates.- Lines 190-192: The new
groupRulesSchemaexport provides a clean, typed subset for the API route.This pattern aligns with the codebase's established approach for form and validation schemas.
Also applies to: 74-74, 152-152, 190-192
apps/web/app/(ee)/api/groups/rules/route.ts (1)
1-39: LGTM!This API route implementation is clean and secure:
- Properly wrapped with
withWorkspacefor authentication and authorization.- Efficient Prisma query that selects only the needed fields.
- Correct use of optional chaining (
workflow?.triggerConditions) to handle cases where a group doesn't have an associated workflow.- Schema validation with
groupRulesSchema.parse()ensures type safety before returning the response.- Appropriate permission (
groups.read) and plan requirements (advanced,enterprise) are enforced.
… fetching rules, and implement validation checks
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (4)
3-4: Remove explicit.tsextensions from imports.This comment duplicates a previous review suggesting removal of
.tsextensions from import statements to follow TypeScript and Next.js conventions.🔎 Proposed fix
-import { findGroupsWithMatchingRules } from "@/lib/api/groups/find-groups-with-matching-rules.ts"; +import { findGroupsWithMatchingRules } from "@/lib/api/groups/find-groups-with-matching-rules"; -import { validateGroupMoveRules } from "@/lib/api/groups/validate-group-move-rules.ts"; +import { validateGroupMoveRules } from "@/lib/api/groups/validate-group-move-rules";Also applies to: 9-9
113-115: Unsafe error property access without type guard.This duplicates a previous review comment: Line 114 accesses
error.messagedirectly without verifying thaterroris anErrorobject.🔎 Proposed fix with type guard
} catch (error) { - toast.error(error.message); + toast.error(error instanceof Error ? error.message : String(error)); return; }
36-36: Duplicate check can be skipped if groups data hasn't loaded yet.This duplicates a previous review comment: The
useGroupsRules()hook exposes aloadingstate, but the duplicate rule check at lines 118-134 is only guarded byif (groups). Consider disabling form submission while groups are loading.Also applies to: 118-134, 234-236
277-277: Placeholder link for documentation.This duplicates a previous review comment: The "Learn more" link points to a generic help page rather than specific group move rules documentation.
🧹 Nitpick comments (4)
apps/web/lib/api/groups/validate-group-move-rules.ts (1)
16-19: Redundant null check.Line 17 uses both
== nulland=== undefined, but== nullalready matches bothnullandundefinedin JavaScript.🔎 Proposed simplification
- if (rule.value == null || rule.value === undefined) { + if (rule.value == null) { throw new Error(`Rule ${i + 1}: Please enter a value.`); }apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)
132-153: Conditionally include workflowId in update payload.Lines 138-153 unconditionally include
workflowIdin the group update data, even whenupsertGroupMoveRulesreturns{ workflowId: undefined }(the no-op case). While Prisma may ignoreundefinedvalues, it's clearer to conditionally spread the workflowId only when it's not undefined.🔎 Proposed fix to conditionally include workflowId
const [updatedGroup] = await Promise.all([ prisma.partnerGroup.update({ where: { id: group.id, }, data: { name, slug, color, additionalLinks, maxPartnerLinks, linkStructure, utmTemplateId, applicationFormData, landerData, - workflowId, + ...(workflowId !== undefined && { workflowId }), ...(holdingPeriodDays !== undefined && !updateHoldingPeriodDaysForAllGroups && { holdingPeriodDays, }),apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (1)
60-60: Cache invalidation fixed for move rules but inconsistent for other updates.Line 143 correctly uses
mutate(\/api/groups/${group.id}`)for move rule updates. However, lines 60 and 83 still usemutatePrefix(`/api/groups/${group.slug}`)` for auto-approve and holding period updates. Since all API calls use the group ID (not slug), these invalidations should also use the group ID for consistency.🔎 Proposed fix for consistent cache invalidation
onSuccess: async () => { - await mutatePrefix(`/api/groups/${group.slug}`); + await mutatePrefix(`/api/groups/${group.id}`); setShowConfirmAutoApproveModal(false);Also applies to: 83-83
apps/web/lib/api/groups/upsert-group-move-rules.ts (1)
9-10: Remove explicit.tsextensions from imports.Imports should not include explicit
.tsextensions to follow TypeScript and Next.js conventions.🔎 Proposed fix
-import { findGroupsWithMatchingRules } from "./find-groups-with-matching-rules.ts"; +import { findGroupsWithMatchingRules } from "./find-groups-with-matching-rules"; -import { getGroupMoveRules } from "./get-group-move-rules.ts"; +import { getGroupMoveRules } from "./get-group-move-rules";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.tsapps/web/app/(ee)/api/groups/rules/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsxapps/web/lib/api/groups/find-groups-with-matching-rules.ts.tsapps/web/lib/api/groups/get-group-move-rules.tsapps/web/lib/api/groups/upsert-group-move-rules.tsapps/web/lib/api/groups/validate-group-move-rules.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(ee)/api/groups/rules/route.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-15T01:05:43.266Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx:432-457
Timestamp: 2025-10-15T01:05:43.266Z
Learning: In apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx, defer refactoring the custom MenuItem component (lines 432-457) to use the shared dub/ui MenuItem component to a future PR, as requested by steven-tey.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
📚 Learning: 2025-11-24T16:36:36.196Z
Learnt from: CR
Repo: dubinc/dub PR: 0
File: packages/hubspot-app/CLAUDE.md:0-0
Timestamp: 2025-11-24T16:36:36.196Z
Learning: Applies to packages/hubspot-app/app/settings/**/*.{js,jsx,ts,tsx} : Only components exported from the `hubspot/ui-extensions` npm package can be used in settings components
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
📚 Learning: 2025-10-15T01:52:37.048Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx:270-303
Timestamp: 2025-10-15T01:52:37.048Z
Learning: In React components with dropdowns or form controls that show modals for confirmation (e.g., role selection, delete confirmation), local state should be reverted to match the prop value when the modal is cancelled. This prevents the UI from showing an unconfirmed change. The solution is to either: (1) pass an onClose callback to the modal that resets the local state, or (2) observe modal visibility state and reset on close. Example context: RoleCell component in apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx where role dropdown should revert to user.role when UpdateUserModal is cancelled.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
📚 Learning: 2025-08-14T05:17:51.825Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2735
File: apps/web/lib/actions/partners/delete-reward.ts:33-41
Timestamp: 2025-08-14T05:17:51.825Z
Learning: In the partner groups system, a rewardId can only belong to one group, establishing a one-to-one relationship between rewards and groups. This means using Prisma's `update` method (rather than `updateMany`) is appropriate when updating groups by rewardId.
Applied to files:
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts
🧬 Code graph analysis (6)
apps/web/lib/api/groups/validate-group-move-rules.ts (1)
apps/web/lib/types.ts (1)
WorkflowCondition(626-626)
apps/web/lib/api/groups/get-group-move-rules.ts (2)
packages/prisma/index.ts (1)
prisma(16-16)apps/web/lib/zod/schemas/groups.ts (1)
groupRulesSchema(190-192)
apps/web/lib/api/groups/upsert-group-move-rules.ts (8)
apps/web/lib/types.ts (2)
WorkflowCondition(626-626)WorkflowAction(633-633)apps/web/lib/plan-capabilities.ts (1)
getPlanCapabilities(4-23)apps/web/lib/api/errors.ts (1)
DubApiError(58-75)packages/prisma/index.ts (1)
prisma(16-16)apps/web/lib/api/groups/find-groups-with-matching-rules.ts.ts (1)
findGroupsWithMatchingRules(5-32)apps/web/lib/api/groups/get-group-move-rules.ts (1)
getGroupMoveRules(4-25)packages/prisma/client.ts (1)
WorkflowTrigger(36-36)apps/web/lib/api/create-id.ts (1)
createId(69-74)
apps/web/lib/api/groups/find-groups-with-matching-rules.ts.ts (2)
apps/web/lib/zod/schemas/groups.ts (1)
groupRulesSchema(190-192)apps/web/lib/types.ts (1)
WorkflowCondition(626-626)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (5)
apps/web/lib/zod/schemas/groups.ts (1)
updateGroupSchema(130-153)apps/web/lib/swr/use-groups-rules.ts (1)
useGroupsRules(9-25)apps/web/lib/api/groups/validate-group-move-rules.ts (1)
validateGroupMoveRules(3-61)apps/web/lib/constants/payouts.ts (1)
PAYOUT_HOLDING_PERIOD_DAYS(9-9)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-move-rules.tsx (1)
GroupMoveRules(43-158)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)
apps/web/lib/api/groups/upsert-group-move-rules.ts (1)
upsertGroupMoveRules(12-104)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
apps/web/lib/api/groups/get-group-move-rules.ts (1)
4-25: LGTM!The function correctly queries partner groups with their workflow trigger conditions, maps them to the moveRules format expected by the schema, and validates the results before returning. The use of optional chaining and nullish coalescing ensures safe handling of groups without workflows.
apps/web/lib/api/groups/validate-group-move-rules.ts (1)
3-60: LGTM!The validation logic is comprehensive and provides clear, user-friendly error messages. It correctly validates:
- Attribute selection
- Value presence
- Operator-specific constraints (gte requires number > 0, between requires valid min/max range)
- Relationship constraints (max > min for between operator)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)
385-392: LGTM!The workflow deletion logic is correctly placed before the group deletion (step 5 before step 6) to respect foreign key constraints. The conditional check ensures the workflow is only deleted when present.
apps/web/lib/api/groups/find-groups-with-matching-rules.ts.ts (3)
5-32: LGTM!The main function correctly handles edge cases (null/empty inputs) and filters groups based on rule overlap detection. The logic properly excludes the current group and only considers groups with defined move rules.
34-78: LGTM!The rule overlap detection logic is well-designed:
- Creates attribute-indexed maps for efficient lookup
- Correctly identifies that rule sets with no shared attributes cannot conflict
- Requires ALL shared attributes to overlap for a conflict, which ensures proper conflict detection
The comment on lines 34-35 clearly explains the intent.
80-124: LGTM!The interval conversion and overlap checking logic is mathematically correct:
conditionToIntervalproperly convertsgteandbetweenoperators to numeric intervalsdoConditionsOverlapuses the standard interval overlap formula:[a, b]overlaps[c, d]ifa <= d && c <= bapps/web/lib/api/groups/upsert-group-move-rules.ts (4)
21-29: LGTM!The plan capability check correctly enforces that group move rules are only available on Advanced and Enterprise plans, throwing an appropriate error for unauthorized access.
31-48: LGTM!The function correctly handles three distinct cases:
- Empty array: deletes the existing workflow and returns
nullto clear the field- Undefined: no-op, returns
undefinedto preserve existing state- Populated array: proceeds to validation and upsert
This design provides clear semantics for each case.
50-63: LGTM!The conflict detection logic correctly:
- Fetches all existing group move rules
- Checks for overlapping rules using the helper function
- Provides a clear, user-friendly error message with group names
- Uses pluralization for proper grammar
65-103: LGTM!The workflow upsert logic is well-structured:
- Builds a proper
MoveGroupaction with the group ID- Uses the correct trigger type
partnerMetricsUpdated- Includes trigger conditions and actions in the workflow data
- Creates new workflows with properly prefixed IDs
- Updates existing workflows with new data
…tion checks for partner group movement
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/lib/api/workflows/execute-workflows.ts (1)
169-171: Consider removing unreachable defensive check.The
!actioncheck is defensive but unreachable in practice. TheparseWorkflowConfigfunction always returns an action or throws an error (whenactions.length === 0), soactionwill never be null/undefined at this point.🔎 Proposed refactor to remove unreachable check
const { action } = parseWorkflowConfig(workflow); - if (!action) { - continue; - } - const handler = ACTION_HANDLERS[action.type];
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/lib/api/workflows/execute-move-group-workflow.tsapps/web/lib/api/workflows/execute-workflows.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/lib/api/workflows/execute-move-group-workflow.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
📚 Learning: 2025-12-18T16:55:57.098Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:57.098Z
Learning: In the bounty system (e.g., apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts), bounties cannot be created with workflow attributes `partnerEnrolledDays` and `partnerJoined`. Bounty workflows only support attributes available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions, which is a subset of the general WORKFLOW_ATTRIBUTES schema.
Applied to files:
apps/web/lib/api/workflows/execute-workflows.ts
📚 Learning: 2025-08-25T17:33:45.072Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
Applied to files:
apps/web/lib/api/workflows/execute-workflows.ts
🧬 Code graph analysis (1)
apps/web/lib/api/workflows/execute-workflows.ts (5)
apps/web/lib/types.ts (2)
WorkflowContext(743-751)WorkflowConditionAttribute(628-628)apps/web/lib/api/workflows/execute-complete-bounty-workflow.ts (1)
executeCompleteBountyWorkflow(13-229)apps/web/lib/api/workflows/execute-send-campaign-workflow.ts (1)
executeSendCampaignWorkflow(21-210)apps/web/lib/api/workflows/execute-move-group-workflow.ts (1)
executeMoveGroupWorkflow(10-99)apps/web/lib/api/workflows/parse-workflow-config.ts (1)
parseWorkflowConfig(8-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
apps/web/lib/api/workflows/execute-workflows.ts (6)
66-81: LGTM!The reason-based filtering logic correctly implements the optimization strategy. By filtering out workflows whose conditions don't match the reason, you avoid unnecessary data fetching and execution. The logging provides good visibility into the filtering behavior.
83-128: Excellent conditional data fetching optimization.The lazy loading of commission aggregates is a smart performance improvement. By only executing the expensive aggregate query when workflows actually require
totalCommissions, you avoid unnecessary database load. The parallel fetching withPromise.allefficiently retrieves both programEnrollment and commissions when needed.
144-163: LGTM!The context construction properly enriches the workflow context with aggregated metrics computed from partner link stats and commissions. The addition of
groupIdto identity enables downstream workflows (like MoveGroup) to access the partner's current group.
165-187: LGTM!The handler dispatch pattern with error handling is well-implemented. Individual workflow failures are logged and don't prevent other workflows from executing, which provides good fault isolation.
11-30: Excellent refactoring to a handler-based dispatch pattern.The introduction of
WorkflowActionHandlerinterface andACTION_HANDLERSmapping provides a clean, extensible architecture for workflow actions. This eliminates the need for switch statements and makes adding new action types straightforward.Verify that
ACTION_HANDLERScovers all values in theWORKFLOW_ACTION_TYPESenum to ensure no action types are missing.
32-41: Smart optimization with reason-based filtering.The
REASON_TO_ATTRIBUTESmapping enables early filtering of workflows, avoiding unnecessary parsing and execution. The mapping is complete and correct:
lead→totalLeadssale→totalConversions,totalSaleAmountcommission→totalCommissionsAll four
WorkflowConditionAttributevalues are accounted for, and the semantic relationships are sound.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (3)
9-9: Remove explicit.tsextension from import.TypeScript and Next.js automatically resolve module extensions. Explicit
.tsextensions in imports are non-standard and can cause issues with some bundlers or configurations.🔎 Proposed fix
-import useGroupsRules from "@/lib/swr/use-groups-rules.ts"; +import useGroupsRules from "@/lib/swr/use-groups-rules";
113-115: Unsafe error property access without type guard.Line 114 accesses
error.messagedirectly without verifying thaterroris anErrorobject. IfvalidateGroupMoveRulesthrows a non-Error value, this will cause a runtime error or displayundefined.🔎 Proposed fix with type guard
} catch (error) { - toast.error(error.message); + toast.error(error instanceof Error ? error.message : String(error)); return; }
276-282: Placeholder link for documentation.The "Learn more" link points to
https://dub.co/helpwhich appears to be a generic help page rather than documentation specific to group move rules. Consider updating this to a more specific documentation URL when available, or adding a TODO comment.
🧹 Nitpick comments (1)
apps/web/lib/api/groups/find-groups-with-matching-rules.ts (1)
93-100: Consider adding defensive validation forbetweenoperator values.The
betweencase accessesvalue.minandvalue.maxwithout validating they are numbers. WhilevalidateGroupMoveRulesvalidates these upstream, adding defensive checks here would make the function more robust if called from other contexts.🔎 Proposed defensive check
case "between": if (typeof condition.value === "object" && condition.value !== null) { + const min = condition.value.min; + const max = condition.value.max; + if (typeof min !== "number" || typeof max !== "number") { + return null; + } return { - min: condition.value.min, - max: condition.value.max, + min, + max, }; } return null;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsxapps/web/lib/api/groups/find-groups-with-matching-rules.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
📚 Learning: 2025-10-15T01:05:43.266Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx:432-457
Timestamp: 2025-10-15T01:05:43.266Z
Learning: In apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx, defer refactoring the custom MenuItem component (lines 432-457) to use the shared dub/ui MenuItem component to a future PR, as requested by steven-tey.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
📚 Learning: 2025-11-24T16:36:36.196Z
Learnt from: CR
Repo: dubinc/dub PR: 0
File: packages/hubspot-app/CLAUDE.md:0-0
Timestamp: 2025-11-24T16:36:36.196Z
Learning: Applies to packages/hubspot-app/app/settings/**/*.{js,jsx,ts,tsx} : Only components exported from the `hubspot/ui-extensions` npm package can be used in settings components
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
🧬 Code graph analysis (2)
apps/web/lib/api/groups/find-groups-with-matching-rules.ts (2)
apps/web/lib/zod/schemas/groups.ts (1)
groupRulesSchema(190-192)apps/web/lib/types.ts (1)
WorkflowCondition(626-626)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (5)
apps/web/lib/zod/schemas/groups.ts (1)
updateGroupSchema(130-153)apps/web/lib/swr/use-groups-rules.ts (1)
useGroupsRules(9-25)apps/web/lib/api/groups/validate-group-move-rules.ts (1)
validateGroupMoveRules(3-61)apps/web/lib/api/groups/find-groups-with-matching-rules.ts (1)
findGroupsWithMatchingRules(5-32)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-move-rules.tsx (1)
GroupMoveRules(43-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (1)
97-149: LGTM on the form submission flow.The form handling implementation is well-structured:
- Form state is properly reset after successful save (line 145)
- Duplicate rule validation guards against missing groups data
- Save button correctly disables during
groupsLoadingstate- Cache invalidation covers both the specific group and the rules prefix
apps/web/lib/api/groups/find-groups-with-matching-rules.ts (3)
55-59: Verify: Rule sets with no shared attributes may still conflict.The comment on line 34-35 states rule sets conflict "if there exists ANY set of attribute values that would satisfy both simultaneously." However, when rule sets have no shared attributes (e.g., Group A:
leads >= 5, Group B:conversions >= 10), the function returnsfalse(no conflict).A partner with
leads=10ANDconversions=20would satisfy both rule sets simultaneously. Unless the system evaluates rules such that only one group can ever match per attribute type, this logic may produce false negatives.Please confirm whether this is the intended behavior. If rules on different attributes should be allowed to coexist (e.g., evaluated in priority order), the current logic is correct. Otherwise, this needs adjustment.
5-32: LGTM on the main function structure.The
findGroupsWithMatchingRulesfunction is well-structured with appropriate early returns for empty inputs, proper filtering logic, and clean mapping to the result type.
107-124: LGTM on interval overlap logic.The
doConditionsOverlapfunction correctly implements standard interval overlap detection using themin1 <= max2 && min2 <= max1formula.
…rmanceConditionSchema, update related types and API functions for improved validation and consistency
There was a problem hiding this comment.
Actionable comments posted: 4
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/zod/schemas/campaigns.ts (1)
115-123: Schema mismatch: query parameter validation inconsistent with data model.The
getCampaignsQuerySchemastill usesworkflowConditionSchemafor parsing thetriggerConditionquery parameter (line 120), but the actual campaign data model now usescampaignTriggerConditionSchema(lines 64, 97). This inconsistency may cause:
- Query filters that don't match the stored data structure
- Runtime validation errors when filtering campaigns
- Inability to query campaigns using the new schema format
🔎 Proposed fix to align query schema with data model
triggerCondition: z .string() .pipe( z.preprocess( (input: string) => JSON.parse(input), - workflowConditionSchema, + campaignTriggerConditionSchema, ), ) .optional(),apps/web/lib/api/workflows/execute-send-campaign-workflow.ts (1)
312-333: Scheduled workflow path assumespartnerJoinedcondition without validation.When
partnerIdis not provided, the function uses a date-based query that implicitly assumes:
condition.attributeis"partnerJoined"condition.valuerepresents days- No condition evaluation is needed—all enrollments within the date range match
If this function is called with a different condition type (e.g.,
totalLeads > 100), the logic would be incorrect. Consider adding validation or documentation to clarify that scheduled workflows only supportpartnerJoinedconditions.🔎 Proposed validation to prevent incorrect usage
+ if (condition.attribute !== "partnerJoined") { + throw new Error( + `Scheduled workflows without partnerId only support partnerJoined conditions, got: ${condition.attribute}` + ); + } + const startDate = subDays(new Date(), condition.value); // add 12 hours to the start date since we run the partnerEnrolled workflow every 12 hours const endDate = addHours(startDate, 12);
🧹 Nitpick comments (1)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx (1)
124-130: Optional: Formatting change has no functional impact.The arrow function syntax change for
requireImageandrequireUrlinitializers is purely cosmetic. Both the original and new versions produce identical results.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsxapps/web/lib/api/bounties/generate-performance-bounty-name.tsapps/web/lib/api/workflows/execute-send-campaign-workflow.tsapps/web/lib/types.tsapps/web/lib/zod/schemas/bounties.tsapps/web/lib/zod/schemas/campaigns.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/lib/types.ts
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-12T17:36:09.551Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2833
File: apps/web/app/(ee)/api/bounties/[bountyId]/route.ts:100-107
Timestamp: 2025-09-12T17:36:09.551Z
Learning: For performance bounties in the bounty system, names cannot be provided by users - they are always auto-generated based on the performance condition using generateBountyName(). This ensures consistency and clarity about what the bounty actually measures. Any provided name should be overridden for performance bounties when a performanceCondition exists.
Applied to files:
apps/web/lib/api/bounties/generate-performance-bounty-name.ts
📚 Learning: 2025-12-18T16:55:47.136Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:47.136Z
Learning: In bounty creation logic, prohibit using workflow attributes partnerEnrolledDays and partnerJoined. Workflow attributes should be limited to those available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions. Apply this rule to all bounty creation endpoints or handlers (e.g., any file under apps/.../bounties/ that creates or drafts submissions), not just the specific route. This ensures consistency with the WORKFLOW_ATTRIBUTES schema subset across the codebase.
Applied to files:
apps/web/lib/api/bounties/generate-performance-bounty-name.ts
📚 Learning: 2025-09-12T17:31:10.548Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2833
File: apps/web/lib/actions/partners/approve-bounty-submission.ts:53-61
Timestamp: 2025-09-12T17:31:10.548Z
Learning: In approve-bounty-submission.ts, the logic `bounty.rewardAmount ?? rewardAmount` is intentional. Bounties with preset reward amounts should use those fixed amounts, and the rewardAmount override parameter is only used when bounty.rewardAmount is null/undefined (for custom reward bounties). This follows the design pattern where bounties are either "flat rate" (fixed amount) or "custom" (variable amount set during approval).
Applied to files:
apps/web/lib/api/bounties/generate-performance-bounty-name.tsapps/web/lib/zod/schemas/bounties.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx
📚 Learning: 2025-07-30T15:25:13.936Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2673
File: apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx:56-66
Timestamp: 2025-07-30T15:25:13.936Z
Learning: In apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition objects to allow users to add empty/unconfigured condition fields without type errors, while submission validation uses strict schemas to ensure data integrity. This two-stage validation pattern improves UX by allowing progressive completion of complex forms.
Applied to files:
apps/web/lib/zod/schemas/campaigns.tsapps/web/lib/zod/schemas/bounties.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx
📚 Learning: 2025-12-18T16:55:57.098Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:57.098Z
Learning: In the bounty system (e.g., apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts), bounties cannot be created with workflow attributes `partnerEnrolledDays` and `partnerJoined`. Bounty workflows only support attributes available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions, which is a subset of the general WORKFLOW_ATTRIBUTES schema.
Applied to files:
apps/web/lib/zod/schemas/campaigns.tsapps/web/lib/zod/schemas/bounties.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx
📚 Learning: 2025-08-25T17:33:45.072Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
Applied to files:
apps/web/lib/zod/schemas/campaigns.ts
📚 Learning: 2025-08-26T14:32:33.851Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2736
File: apps/web/lib/actions/partners/create-bounty-submission.ts:105-112
Timestamp: 2025-08-26T14:32:33.851Z
Learning: Non-performance bounties are required to have submissionRequirements. In create-bounty-submission.ts, it's appropriate to let the parsing fail if submissionRequirements is null for non-performance bounties, as this indicates a data integrity issue that should be caught.
Applied to files:
apps/web/lib/zod/schemas/bounties.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx
📚 Learning: 2025-07-30T15:29:54.131Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2673
File: apps/web/ui/partners/rewards/rewards-logic.tsx:268-275
Timestamp: 2025-07-30T15:29:54.131Z
Learning: In apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward condition, dependent fields (attribute, operator, value) should be reset rather than preserved because different entities (customer vs sale) have different available attributes. Maintaining existing fields when the entity changes would create invalid state combinations and confusing UX.
Applied to files:
apps/web/lib/zod/schemas/bounties.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx
📚 Learning: 2025-08-26T14:20:23.943Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2736
File: apps/web/app/api/workspaces/[idOrSlug]/notification-preferences/route.ts:13-14
Timestamp: 2025-08-26T14:20:23.943Z
Learning: The updateNotificationPreference action in apps/web/lib/actions/update-notification-preference.ts already handles all notification preference types dynamically, including newBountySubmitted, through its schema validation using the notificationTypes enum and Prisma's dynamic field update pattern.
Applied to files:
apps/web/lib/zod/schemas/bounties.ts
🧬 Code graph analysis (5)
apps/web/lib/api/bounties/generate-performance-bounty-name.ts (1)
apps/web/lib/types.ts (1)
BountyPerformanceCondition(630-632)
apps/web/lib/zod/schemas/campaigns.ts (1)
apps/web/lib/zod/schemas/workflows.ts (1)
WORKFLOW_ATTRIBUTES(9-16)
apps/web/lib/zod/schemas/bounties.ts (1)
apps/web/lib/zod/schemas/workflows.ts (1)
WORKFLOW_ATTRIBUTES(9-16)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx (1)
apps/web/lib/zod/schemas/bounties.ts (1)
bountyPerformanceConditionSchema(20-24)
apps/web/lib/api/workflows/execute-send-campaign-workflow.ts (1)
apps/web/lib/api/workflows/evaluate-workflow-conditions.ts (1)
evaluateWorkflowConditions(4-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
apps/web/lib/zod/schemas/campaigns.ts (3)
11-11: LGTM!The imports are correctly added and used within the file.
49-53: Verify if restricting the operator to only "gte" is intentional.The new schema only allows the "gte" (greater than or equal) operator. This restricts campaign trigger conditions to a single comparison type. Consider whether other operators (e.g., "lte", "eq", "lt", "gt") might be needed for various campaign scenarios, such as triggering when a metric falls below a threshold or equals a specific value.
If this restriction is intentional for the group-move workflow feature, please confirm. Otherwise, consider supporting additional operators.
64-64: LGTM!The consistent use of
campaignTriggerConditionSchemain bothCampaignSchemaandupdateCampaignSchemaaligns the trigger condition validation across create and update operations.Also applies to: 97-97
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx (1)
14-17: LGTM! Schema import updated correctly.The change from
workflowConditionSchematobountyPerformanceConditionSchemais appropriate as it uses a bounty-specific schema rather than the generic workflow schema. The usage at line 552 is consistent with this import change.Also applies to: 552-554
apps/web/lib/zod/schemas/bounties.ts (1)
76-76: Verify schema usage is correct after fixing allowed attributes.Once the
bountyPerformanceConditionSchemais updated to use the restricted attribute set, ensure these usages still provide the correct validation for bounty creation and retrieval.Also applies to: 106-108
apps/web/lib/api/bounties/generate-performance-bounty-name.ts (1)
1-1: LGTM! Type updated to use bounty-specific condition.The type change from
WorkflowConditiontoBountyPerformanceConditionappropriately narrows the function's type signature to use the bounty-specific performance condition type. The function logic remains unchanged and continues to work correctly with the new type.Also applies to: 11-11
apps/web/lib/api/workflows/execute-send-campaign-workflow.ts (1)
1-8: LGTM! Import changes align with the new evaluation approach.The updated imports correctly reflect the transition from
evaluateWorkflowConditiontoevaluateWorkflowConditionsand the introduction ofCampaignTriggerConditiontype.
| const { programId, partnerId } = context?.identity || { | ||
| programId: workflow.programId, | ||
| partnerId: undefined, | ||
| }; |
There was a problem hiding this comment.
Fallback logic may not trigger if context.identity is an empty object.
The current destructuring with || fallback only applies when context?.identity is nullish (null/undefined). If context.identity exists but lacks programId, the variable will be undefined instead of falling back to workflow.programId, potentially causing database query failures.
🔎 Proposed fix to ensure programId fallback
- const { programId, partnerId } = context?.identity || {
- programId: workflow.programId,
- partnerId: undefined,
- };
+ const programId = context?.identity?.programId ?? workflow.programId;
+ const partnerId = context?.identity?.partnerId;📝 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.
| const { programId, partnerId } = context?.identity || { | |
| programId: workflow.programId, | |
| partnerId: undefined, | |
| }; | |
| const programId = context?.identity?.programId ?? workflow.programId; | |
| const partnerId = context?.identity?.partnerId; |
🤖 Prompt for AI Agents
In apps/web/lib/api/workflows/execute-send-campaign-workflow.ts around lines 38
to 41, the current destructuring uses context?.identity || { ... } which fails
to fallback when context.identity exists but is an empty object; change the
logic to individually fallback each property (e.g. programId =
context?.identity?.programId ?? workflow.programId and partnerId =
context?.identity?.partnerId ?? undefined) so missing properties on an existing
identity object correctly use the intended defaults.
| partnerId, | ||
| groupIds: campaign.groups.map(({ groupId }) => groupId), | ||
| condition, | ||
| condition: condition as CampaignTriggerCondition, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the target file
fd "execute-send-campaign-workflow.ts" --type fRepository: dubinc/dub
Length of output: 114
🏁 Script executed:
# Read the file to see the context around line 75
cat -n apps/web/lib/api/workflows/execute-send-campaign-workflow.ts | head -90 | tail -40Repository: dubinc/dub
Length of output: 1358
🏁 Script executed:
# Find parseWorkflowConfig function definition
rg -n "parseWorkflowConfig" --type ts -B 2 -A 10Repository: dubinc/dub
Length of output: 21179
🏁 Script executed:
# Search for CampaignTriggerCondition type definition
rg -n "type CampaignTriggerCondition|interface CampaignTriggerCondition" --type ts -A 5Repository: dubinc/dub
Length of output: 381
🏁 Script executed:
# Search for WorkflowCondition type definition
rg -n "type WorkflowCondition|interface WorkflowCondition" --type ts -A 5Repository: dubinc/dub
Length of output: 777
🏁 Script executed:
# Read the full parse-workflow-config.ts file to understand the return type
cat -n apps/web/lib/api/workflows/parse-workflow-config.tsRepository: dubinc/dub
Length of output: 989
🏁 Script executed:
# Find the schema definitions for workflowConditionSchema and campaignTriggerConditionSchema
rg -n "workflowConditionSchema|campaignTriggerConditionSchema" --type ts -B 2 -A 10 | head -100Repository: dubinc/dub
Length of output: 6041
🏁 Script executed:
# Check how condition is used in the file to understand its actual type
rg -n "const { condition" --type ts -A 3 apps/web/lib/api/workflows/Repository: dubinc/dub
Length of output: 161
🏁 Script executed:
# Find campaignTriggerConditionSchema definition
rg -n "campaignTriggerConditionSchema" --type ts -B 2 -A 15 apps/web/lib/zod/schemas/campaigns.tsRepository: dubinc/dub
Length of output: 1724
🏁 Script executed:
# Check getProgramEnrollments function signature to see what it expects
rg -n "getProgramEnrollments" --type ts -B 2 -A 10 | head -60Repository: dubinc/dub
Length of output: 2364
🏁 Script executed:
# Compare the two schema definitions side by side
rg -n "workflowConditionSchema|campaignTriggerConditionSchema" --type ts -A 12Repository: dubinc/dub
Length of output: 19967
Remove unsafe type cast or add runtime validation.
The cast condition as CampaignTriggerCondition is unsafe. parseWorkflowConfig returns a WorkflowCondition which allows any operator from WORKFLOW_COMPARISON_OPERATORS and value as number | {min: number, max: number}, while CampaignTriggerCondition restricts operator to "gte" and value to only number. Without runtime validation, a workflow condition with a different operator or range value could bypass type checking and cause runtime issues in getProgramEnrollments. Either validate that the condition conforms to CampaignTriggerCondition before passing it, or update the function signature to accept the broader WorkflowCondition type.
🤖 Prompt for AI Agents
In apps/web/lib/api/workflows/execute-send-campaign-workflow.ts around line 75,
the code unsafely casts condition as CampaignTriggerCondition; instead either
perform a runtime validation that condition.operator === "gte" and typeof
condition.value === "number" (or convert a {min,max} range to a number/choose
behavior) and throw/return an error if it doesn't match, or change the called
function signature to accept WorkflowCondition; implement a small type-guard
function isCampaignTriggerCondition(cond): cond is CampaignTriggerCondition and
use it before calling getProgramEnrollments, returning a clear error or mapping
the condition to the expected shape when validation fails.
| partnerId?: string; | ||
| groupIds: string[]; | ||
| condition: WorkflowCondition; | ||
| condition: CampaignTriggerCondition; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find type definitions for CampaignTriggerCondition and WorkflowCondition
rg -n 'type CampaignTriggerCondition|type WorkflowCondition' -A 5 -t tsRepository: dubinc/dub
Length of output: 970
🏁 Script executed:
# Find the evaluateWorkflowConditions function signature
rg -n 'evaluateWorkflowConditions' -B 2 -A 8 -t tsRepository: dubinc/dub
Length of output: 9743
🏁 Script executed:
# Check if there are schema definitions
rg -n 'campaignTriggerConditionSchema|workflowConditionSchema' -A 5 -t tsRepository: dubinc/dub
Length of output: 9919
🏁 Script executed:
# Get the function signature with parameter at line 233
sed -n '220,240p' apps/web/lib/api/workflows/execute-send-campaign-workflow.tsRepository: dubinc/dub
Length of output: 460
Fix type mismatch: CampaignTriggerCondition passed to function expecting WorkflowCondition[].
The parameter condition at line 233 is typed as CampaignTriggerCondition, but at line 299 it's passed to evaluateWorkflowConditions which expects conditions: WorkflowCondition[]. These are two distinct types: campaignTriggerConditionSchema restricts the operator to "gte" and value to numbers only, while workflowConditionSchema supports multiple operators and object values. Either the parameter type should be changed to WorkflowCondition, or the function signature should be updated to accept CampaignTriggerCondition.
🤖 Prompt for AI Agents
In apps/web/lib/api/workflows/execute-send-campaign-workflow.ts around line 233,
the parameter `condition: CampaignTriggerCondition` is a type mismatch because
at line 299 it’s passed into evaluateWorkflowConditions which expects
`WorkflowCondition[]`; change the parameter to the correct type (e.g.
`conditions: WorkflowCondition[]`) or accept/convert to that shape before
calling evaluateWorkflowConditions (map the CampaignTriggerCondition(s) into
WorkflowCondition objects or update the function signature to take
CampaignTriggerCondition[] and perform the conversion), and update any call
sites and variable names accordingly so the value passed at line 299 matches
`WorkflowCondition[]`.
| export const bountyPerformanceConditionSchema = z.object({ | ||
| attribute: z.enum(WORKFLOW_ATTRIBUTES), | ||
| operator: z.literal("gte").default("gte"), | ||
| value: z.number(), | ||
| }); |
There was a problem hiding this comment.
Critical: bountyPerformanceConditionSchema allows prohibited attributes.
The schema uses the full WORKFLOW_ATTRIBUTES enum, which includes partnerEnrolledDays and partnerJoined. However, based on learnings, bounties should only support attributes available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions. Using the unrestricted enum allows invalid bounty conditions to be created.
🔎 Recommended fix: Create a restricted attribute set for bounties
Define a bounty-specific attributes constant in this file:
import { WORKFLOW_ATTRIBUTES } from "./workflows";
+
+// Bounties only support attributes from partner link stats and commissions
+export const BOUNTY_PERFORMANCE_ATTRIBUTES = [
+ "totalLeads",
+ "totalConversions",
+ "totalSaleAmount",
+ "totalCommissions",
+] as const;
export const bountyPerformanceConditionSchema = z.object({
- attribute: z.enum(WORKFLOW_ATTRIBUTES),
+ attribute: z.enum(BOUNTY_PERFORMANCE_ATTRIBUTES),
operator: z.literal("gte").default("gte"),
value: z.number(),
});Based on learnings, bounties cannot use partnerEnrolledDays and partnerJoined attributes - these should be excluded from the bounty schema.
📝 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.
| export const bountyPerformanceConditionSchema = z.object({ | |
| attribute: z.enum(WORKFLOW_ATTRIBUTES), | |
| operator: z.literal("gte").default("gte"), | |
| value: z.number(), | |
| }); | |
| import { WORKFLOW_ATTRIBUTES } from "./workflows"; | |
| // Bounties only support attributes from partner link stats and commissions | |
| export const BOUNTY_PERFORMANCE_ATTRIBUTES = [ | |
| "totalLeads", | |
| "totalConversions", | |
| "totalSaleAmount", | |
| "totalCommissions", | |
| ] as const; | |
| export const bountyPerformanceConditionSchema = z.object({ | |
| attribute: z.enum(BOUNTY_PERFORMANCE_ATTRIBUTES), | |
| operator: z.literal("gte").default("gte"), | |
| value: z.number(), | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/lib/types.ts (1)
645-648: Consider more descriptive parameter names.The parameter names
aVandcVare cryptic. Consider renaming them for better readability, such asactualValueandcomparisonValue, orvalueandthreshold.🔎 Proposed refactor with clearer names
-export type OperatorFn = ( - aV: number, - cV: number | { min: number; max?: number }, -) => boolean; +export type OperatorFn = ( + actualValue: number, + comparisonValue: number | { min: number; max?: number }, +) => boolean;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/lib/types.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-11-24T09:10:12.536Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3089
File: apps/web/lib/api/fraud/fraud-rules-registry.ts:17-25
Timestamp: 2025-11-24T09:10:12.536Z
Learning: In apps/web/lib/api/fraud/fraud-rules-registry.ts, the fraud rules `partnerCrossProgramBan` and `partnerDuplicatePayoutMethod` intentionally have stub implementations that return `{ triggered: false }` because they are partner-scoped rules handled separately during partner application/onboarding flows (e.g., in detect-record-fraud-application.ts), rather than being evaluated per conversion event like other rules in the registry. The stubs exist only to satisfy the `Record<FraudRuleType, ...>` type constraint.
Applied to files:
apps/web/lib/types.ts
📚 Learning: 2025-08-25T17:33:45.072Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
Applied to files:
apps/web/lib/types.ts
📚 Learning: 2025-08-26T14:32:33.851Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2736
File: apps/web/lib/actions/partners/create-bounty-submission.ts:105-112
Timestamp: 2025-08-26T14:32:33.851Z
Learning: Non-performance bounties are required to have submissionRequirements. In create-bounty-submission.ts, it's appropriate to let the parsing fail if submissionRequirements is null for non-performance bounties, as this indicates a data integrity issue that should be caught.
Applied to files:
apps/web/lib/types.ts
📚 Learning: 2025-07-30T15:25:13.936Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2673
File: apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx:56-66
Timestamp: 2025-07-30T15:25:13.936Z
Learning: In apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx, the form schema uses partial condition objects to allow users to add empty/unconfigured condition fields without type errors, while submission validation uses strict schemas to ensure data integrity. This two-stage validation pattern improves UX by allowing progressive completion of complex forms.
Applied to files:
apps/web/lib/types.ts
📚 Learning: 2025-08-26T14:20:23.943Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2736
File: apps/web/app/api/workspaces/[idOrSlug]/notification-preferences/route.ts:13-14
Timestamp: 2025-08-26T14:20:23.943Z
Learning: The updateNotificationPreference action in apps/web/lib/actions/update-notification-preference.ts already handles all notification preference types dynamically, including newBountySubmitted, through its schema validation using the notificationTypes enum and Prisma's dynamic field update pattern.
Applied to files:
apps/web/lib/types.ts
📚 Learning: 2025-12-18T16:55:57.098Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:57.098Z
Learning: In the bounty system (e.g., apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts), bounties cannot be created with workflow attributes `partnerEnrolledDays` and `partnerJoined`. Bounty workflows only support attributes available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions, which is a subset of the general WORKFLOW_ATTRIBUTES schema.
Applied to files:
apps/web/lib/types.ts
📚 Learning: 2025-12-08T09:44:28.429Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3200
File: apps/web/lib/api/fraud/detect-duplicate-payout-method-fraud.ts:55-73
Timestamp: 2025-12-08T09:44:28.429Z
Learning: In apps/web/lib/api/fraud/detect-duplicate-payout-method-fraud.ts, the fraud event creation logic intentionally generates self-referential fraud events (where partnerId equals duplicatePartnerId) for partners with duplicate payout methods. This is by design to create raw events for all partners involved in a duplicate payout method scenario, regardless of whether they reference themselves.
Applied to files:
apps/web/lib/types.ts
🧬 Code graph analysis (1)
apps/web/lib/types.ts (4)
apps/web/lib/zod/schemas/bounties.ts (1)
bountyPerformanceConditionSchema(20-24)apps/web/lib/zod/schemas/campaigns.ts (1)
campaignTriggerConditionSchema(49-53)apps/web/lib/zod/schemas/workflows.ts (3)
WORKFLOW_ATTRIBUTES(9-16)WORKFLOW_COMPARISON_OPERATORS(30-30)workflowActionSchema(102-123)packages/prisma/client.ts (1)
WorkflowTrigger(36-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
apps/web/lib/types.ts (5)
25-25: LGTM: New imports are properly utilized.The new imports for
WorkflowTrigger,bountyPerformanceConditionSchema, andcampaignTriggerConditionSchemaare all used later in the file to define new exported types.Also applies to: 39-39, 48-48
630-636: LGTM: Type aliases follow established patterns.The new type aliases for bounty and campaign conditions are properly derived from their respective zod schemas and follow the existing convention in this file.
739-743: LGTM: Metadata field adds extensibility.The addition of the optional
metadatafield toCreateFraudEventInputmaintains backward compatibility while allowing richer fraud event data.
745-766: All usages of WorkflowContext have been properly updated for the new structure.The restructuring with nested
identityandmetricsobjects has been correctly implemented across all consuming code:
execute-workflows.ts: Properly destructures and constructs the nested structureexecute-send-campaign-workflow.ts: Uses safe optional chaining with fallback valuesexecute-complete-bounty-workflow.ts: Correctly accesses metrics.current propertiesexecute-move-group-workflow.ts: Correctly accesses metrics.aggregated propertiesAll property access patterns follow the new structure consistently—no breaking patterns remain.
645-648: This is not a breaking change. AllOperatorFnimplementations inOPERATOR_FUNCTIONSalready handle the new union type signature correctly:gtevalidates thatcVis a number, andbetweenvalidates thatcVis an object. The schema validation ensures thatcondition.valuematches the expected union type before being passed to these functions, making this change fully backward compatible.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 4
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)
486-491: VerifyfirstConversionFlagcalculation when using existing events.When
useExistingEventsistrue, thefirstConversionFlagis calculated using the originalcustomerobject (line 486-491), but the commissions are created for aduplicateCustomerId(line 280). The duplicate customer is created withsales = totalSaleswhenrecordSaleEventsis true (line 337), meaning it can have multiple sales transferred to it. However,firstConversionFlagis still determined by checking the original customer's state against the newlinkId, which may incorrectly indicate this is a first conversion even when the duplicate customer has multiple sales.This can cause
metrics.current.conversions(line 574) to be inaccurate, potentially affecting workflow triggers.
🤖 Fix all issues with AI agents
In
@apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx:
- Around line 111-116: The catch block uses error.message but caught exceptions
are unknown in TypeScript; update the catch to narrow the type before using
message (for example, check if error instanceof Error and call
toast.error(error.message), otherwise call toast.error(String(error)) or a
generic message); apply this change around the validateGroupMoveRules(...)
try/catch where toast.error is invoked so access to error.message is type-safe.
- Line 9: The import for useGroupsRules includes a .ts extension which is
invalid for ESM/TS imports; update the import statement that references
useGroupsRules (symbol: useGroupsRules) to remove the ".ts" suffix (use
"@/lib/swr/use-groups-rules" instead) and check for any other imports in this
file using explicit .ts extensions and remove them to match standard
TypeScript/ESM import semantics.
In
@apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-settings.tsx:
- Around line 208-213: The skeleton currently always renders the SettingsRow for
"Group slug" while the real form conditionally omits it when group.slug ===
DEFAULT_PARTNER_GROUP.slug, causing layout shift; update the skeleton to match
the form's conditional rendering by either (preferred simple) also always
rendering the row only when the real form would (i.e., replicate the condition
used in the form: check group.slug against DEFAULT_PARTNER_GROUP.slug before
rendering the SettingsRow in the skeleton), or (if you want no data dependency
in skeleton) fetch/evaluate the group slug earlier so the skeleton can use the
same condition; target the SettingsRow usage and the group.slug /
DEFAULT_PARTNER_GROUP.slug check to implement the change.
In @apps/web/lib/actions/partners/create-manual-commission.ts:
- Line 572: metrics.current.leads is computed with commissionType === "lead" but
elsewhere the code always creates a lead event and increments link.leads,
causing inconsistency; update the leads metric calculation in
create-manual-commission.ts to reflect whether a lead event was actually created
or link.leads was incremented (use the same boolean/flag used when creating the
lead event or the result of that operation instead of commissionType), and
ensure the same logic is used when building the metrics payload (the expression
around leads: ...) so metrics.current.leads increments when a lead
event/link.leads is incremented.
🧹 Nitpick comments (6)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (1)
97-101: Form does not reset whengroup.moveRuleschanges.The
useFormhook only uses the initialgroup.moveRulesvalue asdefaultValues. If the group data is refetched or changes externally (e.g., after another user modifies it), the form won't reflect those updates. Consider adding auseEffectto reset the form whengroup.moveRuleschanges.Proposed fix
const form = useForm<FormData>({ defaultValues: { moveRules: group.moveRules ?? [], }, }); +useEffect(() => { + reset({ moveRules: group.moveRules ?? [] }); +}, [group.moveRules, reset]);apps/web/lib/api/groups/find-groups-with-matching-rules.ts (1)
93-100: Consider validatingminandmaxproperties exist before accessing.The code checks that
condition.valueis a non-null object, but doesn't verify thatminandmaxproperties exist and are numbers. If the data is malformed, this could return an interval withundefinedvalues, causing incorrect overlap calculations.Proposed defensive check
case "between": if (typeof condition.value === "object" && condition.value !== null) { + const { min, max } = condition.value as { min?: number; max?: number }; + if (typeof min !== "number" || typeof max !== "number") { + return null; + } return { - min: condition.value.min, - max: condition.value.max, + min, + max, }; } return null;apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts (2)
135-137: Consider adding validation for empty conditions array.If
bounty.workflow.triggerConditionsis an empty array,conditionwill beundefined, causing a runtime error at line 151 when accessingcondition.attribute.While existing bounties likely have at least one condition, adding defensive validation would improve robustness.
🛡️ Proposed defensive validation
// Find the workflow condition -const condition = z +const conditions = z .array(workflowConditionSchema) - .parse(bounty.workflow.triggerConditions)[0]; + .parse(bounty.workflow.triggerConditions); + +if (conditions.length === 0) { + return logAndRespond( + `Bounty ${bountyId} workflow has no conditions.`, + { logLevel: "error" } + ); +} + +const condition = conditions[0];
155-160: Function call correctly updated; consider leveraging multi-condition capability.The function call correctly adopts the new
evaluateWorkflowConditionsAPI. However, the current pattern extracts only the first condition (line 137) and then wraps it back in an array, which doesn't fully leverage the multi-condition evaluation capability.If bounty workflows should support multiple conditions, consider passing all conditions directly instead of just the first one. Otherwise, the current single-condition behavior is maintained.
💡 Alternative: Evaluate all conditions
If bounties should support multiple workflow conditions:
-// Find the workflow condition -const condition = z +// Find the workflow conditions +const conditions = z .array(workflowConditionSchema) - .parse(bounty.workflow.triggerConditions)[0]; + .parse(bounty.workflow.triggerConditions); + +if (conditions.length === 0) { + return logAndRespond(`Bounty ${bountyId} workflow has no conditions.`, { + logLevel: "error", + }); +} + +// Use first condition for attribute extraction (or adjust logic for multiple) +const primaryCondition = conditions[0]; // Partners with their link metrics const partners = programEnrollments.map((partner) => { return { id: partner.partnerId, ...aggregatePartnerLinksStats(partner.links), totalCommissions: partner.totalCommissions, }; }); const bountySubmissionsToCreate: Prisma.BountySubmissionCreateManyInput[] = partners - // only create submissions for partners that have at least 1 performanceCount - .filter((partner) => partner[condition.attribute] > 0) + .filter((partner) => partner[primaryCondition.attribute] > 0) .map((partner) => { - const performanceCount = partner[condition.attribute]; + const performanceCount = partner[primaryCondition.attribute]; const conditionMet = evaluateWorkflowConditions({ - conditions: [condition], + conditions: conditions, attributes: { - [condition.attribute]: performanceCount, + [primaryCondition.attribute]: performanceCount, }, });Note: If supporting multiple conditions with different attributes, the attributes object construction (lines 157-159) may need adjustment to include all referenced attributes.
apps/web/lib/api/conversions/track-lead.ts (1)
322-334: Consider using a string constant for the trigger value.The migration to a consolidated
"partnerMetricsUpdated"trigger with areasondiscriminator is a good approach for flexibility. Theidentityandmetricsfields provide proper context for workflow evaluation.To prevent typos and improve maintainability, consider defining the trigger string as a constant:
♻️ Recommended: Define trigger as a constant
Create a constants file (e.g.,
lib/workflows/constants.ts):export const WORKFLOW_TRIGGERS = { PARTNER_METRICS_UPDATED: "partnerMetricsUpdated", // ... other triggers } as const; export const WORKFLOW_REASONS = { LEAD: "lead", SALE: "sale", COMMISSION: "commission", } as const;Then update the usage:
await executeWorkflows({ - trigger: "partnerMetricsUpdated", - reason: "lead", + trigger: WORKFLOW_TRIGGERS.PARTNER_METRICS_UPDATED, + reason: WORKFLOW_REASONS.LEAD, identity: { programId: link.programId, partnerId: link.partnerId, }, metrics: { current: { leads: 1, }, }, }),apps/web/lib/actions/partners/create-manual-commission.ts (1)
570-576: Consider adding sales count to workflow metrics.The
metrics.currentpayload includessaleAmountbut not asalescount. WhencommissionType === "sale", the link stats are updated to incrementlink.sales(line 521-527), and whenuseExistingEventsistrue, multiple sales can be created (up to 5, as validated on line 245-249).Workflows may need to evaluate conditions based on total sales count (e.g., "partner reached 10 sales"), not just sale amount or first conversions. Consider adding a
salesfield to the metrics payload for consistency with the link stats tracking.💡 Suggested enhancement
metrics: { current: { leads: commissionType === "lead" ? 1 : 0, saleAmount: saleAmount ?? totalSaleAmount, + sales: commissionType === "sale" ? (saleAmount ? 1 : totalSales) : 0, conversions: firstConversionFlag ? 1 : 0, }, },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.tsapps/web/app/(ee)/api/groups/[groupIdOrSlug]/partners/route.tsapps/web/app/(ee)/api/workflows/partner-approved/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-settings.tsxapps/web/lib/actions/partners/create-manual-commission.tsapps/web/lib/api/conversions/track-lead.tsapps/web/lib/api/conversions/track-sale.tsapps/web/lib/api/get-workspace-users.tsapps/web/lib/api/groups/find-groups-with-matching-rules.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/app/(ee)/api/workflows/partner-approved/route.ts
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty-sheet.tsx
- apps/web/lib/api/get-workspace-users.ts
- apps/web/app/(ee)/api/groups/[groupIdOrSlug]/partners/route.ts
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-12-18T16:55:47.136Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:47.136Z
Learning: In bounty creation logic, prohibit using workflow attributes partnerEnrolledDays and partnerJoined. Workflow attributes should be limited to those available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions. Apply this rule to all bounty creation endpoints or handlers (e.g., any file under apps/.../bounties/ that creates or drafts submissions), not just the specific route. This ensures consistency with the WORKFLOW_ATTRIBUTES schema subset across the codebase.
Applied to files:
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts
📚 Learning: 2025-09-12T17:31:10.548Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2833
File: apps/web/lib/actions/partners/approve-bounty-submission.ts:53-61
Timestamp: 2025-09-12T17:31:10.548Z
Learning: In approve-bounty-submission.ts, the logic `bounty.rewardAmount ?? rewardAmount` is intentional. Bounties with preset reward amounts should use those fixed amounts, and the rewardAmount override parameter is only used when bounty.rewardAmount is null/undefined (for custom reward bounties). This follows the design pattern where bounties are either "flat rate" (fixed amount) or "custom" (variable amount set during approval).
Applied to files:
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts
📚 Learning: 2025-08-26T14:32:33.851Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2736
File: apps/web/lib/actions/partners/create-bounty-submission.ts:105-112
Timestamp: 2025-08-26T14:32:33.851Z
Learning: Non-performance bounties are required to have submissionRequirements. In create-bounty-submission.ts, it's appropriate to let the parsing fail if submissionRequirements is null for non-performance bounties, as this indicates a data integrity issue that should be caught.
Applied to files:
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts
📚 Learning: 2025-12-15T16:45:51.667Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:45:51.667Z
Learning: In cron endpoints under apps/web/app/(ee)/api/cron, continue using handleCronErrorResponse for error handling. Do not detect QStash callbacks or set Upstash-NonRetryable-Error headers in these cron routes, so QStash can retry cron errors via its native retry mechanism. The existing queueFailedRequestForRetry logic should remain limited to specific user-facing API endpoints (e.g., /api/track/lead, /api/track/sale, /api/links) to retry only transient Prisma/database errors. This pattern should apply to all cron endpoints under the cron directory in this codebase.
Applied to files:
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts
📚 Learning: 2025-08-25T17:33:45.072Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2736
File: apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:12-12
Timestamp: 2025-08-25T17:33:45.072Z
Learning: The WorkflowTrigger enum in packages/prisma/schema/workflow.prisma contains three values: leadRecorded, saleRecorded, and commissionEarned. All three are properly used throughout the codebase.
Applied to files:
apps/web/lib/actions/partners/create-manual-commission.tsapps/web/lib/api/conversions/track-sale.tsapps/web/lib/api/conversions/track-lead.ts
📚 Learning: 2025-12-18T16:55:57.098Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3244
File: apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts:148-174
Timestamp: 2025-12-18T16:55:57.098Z
Learning: In the bounty system (e.g., apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts), bounties cannot be created with workflow attributes `partnerEnrolledDays` and `partnerJoined`. Bounty workflows only support attributes available from partner link stats (clicks, sales, leads, conversions, saleAmount) and totalCommissions, which is a subset of the general WORKFLOW_ATTRIBUTES schema.
Applied to files:
apps/web/lib/actions/partners/create-manual-commission.tsapps/web/lib/api/conversions/track-sale.ts
📚 Learning: 2025-09-17T17:44:03.965Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2857
File: apps/web/lib/actions/partners/update-program.ts:96-0
Timestamp: 2025-09-17T17:44:03.965Z
Learning: In apps/web/lib/actions/partners/update-program.ts, the team prefers to keep the messagingEnabledAt update logic simple by allowing client-provided timestamps rather than implementing server-controlled timestamp logic to avoid added complexity.
Applied to files:
apps/web/lib/actions/partners/create-manual-commission.ts
📚 Learning: 2025-10-02T22:46:22.739Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2924
File: apps/web/lib/api/conversions/track-lead.ts:7-7
Timestamp: 2025-10-02T22:46:22.739Z
Learning: In apps/web/lib/api/conversions/track-lead.ts, lead events are cached in Redis for 5 minutes (keys: `leadCache:${customer.id}` and `leadCache:${customer.id}:${stringifiedEventName}`) to provide immediate data availability while Tinybird ingestion happens asynchronously. This caching pattern allows for async-only recording without breaking "wait" mode semantics.
Applied to files:
apps/web/lib/api/conversions/track-sale.tsapps/web/lib/api/conversions/track-lead.ts
📚 Learning: 2025-10-15T01:05:43.266Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2958
File: apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx:432-457
Timestamp: 2025-10-15T01:05:43.266Z
Learning: In apps/web/app/app.dub.co/(dashboard)/[slug]/settings/members/page-client.tsx, defer refactoring the custom MenuItem component (lines 432-457) to use the shared dub/ui MenuItem component to a future PR, as requested by steven-tey.
Applied to files:
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-settings.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
🧬 Code graph analysis (4)
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts (1)
apps/web/lib/api/workflows/evaluate-workflow-conditions.ts (1)
evaluateWorkflowConditions(4-34)
apps/web/lib/api/groups/find-groups-with-matching-rules.ts (2)
apps/web/lib/zod/schemas/groups.ts (1)
groupRulesSchema(190-192)apps/web/lib/types.ts (1)
WorkflowCondition(628-628)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-settings.tsx (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/settings-row.tsx (1)
SettingsRow(3-23)packages/email/src/react-email.d.ts (1)
Link(14-14)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (6)
apps/web/lib/zod/schemas/groups.ts (1)
updateGroupSchema(130-153)apps/web/lib/swr/use-groups-rules.ts (1)
useGroupsRules(9-25)apps/web/lib/api/groups/validate-group-move-rules.ts (1)
validateGroupMoveRules(3-61)apps/web/lib/api/groups/find-groups-with-matching-rules.ts (1)
findGroupsWithMatchingRules(5-32)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/settings-row.tsx (1)
SettingsRow(3-23)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-move-rules.tsx (1)
GroupMoveRules(43-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-settings.tsx (1)
201-232: Nice refactor to use SettingsRow components in skeleton.The skeleton now properly mirrors the structure of the actual form, including the Group ID description with the "Learn more" link. This improves consistency and makes the loading experience more accurate.
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx (3)
229-238: LGTM!The save button correctly reflects the form's dirty state and loading conditions. The separation between form-based move rules and modal-based settings (payout, auto-approve) is clean.
246-291: LGTM!The skeleton component properly mirrors the structure of the actual form with appropriate loading placeholders for each settings section.
293-428: LGTM!The confirmation modal components follow consistent patterns with proper state cleanup on close.
apps/web/lib/api/groups/find-groups-with-matching-rules.ts (3)
5-32: LGTM!The main function has proper null/empty guards and cleanly filters groups that have overlapping rules with the current group. The logic correctly excludes the current group from comparison.
36-78: LGTM!The overlap detection logic correctly identifies when two rule sets could potentially match the same set of attribute values. The approach of checking only shared attributes is appropriate—if all shared attributes have overlapping ranges, a partner could satisfy both rule sets simultaneously, which would create ambiguity.
107-124: LGTM!The interval overlap logic is mathematically correct. Two intervals [a,b] and [c,d] overlap if and only if
a <= d && c <= b, which this implements correctly.apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts (1)
3-3: LGTM: Import updated to multi-condition API.The import correctly adopts the new
evaluateWorkflowConditionsfunction that supports evaluating multiple workflow conditions.apps/web/lib/api/conversions/track-sale.ts (1)
540-553: Correct handling of sale metrics with conditional conversion count.The workflow trigger correctly distinguishes sale metrics from lead metrics:
saleAmounttracks the actual sale amountconversionsis conditionally set based onfirstConversionFlag, ensuring that only the first purchase from a customer increments the conversion countThis aligns well with the business logic for tracking unique conversions vs. repeat sales.
apps/web/lib/api/conversions/track-lead.ts (1)
19-19: The WorkflowTrigger enum is still actively used—this is not a complete migration to strings.The
WorkflowTriggerenum remains fully defined and imported across the codebase (inapps/web/lib/zod/schemas/workflows.ts,apps/web/lib/types.ts, and more). Whiletrack-lead.tsuses string literals forexecuteWorkflows()calls, this reflects inconsistent usage rather than a completed migration. The enum definition and type safety mechanisms are still present elsewhere, making the claim of a migration inaccurate.Likely an incorrect or invalid review comment.
apps/web/lib/actions/partners/create-manual-commission.ts (1)
564-564: No action required — the trigger value is valid.The
"partnerMetricsUpdated"string literal is a valid and registered trigger in theWorkflowTriggerenum defined inpackages/prisma/schema/workflow.prisma. It is used consistently across the codebase and will be properly handled by the workflow execution system at runtime. While using the enum value directly would provide compile-time type checking, the current string literal approach is an intentional pattern throughout the codebase.
| import { mutatePrefix } from "@/lib/swr/mutate"; | ||
| import { useApiMutation } from "@/lib/swr/use-api-mutation"; | ||
| import useGroup from "@/lib/swr/use-group"; | ||
| import useGroupsRules from "@/lib/swr/use-groups-rules.ts"; |
There was a problem hiding this comment.
Remove .ts extension from import path.
TypeScript/ESM imports should not include the .ts extension. This could cause issues depending on the bundler configuration.
-import useGroupsRules from "@/lib/swr/use-groups-rules.ts";
+import useGroupsRules from "@/lib/swr/use-groups-rules";📝 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.
| import useGroupsRules from "@/lib/swr/use-groups-rules.ts"; | |
| import useGroupsRules from "@/lib/swr/use-groups-rules"; |
🤖 Prompt for AI Agents
In
@apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
at line 9, The import for useGroupsRules includes a .ts extension which is
invalid for ESM/TS imports; update the import statement that references
useGroupsRules (symbol: useGroupsRules) to remove the ".ts" suffix (use
"@/lib/swr/use-groups-rules" instead) and check for any other imports in this
file using explicit .ts extensions and remove them to match standard
TypeScript/ESM import semantics.
| try { | ||
| validateGroupMoveRules(data.moveRules); | ||
| } catch (error) { | ||
| toast.error(error.message); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Add type narrowing for caught error.
In TypeScript, caught errors are typed as unknown. Accessing error.message directly will cause a type error. Add proper type narrowing.
try {
validateGroupMoveRules(data.moveRules);
} catch (error) {
- toast.error(error.message);
+ toast.error(error instanceof Error ? error.message : "Invalid move rules");
return;
}📝 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.
| try { | |
| validateGroupMoveRules(data.moveRules); | |
| } catch (error) { | |
| toast.error(error.message); | |
| return; | |
| } | |
| try { | |
| validateGroupMoveRules(data.moveRules); | |
| } catch (error) { | |
| toast.error(error instanceof Error ? error.message : "Invalid move rules"); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In
@apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-additional-settings.tsx
around lines 111 - 116, The catch block uses error.message but caught exceptions
are unknown in TypeScript; update the catch to narrow the type before using
message (for example, check if error instanceof Error and call
toast.error(error.message), otherwise call toast.error(String(error)) or a
generic message); apply this change around the validateGroupMoveRules(...)
try/catch where toast.error is invoked so access to error.message is type-safe.
| <SettingsRow | ||
| heading="Group slug" | ||
| description="For program landing page and internal group page URLs" | ||
| > | ||
| <div className="h-[38px] w-full animate-pulse rounded-md bg-neutral-200" /> | ||
| </SettingsRow> |
There was a problem hiding this comment.
Conditional rendering mismatch between skeleton and actual form.
The skeleton unconditionally renders the "Group slug" row, but the actual form only shows it when group.slug !== DEFAULT_PARTNER_GROUP.slug (Line 122). This will cause a visible layout shift when the skeleton is replaced by the loaded form for the default partner group.
🔧 Proposed fix to match conditional rendering
Since the skeleton doesn't have access to the group data, you have two options:
Option 1: Always show the row in the skeleton (accept minor layout shift for default group):
// No change needed - current implementationOption 2: Conditionally render in skeleton by fetching group slug early:
function GroupSettingsFormSkeleton() {
+ const { group } = useGroup();
+ const isDefaultGroup = group?.slug === DEFAULT_PARTNER_GROUP.slug;
+
return (
<div className="border-border-subtle rounded-lg border">
<div className="flex flex-col divide-y divide-neutral-200">
<div className="px-6 py-6">
<h3 className="text-content-emphasis text-lg font-semibold leading-7">
Group settings
</h3>
</div>
<SettingsRow
heading="Name"
description="For internal use only, never visible to partners."
>
<div className="h-[38px] w-full animate-pulse rounded-md bg-neutral-200" />
</SettingsRow>
- <SettingsRow
- heading="Group slug"
- description="For program landing page and internal group page URLs"
- >
- <div className="h-[38px] w-full animate-pulse rounded-md bg-neutral-200" />
- </SettingsRow>
+ {!isDefaultGroup && (
+ <SettingsRow
+ heading="Group slug"
+ description="For program landing page and internal group page URLs"
+ >
+ <div className="h-[38px] w-full animate-pulse rounded-md bg-neutral-200" />
+ </SettingsRow>
+ )}
<SettingsRowOption 1 is simpler and the layout shift is minor. Option 2 eliminates the shift but adds complexity.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/settings/group-settings.tsx
around lines 208 - 213, The skeleton currently always renders the SettingsRow
for "Group slug" while the real form conditionally omits it when group.slug ===
DEFAULT_PARTNER_GROUP.slug, causing layout shift; update the skeleton to match
the form's conditional rendering by either (preferred simple) also always
rendering the row only when the real form would (i.e., replicate the condition
used in the form: check group.slug against DEFAULT_PARTNER_GROUP.slug before
rendering the SettingsRow in the skeleton), or (if you want no data dependency
in skeleton) fetch/evaluate the group slug earlier so the skeleton can use the
same condition; target the SettingsRow usage and the group.slug /
DEFAULT_PARTNER_GROUP.slug check to implement the change.
| }, | ||
| metrics: { | ||
| current: { | ||
| leads: commissionType === "lead" ? 1 : 0, |
There was a problem hiding this comment.
Inconsistent leads metric calculation.
The metrics.current.leads is set to 0 when commissionType === "sale", but the code always creates a lead event (lines 205-239 or 372-383) and increments link.leads by 1 (line 505) regardless of the commission type. This inconsistency could cause workflows that evaluate lead-based conditions to not trigger correctly.
🔧 Proposed fix
metrics: {
current: {
- leads: commissionType === "lead" ? 1 : 0,
+ leads: 1,
saleAmount: saleAmount ?? totalSaleAmount,
conversions: firstConversionFlag ? 1 : 0,
},📝 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.
| leads: commissionType === "lead" ? 1 : 0, | |
| leads: 1, |
🤖 Prompt for AI Agents
In @apps/web/lib/actions/partners/create-manual-commission.ts at line 572,
metrics.current.leads is computed with commissionType === "lead" but elsewhere
the code always creates a lead event and increments link.leads, causing
inconsistency; update the leads metric calculation in
create-manual-commission.ts to reflect whether a lead event was actually created
or link.leads was incremented (use the same boolean/flag used when creating the
lead event or the result of that operation instead of commissionType), and
ensure the same logic is used when building the metrics payload (the expression
around leads: ...) so metrics.current.leads increments when a lead
event/link.leads is incremented.
Summary by CodeRabbit
New Features
Improvements
Plan Updates
✏️ Tip: You can customize this high-level summary in your review settings.