Customer subscription duration reward modifier#3461
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a customer attribute subscriptionDurationMonths to reward schemas and tests, threads subscriptionDurationMonths into partner-commission evaluation (including a re-evaluation using the first commission date), updates UI formatting for subscription duration, and adjusts a couple of commission-context payload shapes/formatting sites. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/API
participant TrackSale as TrackSale (apps/web/lib/api/conversions/track-sale.ts)
participant CommAction as ManualCommission (apps/web/lib/actions/partners/create-manual-commission.ts)
participant CommEval as CommEval (apps/web/lib/partners/create-partner-commission.ts)
participant RewardEngine as RewardEngine (determinePartnerReward)
participant DB as Database
Client->>TrackSale: send sale event (metadata, source?)
TrackSale->>CommEval: createPartnerCommission(context with productId?, source?)
CommEval->>RewardEngine: determinePartnerReward(initial context)
RewardEngine-->>CommEval: reward (or null)
CommEval->>DB: fetch first commission (if customerId & lead/sale)
DB-->>CommEval: firstCommission.createdAt
CommEval->>CommEval: compute subscriptionDurationMonths
CommEval->>RewardEngine: determinePartnerReward(augmented context + subscriptionDurationMonths)
RewardEngine-->>CommEval: updated reward (or null)
alt reward valid
CommEval->>DB: persist commission
else reward null
CommEval-->>CommEval: return commission: null
end
CommAction->>CommEval: manual create uses same context shape
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/actions/partners/create-manual-commission.ts (1)
447-454:⚠️ Potential issue | 🟡 MinorMissing
firstSaleAtin the new-events sale commission context.The
useExistingEventspath (Line 295) includescustomer.firstSaleAtin the context, but this path does not. Without it,subscriptionDurationwill always evaluate to0for manually created commissions via new events, even if the customer has been subscribed for months.Proposed fix
context: { - customer: { country: customer.country }, + customer: { + country: customer.country, + firstSaleAt: customer.firstSaleAt, + }, sale: { productId }, },
🤖 Fix all issues with AI agents
In `@apps/web/ui/partners/program-reward-modifiers-tooltip.tsx`:
- Around line 231-235: formatSubscriptionDuration can return an empty string
when v is 0 or NaN because formatDuration returns "" for all-zero parts; to fix,
coerce and guard v at the top (e.g., v = Number(v); if (!Number.isFinite(v) || v
<= 0) return "0 months") or otherwise compute parts and call formatDuration,
then if the result is falsy return a defensive fallback like "0 months". Update
the function formatSubscriptionDuration to normalize v and return the fallback
when formatDuration(...) yields an empty string.
🧹 Nitpick comments (3)
apps/web/tests/rewards/reward-conditions.test.ts (1)
1663-1728: Consider adding a test for nullsubscriptionDuration.The new tests cover the happy path for
subscriptionDurationwithgreater_than_or_equal, but there's no test for whensubscriptionDurationisnull(which would occur whenfirstSaleAtis null — a new customer's first sale). Indetermine-partner-reward.ts, the fallback produces0months, but testing this edge case in the condition evaluator would increase confidence.apps/web/lib/partners/determine-partner-reward.ts (1)
55-61:subscriptionDurationis computed for all event types, not just sales.This is harmless since
subscriptionDurationis only available as a reward condition attribute for thesaleevent type (per the schema inrewards.ts). However, forclickandleadevents,firstSaleAtwill always be absent, causing an unnecessarydifferenceInMonths(new Date(), new Date())call.Not a blocker — just a minor observation if you want to guard it:
context = { ...context, partner: { ... }, - customer: { + customer: event === EventType.sale ? { ...context?.customer, subscriptionDuration: differenceInMonths( new Date(), context?.customer?.firstSaleAt ?? new Date(), ), - }, + } : context?.customer, };apps/web/ui/partners/program-reward-modifiers-tooltip.tsx (1)
175-220: Deeply nested ternary — acknowledged by TODO, but consider extracting early.The condition/value rendering logic is now 4+ levels of nested ternaries, which is quite difficult to follow and modify. Since you've already noted the TODO on lines 122-123, consider extracting the value-formatting logic into a small helper (e.g.,
formatConditionValue(condition, attribute)) sooner rather than later — it would make the next change to this block much less error-prone.
|
@coderabbitai full review, any breaking changes? |
|
Breaking Changes Analysis✅ Non-Breaking:
Key Additions
The changes are purely additive and maintain full backward compatibility with existing partner programs and reward configurations. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/lib/actions/partners/create-manual-commission.ts`:
- Around line 295-302: The "new events" branch that builds the context for
partner reward evaluation is missing customer.firstSaleAt, causing
determine-partner-reward.ts to compute 0 months; in the else branch where
context is built (currently: context: { customer: { country: customer.country },
sale: { productId } }), add firstSaleAt: customer.firstSaleAt to the customer
object so the modifier logic receives the actual firstSaleAt value; update the
context construction in create-manual-commission.ts (the new events path /
useExistingEvents false branch) to include this field.
🧹 Nitpick comments (1)
apps/web/ui/partners/program-reward-modifiers-tooltip.tsx (1)
175-220: Deeply nested ternary rendering logic — aligns with the TODO on line 122.The attribute-value rendering at lines 182–220 now has 5+ levels of nested ternaries after adding the
subscriptionDurationbranch. This is hard to follow and maintain.Not blocking, but the TODO at line 122–123 should be addressed soon — consider extracting a
formatConditionValue(condition, attribute)helper.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/ui/partners/program-reward-modifiers-tooltip.tsx`:
- Around line 191-196: The array path for subscriptionDurationMonths doesn't
coerce items to numbers before formatting; update the branch that handles
Array.isArray(condition.value) so it first converts each element to a Number
(e.g., map(v => Number(v))) and then calls formatSubscriptionDuration on each
numeric value before joining, keeping the scalar path using
Number(condition.value) unchanged; this ensures formatSubscriptionDuration
always receives numeric months and avoids lexicographic comparisons.
- Around line 175-180: The displayed attribute label uses a hardcoded ternary
fallback for "subscriptionDurationMonths" which is unnecessary because the
attribute lookup populates attribute.label; update the JSX expression to use
only attribute?.label ?? condition.attribute (then call .toLowerCase()) instead
of the current conditional, i.e., replace the entire (attribute?.label ??
(condition.attribute === "subscriptionDurationMonths" ? "Subscription duration"
: condition.attribute))?.toLowerCase() with (attribute?.label ??
condition.attribute)?.toLowerCase() so the registered REWARD_CONDITIONS labels
are used.
|
@coderabbitai full review, any breaking changes? |
|
However, I'll conduct a fresh full review as requested to ensure nothing was missed. 🧠 Learnings used✅ Actions performedFull review triggered. |
2 participants
Summary by CodeRabbit
New Features
Tests