fix: round-robin reserve per host capacity#24520
fix: round-robin reserve per host capacity#24520husniabad wants to merge 3 commits intocalcom:mainfrom
Conversation
|
@husniabad is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
teams
WalkthroughThis PR implements capacity-based slot reservation logic for Round-Robin scheduling types. Changes include modifying PrismaSelectedSlotRepository to include generic Round-Robin reservations (userId: -1) in slot queries, updating the reserveSlot handler to detect overbooking by counting total reservations against host capacity, enhancing getAvailableSlots utility with Redis caching and capacity-aware availability logic, adding an availableHosts metric to slot payloads, expanding test coverage with Round-Robin capacity-based scenarios, and updating the SelectedSlot DTO to include the userId field. Possibly related PRs
Pre-merge checks and finishing touches❌ 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 |
dosubot
bot
added
the
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/server/routers/viewer/slots/util.ts (1)
167-178: Filter Round-Robin reservations to the relevant host pool before counting.Because
findManyUnexpiredSlotsnow returns everyuserId: -1row,slotsSelectedByOtherUsersends up holding reservations from unrelated Round-Robin events. The later capacity checks (reservationsForSlot.length >= usersWithCredentials.lengthand theavailableHostsmath) then treat those foreign holds as ours, so a different team’s reservation at the same timestamp makes our slot look full.Once the repository query is fixed, ensure this filter only keeps reservations tied to this event type’s host pool (e.g. match
eventTypeIdor whatever discriminator you choose). Without that, availability for independent RR events collapses.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/test/lib/getSchedule/selectedSlots.test.ts(1 hunks)packages/lib/server/repository/PrismaSelectedSlotRepository.ts(1 hunks)packages/lib/server/repository/dto/SelectedSlot.ts(1 hunks)packages/trpc/server/routers/viewer/slots/reserveSlot.handler.ts(1 hunks)packages/trpc/server/routers/viewer/slots/util.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*Repository.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/PrismaSelectedSlotRepository.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/repository/PrismaSelectedSlotRepository.tspackages/trpc/server/routers/viewer/slots/reserveSlot.handler.tspackages/lib/server/repository/dto/SelectedSlot.tsapps/web/test/lib/getSchedule/selectedSlots.test.tspackages/trpc/server/routers/viewer/slots/util.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/lib/server/repository/PrismaSelectedSlotRepository.tspackages/trpc/server/routers/viewer/slots/reserveSlot.handler.tspackages/lib/server/repository/dto/SelectedSlot.tsapps/web/test/lib/getSchedule/selectedSlots.test.tspackages/trpc/server/routers/viewer/slots/util.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/server/repository/PrismaSelectedSlotRepository.tspackages/trpc/server/routers/viewer/slots/reserveSlot.handler.tspackages/lib/server/repository/dto/SelectedSlot.tsapps/web/test/lib/getSchedule/selectedSlots.test.tspackages/trpc/server/routers/viewer/slots/util.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
🧬 Code graph analysis (1)
apps/web/test/lib/getSchedule/selectedSlots.test.ts (1)
apps/web/test/utils/bookingScenario/bookingScenario.ts (4)
TestData(1276-1548)getGoogleCalendarCredential(1229-1237)createBookingScenario(1015-1046)mockCalendarToHaveNoBusySlots(2013-2026)
| OR: [ | ||
| { userId: { in: userIds } }, | ||
| { userId: -1 } // Include generic Round-Robin reservations | ||
| ], |
There was a problem hiding this comment.
Do not pull every Round-Robin hold into unrelated availability checks.
Adding { userId: -1 } without scoping by event type makes this query return all Round-Robin reservations across the entire table. Downstream consumers (e.g. _getReservedSlotsAndCleanupExpired) now assume every returned row belongs to the current event type, so any other Round-Robin event that reserves the same timestamp instantly reduces capacity (or blocks bookings) for events whose hosts are completely unrelated. This regresses isolated teams: a booking for Team A at 10:00 with hosts {1,2} will prevent Team B with hosts {10,11} from reserving that same 10:00 slot because the global count trips the capacity guard.
We need to keep the fix (one reservation per slot) while still isolating capacity per host group. At minimum the query must restrict the userId: -1 branch to the relevant eventTypeId(s) so unrelated pools stay independent; otherwise every Round-Robin event competes for the same shared counter. Please tighten this filter (or carry eventTypeId context into the repository) before shipping.
🤖 Prompt for AI Agents
In packages/lib/server/repository/PrismaSelectedSlotRepository.ts around lines
63-66 the OR branch currently includes a bare { userId: -1 } which pulls every
Round-Robin reservation globally; narrow this by adding the relevant eventTypeId
condition so the Round-Robin branch only matches rows for the current event
type(s) (e.g. { AND: [{ userId: -1 }, { eventTypeId: currentEventTypeId }] }).
If the repository method does not yet accept eventTypeId, add it to the method
signature and pass it from callers (or otherwise supply the eventTypeId context)
so the query can include the eventTypeId filter while preserving the "one
reservation per slot" behavior.
| const totalReservations = await prisma.selectedSlots.count({ | ||
| where: { | ||
| slotUtcStartDate, | ||
| slotUtcEndDate, | ||
| userId: -1, | ||
| releaseAt: { gt: new Date() }, | ||
| }, | ||
| }); | ||
|
|
||
| if (totalReservations > eventType.users.length) { | ||
| // Rollback: Delete the reservation we just created | ||
| await prisma.selectedSlots.delete({ | ||
| where: { selectedSlotUnique: { userId: -1, slotUtcStartDate, slotUtcEndDate, uid } }, | ||
| }); | ||
| throw new TRPCError({ | ||
| message: "Slot is fully booked", | ||
| code: "BAD_REQUEST", |
There was a problem hiding this comment.
Round-Robin overbooking check needs to isolate by event type.
selectedSlots.count currently sums every Round-Robin hold (userId: -1) at the same timestamp, regardless of which event type created it. That means once some other team has two holds at 10:00, your team’s first hold immediately pushes totalReservations above eventType.users.length and throws “Slot is fully booked”, even though none of your hosts are taken. The repository change mirrors this and both together make unrelated Round-Robin events block each other globally.
Please scope the count to the relevant event type (or another discriminator tied to the actual host pool) so we only compare against the capacity that belongs to this event type’s hosts. Right now this breaks RR booking for any org running multiple independent RR event types.
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/slots/reserveSlot.handler.ts around lines
107-123, the selectedSlots.count and subsequent rollback delete are currently
counting all Round-Robin holds (userId: -1) at the same timestamp across event
types; restrict the count to this event type’s host pool by adding the
event-type discriminator to the where clause (e.g., eventTypeId or
eventType.uid) so you only compare holds belonging to this event type, and
include the same discriminator in the delete where to ensure the rollback
targets the exact reservation row you created.
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
This PR has been closed due to inactivity. Please feel free to reopen it if you'd like to continue the work. |
dhairyashiil
left a comment
dhairyashiil
left a comment
There was a problem hiding this comment.
Please attach before and after loom video, also please update the issue with video as well 🙏🏼
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/trpc/server/routers/viewer/slots/reserveSlot.handler.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/slots/reserveSlot.handler.ts:107">
Round-robin capacity counting is not scoped by eventTypeId, so reservations from other event types with the same time range will prematurely exhaust capacity for this event type.</violation>
</file>
<file name="packages/trpc/server/routers/viewer/slots/util.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/slots/util.ts:129">
Rule violated: **Avoid Logging Sensitive Information**
The new cache miss/hit warnings log the cacheKey string, which embeds the bookerClientUid cookie value. Logging this unique identifier exposes sensitive user data and violates the Avoid Logging Sensitive Information rule.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| }); | ||
|
|
||
| // Check capacity after creation to handle race conditions | ||
| const totalReservations = await prisma.selectedSlots.count({ |
There was a problem hiding this comment.
Round-robin capacity counting is not scoped by eventTypeId, so reservations from other event types with the same time range will prematurely exhaust capacity for this event type.
Prompt for AI agents
Address the following comment on packages/trpc/server/routers/viewer/slots/reserveSlot.handler.ts at line 107:
<comment>Round-robin capacity counting is not scoped by eventTypeId, so reservations from other event types with the same time range will prematurely exhaust capacity for this event type.</comment>
<file context>
@@ -76,35 +76,85 @@ export const reserveSlotHandler = async ({ ctx, input }: ReserveSlotOptions) =>
+ });
+
+ // Check capacity after creation to handle race conditions
+ const totalReservations = await prisma.selectedSlots.count({
+ where: {
+ slotUtcStartDate,
</file context>
| return JSON.parse(cached); | ||
| } | ||
| } catch (error) { | ||
| log.warn('Failed to get cached slots result', { error, cacheKey }); |
There was a problem hiding this comment.
Rule violated: Avoid Logging Sensitive Information
The new cache miss/hit warnings log the cacheKey string, which embeds the bookerClientUid cookie value. Logging this unique identifier exposes sensitive user data and violates the Avoid Logging Sensitive Information rule.
Prompt for AI agents
Address the following comment on packages/trpc/server/routers/viewer/slots/util.ts at line 129:
<comment>The new cache miss/hit warnings log the cacheKey string, which embeds the bookerClientUid cookie value. Logging this unique identifier exposes sensitive user data and violates the Avoid Logging Sensitive Information rule.</comment>
<file context>
@@ -114,36 +115,28 @@ function withSlotsCache(
+ return JSON.parse(cached);
}
+ } catch (error) {
+ log.warn('Failed to get cached slots result', { error, cacheKey });
}
-
</file context>
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
3 participants
What does this PR do?
Fixes Round-Robin slot capacity calculation to allow multiple bookings per time slot until all hosts are booked. Changes reservation filtering logic to only block slots when
reservationsForSlot.length >= usersWithCredentials.lengthand addsavailableHostsfield to display remaining capacity.Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Expected: Round-Robin slots show available host count and remain bookable until capacity reached
- Are there environment variables that should be set? - What are the minimal test data to have? - What is expected (happy path) to have (input and output)? - Any other important info that could help to test that PR
Checklist