Move reward enrollment sync to cron#3923
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds queued, batched reward-enrollment synchronization via a Qstash-verified cron POST endpoint, partner notification helpers and email template, makes Reward.program optional, integrates queueing into reward create/delete/update actions, and adjusts UI defaults for reward modifiers. ChangesReward Enrollment Async Sync & Partner Notification System
Sequence Diagram(s)sequenceDiagram
participant Qstash
participant SyncEndpoint as POST /api/cron/rewards/sync-enrollments
participant DB as Prisma_programEnrollment
participant RewardDB as Prisma_reward
participant Queue as queueRewardEnrollmentSync
participant Notify as notifyPartnersRewardChanged
Qstash->>SyncEndpoint: Publish with signature & payload
SyncEndpoint->>SyncEndpoint: Verify Qstash signature & parse JSON
SyncEndpoint->>DB: Fetch ordered batch by programEnrollment.id
SyncEndpoint->>DB: updateMany to set/clear reward column
SyncEndpoint->>DB: Count remaining matches
alt Remaining enrollments exist
SyncEndpoint->>Queue: Publish next batch with startAfterProgramEnrollmentId
else All processed
alt action == delete
SyncEndpoint->>RewardDB: Hard delete reward (P2025 treated as success)
SyncEndpoint->>Notify: Notify partners (removed)
else action == create
SyncEndpoint->>RewardDB: Fetch reward
SyncEndpoint->>Notify: Notify partners (added)
end
Notify->>DB: Load program & partner enrollments
Notify->>DB: Fetch partner users
Notify->>Notify: Queue PartnerRewardsUpdated per user
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/`(ee)/api/cron/rewards/sync-enrollments/route.ts:
- Line 1: The route is using the generic handleAndReturnErrorResponse which
breaks cron/QStash retry semantics; replace that with the cron-specific handler
handleCronErrorResponse: update the import to bring in handleCronErrorResponse
instead of handleAndReturnErrorResponse and change any call sites in this file
(including the block around the current 186-193 region) to invoke
handleCronErrorResponse so errors are routed through the cron-standard path and
QStash retry behavior is preserved.
In `@apps/web/lib/actions/partners/create-reward.ts`:
- Around line 111-118: The post-commit call to queueRewardEnrollmentSync can
leave the system in a partially-applied state if enqueue fails; modify the flow
to use a durable enqueue pattern: either write an outbox record in the same DB
transaction that creates/updates the reward (e.g., add an Outbox/Job row as part
of the createReward transaction) or atomically set a persistent flag like
reward.enrollmentSyncPending and have a reliable background reconciler scan and
enqueue retries; replace the direct await queueRewardEnrollmentSync(...) call
with creation of an outbox entry (or set the pending flag) inside the same
transaction and implement a separate worker/process to read the outbox and call
queueRewardEnrollmentSync to ensure guaranteed delivery and recovery.
In `@apps/web/lib/actions/partners/delete-reward.ts`:
- Around line 65-72: The delete flow currently clears the partnerGroup link and
nulls reward.programId then calls queueRewardEnrollmentSync, which can fail and
leave the DB in an intermediate state; fix by making the enqueue operation
atomic with the DB change—either perform queueRewardEnrollmentSync before
mutating links and abort the delete on enqueue failure, or implement a
transactional outbox: insert a sync job row (e.g.,
createRewardEnrollmentSyncJob) inside the same transaction that updates
partnerGroup/reward, and have a post-commit worker pick up that outbox row to
call queueRewardEnrollmentSync; update the delete-reward logic (references:
queueRewardEnrollmentSync, partnerGroup, reward.programId) to use one of these
patterns so a publish failure cannot strand detached rewards.
In `@apps/web/lib/actions/partners/update-reward.ts`:
- Around line 160-168: The call to notifyPartnersRewardChanged inside
updateRewardAction should be removed so partner notifications are not fired
before enrollment sync finalization; instead, emit the notification only from
the cron sync finalization path where you detect "no remaining work" (the branch
in the cron job that finalizes a sync), using the same payload shape (programId,
groupId: partnerGroup.id, action: "updated", effectiveAt:
updatedReward.updatedAt, reward: serializeReward(rewardMetadata)) and keep an
idempotencyKey (e.g., `reward-sync-${rewardId}-updated`) to prevent duplicates;
update the code by deleting or commenting out the notifyPartnersRewardChanged
call in updateRewardAction and adding the notification invocation into the cron
finalization branch that confirms enrollment sync completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f87ce71-a309-44c9-8265-a90b8ddf0a8a
📒 Files selected for processing (9)
apps/web/app/(ee)/api/cron/rewards/sync-enrollments/route.tsapps/web/lib/actions/partners/create-reward.tsapps/web/lib/actions/partners/delete-reward.tsapps/web/lib/actions/partners/update-reward.tsapps/web/lib/api/partners/notify-partners-reward-changed.tsapps/web/lib/api/rewards/queue-reward-enrollment-sync.tsapps/web/lib/email/email-templates-map.tspackages/email/src/templates/partner-rewards-updated.tsxpackages/prisma/schema/reward.prisma
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/`(ee)/api/cron/rewards/sync-enrollments/route.ts:
- Around line 127-155: When deleting rewards in the delete branch, make the
delete-finalization retry-safe by treating "record not found" as a successful
delete instead of returning early: inside the prisma.reward.delete error handler
(the try/catch around prisma.reward.delete for rewardId) detect the not-found
error and continue to the notifyPartnersRewardChanged call; only call
logAndRespond/return for unexpected errors. Keep references to the existing
functions/variables: prisma.reward.delete, notifyPartnersRewardChanged, log, and
logAndRespond so the retry will still send the removal notification even if the
reward was already removed.
In `@apps/web/lib/actions/partners/update-reward.ts`:
- Around line 160-168: The idempotency key used in the
notifyPartnersRewardChanged call is static per reward edit and causes Resend
dedup conflicts; update the idempotencyKey generation inside the
notifyPartnersRewardChanged invocation (where notifyPartnersRewardChanged is
called with programId, groupId: partnerGroup.id, action, effectiveAt:
updatedReward.updatedAt, reward, ...) to include the update timestamp (use
updatedReward.updatedAt.getTime() or similar) so the key becomes unique per
update (e.g., `reward-sync-${rewardId}-${updatedReward.updatedAt.getTime()}`).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd8bd531-c099-438f-9dc2-54d55f384a5a
📒 Files selected for processing (10)
apps/web/app/(ee)/api/cron/rewards/sync-enrollments/route.tsapps/web/lib/actions/partners/create-reward.tsapps/web/lib/actions/partners/delete-reward.tsapps/web/lib/actions/partners/update-reward.tsapps/web/lib/api/partners/get-group-rewards-and-bounties.tsapps/web/lib/api/partners/notify-partners-reward-changed.tsapps/web/lib/api/rewards/queue-reward-enrollment-sync.tsapps/web/lib/email/email-templates-map.tspackages/email/src/templates/partner-rewards-updated.tsxpackages/prisma/schema/reward.prisma
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/lib/api/rewards/queue-reward-enrollment-sync.ts (1)
11-19: ⚡ Quick winTighten the queue contract so delete jobs always include
rewardSnapshot.Right now, Line 18 and Line 45 make
rewardSnapshotoptional, soaction: "delete"can compile with an incomplete payload. A discriminated union here prevents invalid jobs at compile time.Proposed diff
export interface RewardSnapshot { title: string; description: string; icon: string; } -export interface QueueRewardEnrollmentSyncProps { - action: "create" | "delete"; - rewardId: string; - groupId: string; - programId: string; - event: EventType; - startAfterProgramEnrollmentId?: string; - rewardSnapshot?: RewardSnapshot; -} +type QueueRewardEnrollmentSyncBase = { + rewardId: string; + groupId: string; + programId: string; + event: EventType; + startAfterProgramEnrollmentId?: string; +}; + +export type QueueRewardEnrollmentSyncProps = + | (QueueRewardEnrollmentSyncBase & { + action: "create"; + rewardSnapshot?: never; + }) + | (QueueRewardEnrollmentSyncBase & { + action: "delete"; + rewardSnapshot: RewardSnapshot; + });Also applies to: 38-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/api/rewards/queue-reward-enrollment-sync.ts` around lines 11 - 19, Replace the loose interface QueueRewardEnrollmentSyncProps with a discriminated union so TypeScript enforces that action === "delete" always includes rewardSnapshot while action === "create" may omit it; specifically declare two variants (e.g., { action: "create"; rewardSnapshot?: RewardSnapshot; ... } and { action: "delete"; rewardSnapshot: RewardSnapshot; ... }) using the same common fields (rewardId, groupId, programId, event, startAfterProgramEnrollmentId) and update any producers/consumers of QueueRewardEnrollmentSyncProps (enqueue calls and handlers) to satisfy the stricter shape for "delete" jobs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/lib/api/rewards/queue-reward-enrollment-sync.ts`:
- Around line 11-19: Replace the loose interface QueueRewardEnrollmentSyncProps
with a discriminated union so TypeScript enforces that action === "delete"
always includes rewardSnapshot while action === "create" may omit it;
specifically declare two variants (e.g., { action: "create"; rewardSnapshot?:
RewardSnapshot; ... } and { action: "delete"; rewardSnapshot: RewardSnapshot;
... }) using the same common fields (rewardId, groupId, programId, event,
startAfterProgramEnrollmentId) and update any producers/consumers of
QueueRewardEnrollmentSyncProps (enqueue calls and handlers) to satisfy the
stricter shape for "delete" jobs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d599d0f-8eaf-4f70-908b-f8bebbc59c75
📒 Files selected for processing (2)
apps/web/app/(ee)/api/cron/rewards/sync-enrollments/route.tsapps/web/lib/api/rewards/queue-reward-enrollment-sync.ts
💤 Files with no reviewable changes (1)
- apps/web/app/(ee)/api/cron/rewards/sync-enrollments/route.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/lib/api/partners/notify-partners-reward-changed.ts (1)
82-113: 💤 Low valueConsider early return when no partners to notify.
If
program.partnersis empty (no partners in the group with valid emails),queueBatchEmailwill be called with an empty array. Adding a guard improves clarity and avoids an unnecessary call.♻️ Suggested change
const partnersToNotify = program.partners.map(({ partner }) => partner); + + if (partnersToNotify.length === 0) { + console.log(`No partners to notify for program ${programId} and group ${groupId}.`); + return; + } await queueBatchEmail<typeof PartnerRewardsUpdated>(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/api/partners/notify-partners-reward-changed.ts` around lines 82 - 113, Check for an empty recipients list before calling queueBatchEmail: compute partnersToNotify (derived from program.partners) and if it's empty (or contains no partners with valid emails) return early instead of invoking queueBatchEmail with an empty array; update the logic around the partnersToNotify variable and the queueBatchEmail<typeof PartnerRewardsUpdated> call to short-circuit when there are no recipients.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/ui/partners/rewards/add-edit-reward-sheet.tsx`:
- Around line 238-262: The computed maxDuration defaults currently let a missing
m.maxDuration pick up a null from defaultValuesSource and later leave it as
null; update the calculation so when m.maxDuration === undefined you treat
defaultValuesSource?.maxDuration === null (or undefined) as Infinity: e.g.
compute const maxDuration = m.maxDuration === undefined ?
(defaultValuesSource?.maxDuration == null ? Infinity :
defaultValuesSource.maxDuration) : m.maxDuration and then keep the final
property normalization (or simply set maxDuration: maxDuration) so any inherited
null becomes Infinity; adjust references in the modifier mapping where
maxDuration is used.
---
Nitpick comments:
In `@apps/web/lib/api/partners/notify-partners-reward-changed.ts`:
- Around line 82-113: Check for an empty recipients list before calling
queueBatchEmail: compute partnersToNotify (derived from program.partners) and if
it's empty (or contains no partners with valid emails) return early instead of
invoking queueBatchEmail with an empty array; update the logic around the
partnersToNotify variable and the queueBatchEmail<typeof PartnerRewardsUpdated>
call to short-circuit when there are no recipients.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eea00e12-b6f5-4a81-a24f-2c275877656b
📒 Files selected for processing (3)
apps/web/app/(ee)/api/cron/rewards/sync-enrollments/route.tsapps/web/lib/api/partners/notify-partners-reward-changed.tsapps/web/ui/partners/rewards/add-edit-reward-sheet.tsx
Collaborator
|
@coderabbitai full review, any breaking changes? |
Contributor
🧩 Analysis chain🏁 Script executed: Length of output: 15427 🏁 Script executed: Length of output: 317 🏁 Script executed: Length of output: 1767
|
| Change | Type | Risk |
|---|---|---|
Reward.programId nullable, no migration |
Schema | 🔴 High — deploy blocker |
| Enrollment sync moved to cron | Behavioral | 🟠 Medium — eventual consistency |
| Reward "orphan" state during deletion | Data integrity | 🟠 Medium — limbo state on cron failure |
| QStash on critical path | Infrastructure | 🟡 Low-Medium — correct fail-fast, but adds external dependency |
✅ Actions performed
Full review triggered.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/lib/api/partners/notify-partners-reward-changed.ts (1)
82-113: 💤 Low valueConsider early-returning when no partners to notify.
If
program.partnersis empty (no partners matchgroupId+ non-null email),partnersToNotifyis an empty array. The code still callsqueueBatchEmailwith an empty array, which may be a silent no-op or unnecessary API call. Adding an early return with a log would improve debuggability.♻️ Suggested improvement
const partnersToNotify = program.partners.map(({ partner }) => partner); + if (partnersToNotify.length === 0) { + console.log( + `No partners to notify for program ${programId} and group ${groupId}.`, + ); + return; + } + await queueBatchEmail<typeof PartnerRewardsUpdated>(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/api/partners/notify-partners-reward-changed.ts` around lines 82 - 113, partnersToNotify is built from program.partners and may be empty; avoid calling queueBatchEmail with an empty array by adding an early return: after computing partnersToNotify (from program.partners.map(...)) check if partnersToNotify.length === 0, log a debug/info message indicating no partners to notify (include program.id or program.name for context) and return early, so queueBatchEmail is only invoked when there are recipients.packages/email/src/templates/partner-rewards-updated.tsx (1)
114-127: 💤 Low valuePotential React key collision when same title appears multiple times.
The key
${change.action}-${change.title}could collide if the same action group contains multiple changes with identical titles. Consider adding an index or unique identifier.♻️ Suggested fix
- {items.map((change) => ( + {items.map((change, idx) => ( <Row - key={`${change.action}-${change.title}`} + key={`${change.action}-${change.title}-${idx}`} className="mb-0 mt-2" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/email/src/templates/partner-rewards-updated.tsx` around lines 114 - 127, The mapped list in items.map uses a fragile key `${change.action}-${change.title}` which can collide when titles repeat; update the Row key to include a stable unique discriminator (e.g., the item's id or the map index) so keys are unique and stable—locate the items.map rendering in partner-rewards-updated.tsx (the Row element inside items.map) and change the key to something like `${change.action}-${change.title}-${change.id}` or `${change.action}-${change.title}-${index}` depending on whether a persistent id field (change.id) exists, ensuring no duplicate keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/`(ee)/api/cron/rewards/sync-enrollments/route.ts:
- Around line 139-154: Add a proper type guard before accessing error.code:
import Prisma and update the catch to first check whether the caught error is a
Prisma.PrismaClientKnownRequestError (or use a user-defined type guard that
asserts 'code' exists) and only then compare error.code to "P2025"; otherwise
treat it as a non-Prisma error, log it with log (including error.message or the
whole error) and call logAndRespond for rewardId as currently done. Ensure you
reference Prisma.PrismaClientKnownRequestError (or your guard) when narrowing
the error type so TypeScript stops complaining about error.code.
---
Nitpick comments:
In `@apps/web/lib/api/partners/notify-partners-reward-changed.ts`:
- Around line 82-113: partnersToNotify is built from program.partners and may be
empty; avoid calling queueBatchEmail with an empty array by adding an early
return: after computing partnersToNotify (from program.partners.map(...)) check
if partnersToNotify.length === 0, log a debug/info message indicating no
partners to notify (include program.id or program.name for context) and return
early, so queueBatchEmail is only invoked when there are recipients.
In `@packages/email/src/templates/partner-rewards-updated.tsx`:
- Around line 114-127: The mapped list in items.map uses a fragile key
`${change.action}-${change.title}` which can collide when titles repeat; update
the Row key to include a stable unique discriminator (e.g., the item's id or the
map index) so keys are unique and stable—locate the items.map rendering in
partner-rewards-updated.tsx (the Row element inside items.map) and change the
key to something like `${change.action}-${change.title}-${change.id}` or
`${change.action}-${change.title}-${index}` depending on whether a persistent id
field (change.id) exists, ensuring no duplicate keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9759412b-a10e-41ea-9dd4-7036ac5b47ea
📒 Files selected for processing (10)
apps/web/app/(ee)/api/cron/rewards/sync-enrollments/route.tsapps/web/lib/actions/partners/create-reward.tsapps/web/lib/actions/partners/delete-reward.tsapps/web/lib/actions/partners/update-reward.tsapps/web/lib/api/partners/notify-partners-reward-changed.tsapps/web/lib/api/rewards/queue-reward-enrollment-sync.tsapps/web/lib/email/email-templates-map.tsapps/web/ui/partners/rewards/add-edit-reward-sheet.tsxpackages/email/src/templates/partner-rewards-updated.tsxpackages/prisma/schema/reward.prisma
steven-tey
approved these changes
May 21, 2026
This was referenced Jun 11, 2026
Merged
This was referenced Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
UI