feat: implement event waitlist functionality (closes #168) - #170
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughAdds event waitlist enrollment, status APIs, FIFO notification with timed holds, BullMQ processing, email delivery, refund/cancellation integration, and comprehensive service tests. ChangesEvent waitlist functionality
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WaitlistController
participant WaitlistService
participant Waitlist
participant QueueService
participant EmailWorker
Client->>WaitlistController: Join sold-out event
WaitlistController->>WaitlistService: join(userId, eventId)
WaitlistService->>Waitlist: create waiting entry
WaitlistService->>Waitlist: select next waiting entry
WaitlistService->>QueueService: enqueue spot-available email
QueueService->>EmailWorker: process email job
EmailWorker-->>Client: send hold notification
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/ticket-order.service.ts (1)
164-193: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake terminal transitions and inventory release atomic.
At Line 164, concurrent requests can both read a non-terminal status before Line 170 updates it. Both then restore
order.quantityand advance the waitlist, over-releasing inventory and notifying too many users. Claim the transition with a conditional atomic update, then update inventory transactionally; publish waitlist work through a post-commit outbox.🤖 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 `@src/services/ticket-order.service.ts` around lines 164 - 193, Replace the separate status read and unconditional findByIdAndUpdate in the order transition flow with a conditional atomic update that claims only orders not already in terminal statuses, and run the inventory release with that status change in a transaction. Move WaitlistService.processNextForEvent out of the transaction and enqueue it in a post-commit outbox, ensuring only the request that successfully claims the transition releases inventory and publishes waitlist work.
🧹 Nitpick comments (4)
tests/event-cancellation.test.ts (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the waitlist cancellation hook.
This mock isolates the test, but the suite never verifies
WaitlistService.cancelForEvent(eventId). Import the mocked service and assert one call in the successful-cancellation test so removal of the integration cannot pass unnoticed.🤖 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 `@tests/event-cancellation.test.ts` at line 12, Update the successful-cancellation test in the event cancellation suite to import the mocked WaitlistService and assert that cancelForEvent is called exactly once with eventId. Keep the existing mock isolation and cancellation assertions unchanged.src/controllers/waitlist.controller.ts (1)
15-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared request/error handling to remove triplication.
joinWaitlist,leaveWaitlist, andgetWaitlistStatusrepeat identical userId/ObjectId validation and identicalAppError→ JSON response mapping, differing only in the fallback error code/message. Consider a small wrapper to centralize this.♻️ Suggested extraction
+function requireAuthenticatedUser(req: UserAuthenticatedReq): string { + const userId = getUserId(req); + if (!userId) throw new UnauthorizedError(); + return userId; +} + +function requireValidEventId(req: UserAuthenticatedReq): string { + const { eventId } = req.params as { eventId: string }; + if (!mongoose.Types.ObjectId.isValid(eventId)) { + throw new ValidationError('Invalid event ID'); + } + return eventId; +} + +function sendControllerError(res: any, error: unknown, fallbackCode: string, fallbackMessage: string) { + if (error instanceof AppError) { + return res.status(error.statusCode).json({ success: false, error: error.code, message: error.message }); + } + return res.status(400).json({ + success: false, + error: fallbackCode, + message: error instanceof Error ? error.message : fallbackMessage, + }); +}🤖 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 `@src/controllers/waitlist.controller.ts` around lines 15 - 112, Extract the repeated authentication, eventId validation, and AppError response mapping from joinWaitlist, leaveWaitlist, and getWaitlistStatus into a small shared wrapper or helper. Update each handler to use it while preserving its distinct service call, success status/payload, fallback error code, and fallback message.tests/waitlist.service.test.ts (1)
62-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
E11000duplicate-key fallback andmarkConverted.The
join()duplicate-key catch (service Lines 70-75) andmarkConverted(service Lines 225-241) are both untested here. The duplicate-key path in particular backs the race-safety ofjoin(), so it's worth locking in with a test.🤖 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 `@tests/waitlist.service.test.ts` around lines 62 - 93, Extend the waitlist service tests with coverage for the E11000 duplicate-key fallback in WaitlistService.join, asserting the race-safe existing-entry behavior, and add a markConverted test covering its successful update path and expected result. Reuse the existing mocks and test identifiers without changing the current join scenarios.src/services/waitlist.service.ts (1)
12-12: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
WAITLIST_HOLD_MINUTES=0silently falls back to 15.
Number(process.env.WAITLIST_HOLD_MINUTES) || 15treats an explicit0the same as unset/invalid, since0is falsy. If disabling the hold window is ever a valid config, this breaks it.Fix
-const HOLD_MINUTES = Number(process.env.WAITLIST_HOLD_MINUTES) || 15; +const parsedHoldMinutes = Number(process.env.WAITLIST_HOLD_MINUTES); +const HOLD_MINUTES = Number.isFinite(parsedHoldMinutes) && parsedHoldMinutes >= 0 ? parsedHoldMinutes : 15;🤖 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 `@src/services/waitlist.service.ts` at line 12, Update the HOLD_MINUTES configuration in waitlist.service.ts to preserve an explicit WAITLIST_HOLD_MINUTES value of 0 while still defaulting to 15 when the environment variable is unset or invalid.
🤖 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 `@src/services/queue.service.ts`:
- Around line 275-277: Update the queue logging statement in the waitlist email
enqueue flow to remove the raw payload.userEmail value from application logs.
Retain only job.id, or use an approved non-reversible correlation value if
additional context is needed.
In `@src/services/ticket-order.service.ts`:
- Around line 195-200: In src/services/ticket-order.service.ts:195-200, update
the waitlist failure handling around the order cancellation/refund lifecycle to
persist a retriable post-commit job instead of only logging the error. In
src/services/event-ticket.service.ts:647-654, update the corresponding waitlist
cancellation flow to cancel entries transactionally or persist a retriable
cancellation job, ensuring both paths retain failed lifecycle work for retry.
In `@src/services/waitlist.service.ts`:
- Around line 152-167: Update processNextForEvent’s candidate selection and
iteration so it continues pulling waiting entries until count users with usable
email addresses are actually notified or the pool is exhausted. Do not limit the
initial query to count; track notified entries, expire candidates without email,
and fetch additional waiting candidates as needed while preserving createdAt
ordering.
- Around line 141-188: Update WaitlistService.processNextForEvent to atomically
claim each candidate inside the iteration using a find-and-update operation that
matches the event and waiting status while setting notified, notifiedAt, and
holdExpiresAt. Only proceed with user lookup and notification jobs when the
atomic claim succeeds; repeat this claim per requested spot so concurrent calls
cannot notify the same entry or skip later waiting entries.
In `@src/workers/email.worker.ts`:
- Around line 54-59: Restore the corrupted UTF-8 literals throughout the email
worker handlers/templates and queue-job configuration comments. In
src/workers/email.worker.ts at lines 54-59, 160, 184, 221, 238, 271, 322, 382,
431, and 485, replace mojibake with the intended symbols and text, or plain
ASCII where appropriate; in src/config/queue-jobs.ts at lines 89-100, 143, 157,
and 171, restore valid UTF-8 separators and punctuation without changing
behavior.
- Around line 597-603: Update the error path in the waitlist notification
delivery method so the catch block logs the failure and re-throws the original
error instead of returning `{ success: false }`. Preserve the existing success
response and allow BullMQ to detect the thrown delivery failure and retry the
job.
- Around line 114-119: Remove the duplicated/unreachable break structure and
ensure the SEND_TICKET_UPDATE_NOTIFICATION case terminates with a break before
execution can reach SEND_WAITLIST_SPOT_AVAILABLE. Preserve the existing
sendWaitlistSpotAvailable behavior for waitlist jobs and prevent ticket updates
from invoking it with an undefined hold duration.
---
Outside diff comments:
In `@src/services/ticket-order.service.ts`:
- Around line 164-193: Replace the separate status read and unconditional
findByIdAndUpdate in the order transition flow with a conditional atomic update
that claims only orders not already in terminal statuses, and run the inventory
release with that status change in a transaction. Move
WaitlistService.processNextForEvent out of the transaction and enqueue it in a
post-commit outbox, ensuring only the request that successfully claims the
transition releases inventory and publishes waitlist work.
---
Nitpick comments:
In `@src/controllers/waitlist.controller.ts`:
- Around line 15-112: Extract the repeated authentication, eventId validation,
and AppError response mapping from joinWaitlist, leaveWaitlist, and
getWaitlistStatus into a small shared wrapper or helper. Update each handler to
use it while preserving its distinct service call, success status/payload,
fallback error code, and fallback message.
In `@src/services/waitlist.service.ts`:
- Line 12: Update the HOLD_MINUTES configuration in waitlist.service.ts to
preserve an explicit WAITLIST_HOLD_MINUTES value of 0 while still defaulting to
15 when the environment variable is unset or invalid.
In `@tests/event-cancellation.test.ts`:
- Line 12: Update the successful-cancellation test in the event cancellation
suite to import the mocked WaitlistService and assert that cancelForEvent is
called exactly once with eventId. Keep the existing mock isolation and
cancellation assertions unchanged.
In `@tests/waitlist.service.test.ts`:
- Around line 62-93: Extend the waitlist service tests with coverage for the
E11000 duplicate-key fallback in WaitlistService.join, asserting the race-safe
existing-entry behavior, and add a markConverted test covering its successful
update path and expected result. Reuse the existing mocks and test identifiers
without changing the current join scenarios.
🪄 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 Plus
Run ID: 0a8d8199-9e3f-4f55-86e6-9e7cc4107707
📒 Files selected for processing (13)
src/config/queue-jobs.tssrc/controllers/waitlist.controller.tssrc/models/waitlist.tssrc/routes/event-ticket.route.tssrc/server.tssrc/services/event-ticket.service.tssrc/services/queue.service.tssrc/services/ticket-order.service.tssrc/services/waitlist.service.tssrc/workers/email.worker.tssrc/workers/waitlist.worker.tstests/event-cancellation.test.tstests/waitlist.service.test.ts
…ail worker, and apply prettier formatting
…st, pre-existing slowness)
|
@Yerimahjr Ensure to resolve all conversations |
|
@DioChuks All conversations resolved. Here's a quick summary of what was fixed vs. deferred and why: Fixed:
Deferred (left as documented follow-ups, not fixed in this PR):
Happy to open follow-up issues for any of these if that's useful. Let me know if you'd like anything reconsidered. |
There was a problem hiding this comment.
Spacing; that's good for developer experience & readability
| } | ||
|
|
||
| // ─── #81 + #78: Payment & Reconciliation Jobs ──────────────────────────────── | ||
| // ─── #81 + #78: Payment & Reconciliation Jobs ──────────────────────────────── |
There was a problem hiding this comment.
hmm, some unknown character format
| * allowed per user per event - see the partial unique index below. | ||
| */ | ||
| export interface IWaitlist extends Document { | ||
| user: mongoose.Types.ObjectId; |
There was a problem hiding this comment.
hmm, would've preferred wallet, since there's no way to store user only thru event organizers.
2 participants
Summary
Closes #168. Adds a waitlist feature so users can express interest in sold-out events and get automatically notified (with a time-limited hold) when a spot frees up from a cancellation or refund.
What's included
src/models/waitlist.ts— links a User to an EventTicket with statuswaiting | notified | converted | expired | cancelled. A partial unique index prevents duplicate active entries per user/event.src/services/waitlist.service.tsjoin/leave/getStatus(with 1-based queue position)processNextForEvent— notifies the oldest waiting user(s), sets a 15-minute hold (WAITLIST_HOLD_MINUTESenv override), enqueues the notification email + a delayed hold-expiry jobexpireHold— if the hold lapses without a purchase, expires it and moves to the next personcancelForEvent— closes out all active entries when the whole event is cancelledAppErrorsubclasses (NotFoundError,ValidationError,ConflictError) for consistent HTTP status mappingsrc/controllers/waitlist.controller.ts+ routes onsrc/routes/event-ticket.route.ts:POST /api/event-tickets/:eventId/waitlist— joinDELETE /api/event-tickets/:eventId/waitlist— leave / give up a held spotGET /api/event-tickets/:eventId/waitlist/status— status + positionNotifications — new
EmailJobType.SEND_WAITLIST_SPOT_AVAILABLEjob type, handled inemail.worker.tswith a template matching the existing branded email style. A newwaitlist-queue(bullmq) +src/workers/waitlist.worker.tshandles the delayed hold-expiry job. Both registered inserver.ts.Refund / cancellation hooks (acceptance criteria)
TicketOrderService.updateOrderStatusWithNotification— on a first-time transition into cancelled (2) or refunded (4), restores the freed inventory on the event and callsprocessNextForEventwith the freed quantity. Guarded so it can't double-fire, and wrapped so a waitlist bookkeeping failure never blocks the order update itself.EventTicketService.cancelEvent— since a full event cancellation means no new tickets will ever go on sale, this closes out all active waitlist entries (cancelForEvent) rather than sending a "spot available" prompt, which would be misleading.Bug fix (unrelated, found along the way)
src/routes/rounds.ts... (not applicable here — see below)Testing
tests/waitlist.service.test.ts— 17 unit tests covering the full flow: join validation (not found / cancelled / tickets still available / duplicate), leave (with and without triggering the next notification), status + position, processNextForEvent (cancelled-event no-op, notify + enqueue, skip users with no email), expireHold, and cancelForEvent.Also updated the pre-existing
tests/event-cancellation.test.tsto mockWaitlistService, sincecancelEventnow calls it — without the mock, the test would try to hit a real (disconnected) MongoDB via the new call and time out.Full suite: 238 tests, 237 passing. The one remaining failure category (
media.controller.tstype errors, 3 suites) is pre-existing onmainand unrelated to this change — verified viagit stashagainst a clean checkout.Notes
jest.mockon models/services).Summary by CodeRabbit
New Features
Bug Fixes
Tests