fix(booking): Pre-MVP algorithm integrity — #1071 #1012 #1005 - #1091
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
|
Warning Review limit reached
Next review available in: 10 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe pull request adds contiguous slot-run persistence, stale-tab allocation protection, server-confirmed subscription counts, appointment-kind consultee actions, group-event self-leave, initiator-specific refunds, and notification inbox layout updates. ChangesBooking lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SlotPicker
participant AllocationRoute
participant SlotAllocationService
participant BookingDatabase
SlotPicker->>AllocationRoute: submit expectedTentativeSlotCount
AllocationRoute->>SlotAllocationService: allocate with precondition
SlotAllocationService->>BookingDatabase: count tentative slots
BookingDatabase-->>SlotAllocationService: return current count
SlotAllocationService-->>AllocationRoute: continue or return 409 conflict
sequenceDiagram
participant ConsulteeAppointmentsAdapter
participant ParticipantRoute
participant BookingDatabase
ConsulteeAppointmentsAdapter->>ParticipantRoute: submit authenticated self-leave
ParticipantRoute->>BookingDatabase: remove consultee from event roster
ParticipantRoute->>BookingDatabase: calculate attendee-initiated refund
ParticipantRoute-->>ConsulteeAppointmentsAdapter: return result and invalidate queries
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 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 |
Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/participants/class/[classId]/route.ts (1)
136-160: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winExtract the self-leave/organiser authorization pattern into a shared helper.
Both routes implement the same two-stage authorization: a coarse self-leave-or-organiser gate, then a Prisma lookup that applies an ownership filter only for non-self-leave, non-privileged callers. Because this logic is security-relevant, keeping it in two files risks divergence if one route is fixed or extended later without updating the other.
app/api/participants/class/[classId]/route.ts#L136-L160: replace the inlineisSelfLeave/isOrganisergate andclassEventownership lookup with a shared helper, for exampleauthorizeParticipantRemoval({ session, userId, planOwnerFilter: { consultantProfileId: ... } }), returning either a forbidden response or the resolved authorization mode.app/api/participants/webinar/[webinarId]/route.ts#L131-L155: call the same shared helper with the webinar plan's ownership filter instead of duplicating the gate and lookup logic.🤖 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 `@app/api/participants/class/`[classId]/route.ts around lines 136 - 160, The duplicated self-leave/organiser authorization should be centralized in a shared authorizeParticipantRemoval helper. In app/api/participants/class/[classId]/route.ts lines 136-160, replace the isSelfLeave/isOrganiser gate and classEvent ownership lookup with the helper using the class plan owner filter; in app/api/participants/webinar/[webinarId]/route.ts lines 131-155, make the equivalent helper call with the webinar plan owner filter. Preserve the helper’s forbidden response and resolved authorization mode for each route.
🤖 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 `@__tests__/booking-algorithm/contiguous-slot-run.test.ts`:
- Around line 78-155: Extend the contiguous-slot-run test suite with coverage
for replaceContiguousSlotRun using a stub transaction client. Assert that
CANCELLED and RESCHEDULED rows are preserved, live rows are deleted, and every
recreated atom retains the previously connected user IDs; include data that
exercises the notIn NULL case in replaceContiguousSlotRun.
- Around line 35-42: Remove the conditional adjacency assertion from the
existing atom timing loop. Add a separate loop beginning at index 1 to assert
each atom’s startsAt matches the previous atom’s endsAt, while preserving the
existing per-atom start and end assertions.
In `@app/api/bookings/webinars/crud-with-plan/route.ts`:
- Around line 276-298: Export a shared buildSlotCreatePayloads helper from
contiguous-slot-run.ts that converts buildContiguousSlotAtoms results into
Prisma slot-create payloads, stripping user and assigning the owner
consultantProfileId. Replace the duplicated inline mappings at
app/api/bookings/webinars/crud-with-plan/route.ts lines 276-298 and 819-836, and
app/api/bookings/classes/crud-with-plan/route.ts lines 274-295, with this helper
while preserving each site’s existing inputs.
- Around line 568-573: Exclude dead slots identified by isDeadSlot from all
slot-run reads in app/api/bookings/webinars/crud-with-plan/route.ts: filter
slotsOfAppointment before selecting the first slot for the duration-only update,
compute runStart and runEnd from live rows only, and add the equivalent live-row
completionStatus filter to both nested selections at lines 478-479 and 512-512.
Apply these changes to the listed sites while preserving ordering and existing
behavior for live slots.
- Around line 790-794: Update the duration validation near
effectiveDurationForSlots to reject non-number and non-positive values, using
TypeError for the type check and the existing invalid-duration error type for
invalid numeric values. In the route handler’s catch block, map
InvalidDurationError to a 400 response alongside CapacityBelowEnrollmentError so
invalid durations do not become 500 responses.
In `@components/appointments/consultee/CancelConfirmationDialog.tsx`:
- Around line 53-58: Extract the nested ternary used by
CancelConfirmationDialog’s AlertDialogTitle into a named value such as title
before the JSX. Keep the existing isLeave, isPendingPayment, and default title
outcomes unchanged, then render the named value in AlertDialogTitle.
In `@components/appointments/consultee/ConsulteeAppointmentsAdapter.tsx`:
- Around line 279-343: Reduce the cognitive complexity of confirmDestructive by
extracting the cancel-trial and leave-event branches into separate helpers, such
as cancelTrial and leaveEvent. Each helper should perform its own validation,
fetch, response handling, and success toast, while confirmDestructive only
dispatches by destructive action and manages the shared loading, error,
dialog-close, invalidation, and finally behavior.
In `@hooks/scheduling/useSlotAllocation.ts`:
- Around line 584-587: Update the later per-day consecutiveness and
progress-feedback block in toggleSlot, specifically the
existingWeeklyConfirmedCallCounts calculation, to seed its targetWeekKey total
from options.weeklyConfirmedCallCounts just as the earlier completeCallsThisWeek
block does. Preserve the currentSlots-derived count while incorporating the
server-confirmed count so that the block’s weekly-limit check and completedCalls
toast progress reflect the same total.
In `@lib/appointments/contiguous-slot-run.ts`:
- Around line 152-163: Update the slot recreation loop in the contiguous-slot
run flow to preserve each existing SlotOfAppointment’s lifecycle fields and
related Recordings, including its id, completionStatus, completedAt, updatedAt,
and other required data; alternatively, explicitly reset lifecycle state and
handle stream-room rekeying. Ensure recreated slots retain correct appointment
status and recording reporting.
In `@utils/slotAllocation/SlotAllocationService.ts`:
- Around line 413-432: Move the expected tentative-slot validation from the
pre-transaction reads in autoAllocate and manualAllocate into their Prisma
transaction callbacks. Inside each transaction, re-read the appointment’s
tentative slots with tx.appointment.findMany, then call
assertExpectedTentativeSlotCount before the delete/recreate allocation path,
including reschedules and partial reschedules rather than only isFreshAllocation
cases.
---
Outside diff comments:
In `@app/api/participants/class/`[classId]/route.ts:
- Around line 136-160: The duplicated self-leave/organiser authorization should
be centralized in a shared authorizeParticipantRemoval helper. In
app/api/participants/class/[classId]/route.ts lines 136-160, replace the
isSelfLeave/isOrganiser gate and classEvent ownership lookup with the helper
using the class plan owner filter; in
app/api/participants/webinar/[webinarId]/route.ts lines 131-155, make the
equivalent helper call with the webinar plan owner filter. Preserve the helper’s
forbidden response and resolved authorization mode for each route.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 02a3a99e-2a65-45b0-aae8-66d4bfe85871
📒 Files selected for processing (28)
__tests__/booking-algorithm/consultee-affordances.test.ts__tests__/booking-algorithm/contiguous-slot-run.test.ts__tests__/booking-algorithm/expected-tentative-count.test.tsapp/api/bookings/classes/[classId]/allocate/route.tsapp/api/bookings/classes/crud-with-plan/route.tsapp/api/bookings/consultations/[consultationId]/allocate/route.tsapp/api/bookings/subscriptions/[subscriptionId]/allocate/route.tsapp/api/bookings/webinars/[webinarId]/allocate/route.tsapp/api/bookings/webinars/crud-with-plan/route.tsapp/api/participants/class/[classId]/route.tsapp/api/participants/webinar/[webinarId]/route.tscomponents/appointments/consultee/CancelConfirmationDialog.tsxcomponents/appointments/consultee/ConsulteeAppointmentsAdapter.tsxcomponents/dashboard/shared/requests/RequestSlotAllocationTab.tsxcomponents/scheduling/SlotPicker.tsxcomponents/scheduling/UnifiedCalendar.tsxdocs/booking/03-slot-math-and-calculations.mddocs/booking/05-troubleshooting-and-changelog.mddocs/booking/07-rescheduling-flow.mddocs/booking/08-cancellation-flow.mdhooks/scheduling/useSlotAllocation.tslib/appointments/consultee-affordances.tslib/appointments/contiguous-slot-run.tslib/scheduling/allocationAlgorithms.tslib/scheduling/allocationService.tsschemas/slotAllocation/validationSchemas.tsutils/slotAllocation/SlotAllocationService.tsutils/slotAllocation/types.ts
Review — #1071 / #1012 / #1005Read the full diff and traced each change against the surrounding code. The three new suites pass locally (13/13). The direction is right — canonicalising planner CRUD onto the allocator's N×30min shape is the correct fix for #1071, and the 1.
|
Reconcile contiguous slot rewrites in place to avoid cascading MeetingSession/Recording deletes, filter dead slots on webinar PATCH, use attendee notice tiers for self-leave refunds, restore detail-page Leave/Cancel trial via sourceId fallback, and fix the notification popover scroll. Part of #1072 #1005 #1071. Co-authored-by: Cursor <cursoragent@cursor.com>
Review round 2 —
|
| step | statement | state |
|---|---|---|
| i=0 | UPDATE s0 SET startsAt=11:00, endsAt=11:30 |
s2 still occupies [11:00,11:30), same consultant, both non-tentative → 23P01 |
isExclusionViolation catches it and returns 409 "That time conflicts with another confirmed session on your calendar." The webinar collides with itself, and the message blames a booking that doesn't exist.
Reachable on any shift smaller than the run length when activePayments === 0 — a free webinar, or one not yet sold. "Push it an hour later" is an ordinary planner action. Shift-and-shrink is worse: the surplus rows are only retired after the update loop, so they're still live non-tentative while the loop runs.
Neither the pre-#1071 single-row [0] update nor the interim delete+recreate could hit this — it is specific to the new shape.
Cheapest robust fix is two-phase: one updateMany flipping every live row to isTentative: true (drops them out of the partial index), then the per-row writes restoring the real isTentative. Since the target atoms are contiguous [) ranges they can't conflict with each other, so the second pass is safe in any order. Ordering the writes by direction (descending when moving forward) fixes the pure shift but not shift-plus-shrink; making the constraint DEFERRABLE INITIALLY DEFERRED also works but is a wider change.
Worth noting the two new replaceContiguousSlotRun tests move to a different day and to the same start — neither exercises an overlapping shift, and the stub tx has no constraint to violate, so this class of bug can't surface there.
3. Class self-leave is permanently blocked after the first session — Blocking
Both DELETE gates use the earliest live slot of the whole event:
const earliestLive = await prisma.slotOfAppointment.findFirst({
where: { appointment: { classId }, deletedAt: null, … },
orderBy: { startsAt: "asc" },
});
if (earliestLive && earliestLive.startsAt.getTime() <= Date.now()) return 400;Correct for a single-session webinar. For a class it's the wrong row: past sessions are stamped COMPLETED / UNVERIFIED, and DEAD_COMPLETION_STATUSES is only {CANCELLED, RESCHEDULED} — so session 1 stays "live" forever. From the moment week 1 starts, every self-leave on a months-long class returns 400 Cannot leave an event that has already started, while the UI keeps offering Leave event the whole time. That's precisely the offer-what-the-server-rejects problem #1005 set out to remove, reintroduced on the other side.
The same orderBy: asc query drives hoursUntilStart in refundRemovedAttendeeSeat, so even with the gate lifted a mid-program leave computes a negative notice and lands at 0 %. The local is already named nextLive — make it that: startsAt: { gte: now } for the notice window, and test "has the program ended" against the last live slot rather than the first.
Non-blocking
-
isDeadSlotgained adeletedAtcheck — right call, wide blast radius. Every consumer changes behaviour:groupSlotsIntoRuns,getSlotJoinState,slotsAllowReschedule, and the session/run math on both dashboards. It degrades safely (callers that don't selectdeletedAtgetundefined), but a tombstoned slot that used to render as a joinable session now vanishes. That's outside Rescheduling a webinar longer than 30 minutes moves only its first slot, splitting one appointment across two days #1071's stated scope — worth a changelog line so it isn't discovered as a regression. -
Retired surplus rows keep their attendee connections.
countWebinarParticipants/countUniqueParticipantsdon't filter dead slots, so capacity is now partly counted off rows deliberately excluded from the run. Harmless today (same people are on the live rows), but it stops being harmless the first time a shrink and a roster change interleave. -
ClassPlanSchemarefine also blocks edits to existing plans. Any class already authored at 0.75 h / 1.25 h now fails validation on its next PATCH, not just on create. Fine given the pre-MVP reset, but it's a silent edit-blocker rather than a create-time one. -
Scope creep.
components/notifications/NotificationInbox.tsx(Novu popover scroll) has nothing to do with Rescheduling a webinar longer than 30 minutes moves only its first slot, splitting one appointment across two days #1071/Stale-tab reschedule can delete+recreate a completed allocation — needs allocation precondition (audit R2, design with #997) #1012/Consultee self-leave for group events + UI kind-gates for impossible actions (audit R4/R5/C4/C6) #1005 and appears in neither the PR description nor the changelog table. Easy to lose at merge. -
createdCountstill returnscreatedLive.length— the total live-row count, not how many were created. Only tests read it today; the name will mislead the first real caller.
Verification run: jest __tests__/booking-algorithm/ __tests__/payments/attendee-removal-refund.test.ts → 764 passed / 36 suites. eslint on the 10 changed files → clean. tsc --noEmit → 17 errors, all traced to #1 above.
Drop invalid null completionStatus filters (tsc), tentative-flip before reconcile to avoid exclusion self-collisions, gate class self-leave on the last session with next-slot notice refunds, and document the planner reconcile / leave / dead-slot behaviour. Part of #1071 #1005 #1072. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@__tests__/booking-algorithm/contiguous-slot-run.test.ts`:
- Line 271: Update the assertions for rows "s0" and the corresponding row at the
other marked test location to verify the expected written startsAt value, rather
than only checking that an update entry with the id exists. Reuse the entries in
updates and assert the final in-place update result so a tentative pre-pass
alone cannot satisfy the tests.
In `@app/api/bookings/webinars/crud-with-plan/route.ts`:
- Around line 582-591: Update the liveSlots lookup in the duration-only change
block to use Array.find with the existing !isDeadSlot predicate, assigning the
first matching slot directly to existingSlot; preserve the subsequent startTime
and endTime behavior.
- Around line 822-827: Update the slot-rewrite logic around
existingPlan.consultantProfileId to resolve the owner from the PATCH request’s
requested consultantProfile first, falling back to the existing plan owner when
no transfer is requested. Use this resolved owner ID for slot rewriting and
retain the missing-owner validation so transferred plans protect the new
consultant’s calendar.
In `@components/appointments/consultee/ConsulteeAppointmentsAdapter.tsx`:
- Around line 340-361: Update leaveEvent’s endpoint selection to use an explicit
switch over AppointmentVM["kind"], preserving the webinar and class endpoints
while throwing in the default branch for unsupported kinds instead of routing
them to the class endpoint.
In `@docs/booking/05-troubleshooting-and-changelog.md`:
- Line 116: Update the ClassPlan validation in schemas/plans.ts to migrate or
grandfather existing rows whose sessionDurationInHours is not a multiple of 0.5,
so unrelated PATCH requests continue to succeed. Add a regression test covering
an unrelated PATCH on a legacy non-aligned plan, and remove the documented
limitation from the changelog.
In `@lib/payments/operations/event-refunds.ts`:
- Around line 253-282: Extract the shared live-slot query into a helper such as
findLiveSlot, centralizing deletedAt and completionStatus filtering while
supporting an optional startsAt lower bound and ascending or descending
ordering. In lib/payments/operations/event-refunds.ts:253-282, replace the
nextLive query with the helper using order "asc" and the current time filter; in
app/api/participants/class/[classId]/route.ts:166-189, use it for lastLive with
order "desc"; and in app/api/participants/webinar/[webinarId]/route.ts:161-185,
use it for earliestLive with order "asc".
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c7cc6d2-2dd2-4aef-b36a-103fe7dbe5e7
📒 Files selected for processing (18)
__tests__/booking-algorithm/contiguous-slot-run.test.ts__tests__/payments/attendee-removal-refund.test.tsapp/api/bookings/classes/crud-with-plan/route.tsapp/api/bookings/webinars/crud-with-plan/route.tsapp/api/participants/class/[classId]/route.tsapp/api/participants/webinar/[webinarId]/route.tscomponents/appointments/consultee/CancelConfirmationDialog.tsxcomponents/appointments/consultee/ConsulteeAppointmentsAdapter.tsxcomponents/notifications/NotificationInbox.tsxdocs/booking/03-slot-math-and-calculations.mddocs/booking/05-troubleshooting-and-changelog.mddocs/booking/07-rescheduling-flow.mddocs/booking/08-cancellation-flow.mdhooks/scheduling/useSlotAllocation.tslib/appointments/contiguous-slot-run.tslib/appointments/slots.tslib/payments/operations/event-refunds.tsschemas/plans.ts
|
Review round 3 —
|
…08-01-2 release: dev → prod — 2026-08-01 (#1091 booking algorithm)
1 participant
Summary
expectedTentativeSlotCountstale-tab reschedule precondition (Stale-tab reschedule can delete+recreate a completed allocation — needs allocation precondition (audit R2, design with #997) #1012)weeklyConfirmedCallCounts(Phase 0–2 already ondev)Closes #1071
Closes #1012
Closes #1005
Closes #1003
Closes #1004
Closes #834
Part of #676
Part of #837
Part of #997
Part of #1072
Test plan
__tests__/booking-algorithm/contiguous-slot-run.test.ts__tests__/booking-algorithm/expected-tentative-count.test.ts__tests__/booking-algorithm/consultee-affordances.test.ts__tests__/booking-algorithm/server-status-grid.test.ts/api/trialsMade with Cursor
Summary by CodeRabbit