fix: booking limits with seats not working (CAL-4553) #23298
Conversation
|
@Vansh5632 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
Medium priority
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes thread an eventType parameter through availability and interval-limit functions, enabling seat-aware booking-limit checks. Logic now distinguishes seated events: when a booking limit period is reached, the code computes remaining seats per time slot and marks busy only if no seats remain. Non-seated behavior stays unchanged. Adjustments occur in getUserAvailability, getBusyTimesFromLimits (and related helpers), and viewer slots utilities. Internal type aliases were renamed to private variants. Function signatures for busy-time retrieval were updated to accept and propagate eventType. Assessment against linked issues
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
dosubot
bot
added
the
bookings
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/23/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/23/25)1 label was added to this PR based on Keith Williams's automation. |
![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 (3)
packages/trpc/server/routers/viewer/slots/util.ts (1)
1191-1313: ThreadeventTypeinto the team‐limits path to enable seat‐aware blockingThe team‐level booking‐limits handler (
_getBusyTimesFromTeamLimitsForUsersin packages/trpc/server/routers/viewer/slots/util.ts) currently ignoreseventType.seatsPerTimeSlot, causing over‐blocking when seats are limited. You need to:• Extend the function signature to accept
eventType: EventType.
• PreserveeventTypeIdon each busy‐time entry.
• In both the global and per‐user loops, wheneventType.seatsPerTimeSlotis defined, count only bookings matching thateventType.idbefore marking the slot busy.
• Wire the neweventTypeargument through the call site at line 1416 of the same file.Suggested diff (in packages/trpc/server/routers/viewer/slots/util.ts):
--- a/packages/trpc/server/routers/viewer/slots/util.ts +++ b/packages/trpc/server/routers/viewer/slots/util.ts @@ 1191,7c1191,8 -const _getBusyTimesFromTeamLimitsForUsers = async ( +const _getBusyTimesFromTeamLimitsForUsers = async ( users: { id: number; email: string }[], bookingLimits: IntervalLimit, + eventType: NonNullable<EventType>, dateFrom: Dayjs, dateTo: Dayjs, teamId: number, @@ 1202,7c1203,8 - rescheduleUid?: string + rescheduleUid?: string, + // New param for seat limits ) => { @@ 1210,7c1212,8 - const busyTimes = bookings.map(({ id, startTime, endTime, eventTypeId, title, userId }) => ({ + const busyTimes = bookings.map(({ id, startTime, endTime, eventTypeId, title, userId }) => ({ start: dayjs(startTime).toDate(), end: dayjs(endTime).toDate(), title, @@ 1217,7c1221,8 userId, - })); + // Preserve for seat filtering + eventTypeId, + })); @@ 1240,7c1242,21 - globalLimitManager.addBusyTime(periodStart, unit, timeZone); + if (eventType.seatsPerTimeSlot) { + const slotCount = busyTimes.filter( + (b) => + b.eventTypeId === eventType.id && + dayjs(b.start).isSame(dayjs(booking.start)) && + isBookingWithinPeriod(b, periodStart, periodEnd, timeZone) + ).length; + if (slotCount >= eventType.seatsPerTimeSlot) { + globalLimitManager.addBusyTime(periodStart, unit, timeZone); + } + } else { + globalLimitManager.addBusyTime(periodStart, unit, timeZone); + } break; @@ 1290,7c1304,21 - limitManager.addBusyTime(periodStart, unit, timeZone); + if (eventType.seatsPerTimeSlot) { + const slotCount = userBusyTimes.filter( + (b) => + b.eventTypeId === eventType.id && + dayjs(b.start).isSame(dayjs(booking.start)) && + isBookingWithinPeriod(b, periodStart, periodEnd, timeZone) + ).length; + if (slotCount >= eventType.seatsPerTimeSlot) { + limitManager.addBusyTime(periodStart, unit, timeZone); + } + } else { + limitManager.addBusyTime(periodStart, unit, timeZone); + } break;And update the call at ~line 1416:
--- a/packages/trpc/server/routers/viewer/slots/util.ts:1414 +++ b/packages/trpc/server/routers/viewer/slots/util.ts:1414 - teamBookingLimitsMap = await getBusyTimesFromTeamLimitsForUsers( + teamBookingLimitsMap = await getBusyTimesFromTeamLimitsForUsers( usersForTeamLimits, teamBookingLimits, startTime, endTime, teamForBookingLimits.id, teamForBookingLimits.includeManagedEventsInLimits, usersWithCredentials[0]?.timeZone || "UTC", - input.rescheduleUid || undefined + input.rescheduleUid || undefined, + eventType );These changes ensure that team‐level limits respect per‐event‐type seat counts.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
110-112: Timezone mismatch: period boundaries should use the provided timeZoneperiodStartDates are currently computed without passing timeZone, while isBookingWithinPeriod() uses timeZone. This can shift week/day boundaries and produce off-by-one-period bugs.
Apply timeZone consistently here and in duration limits:
-const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit); +const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit, timeZone || "UTC");And in _getBusyTimesFromDurationLimits:
-const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit); +const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit, timeZone);
274-281: Keep eventTypeId on mapped items so downstream seat checks can filter correctlyCurrently the mapper drops eventTypeId; retain it to enable per-eventType seat checks inside getBusyTimesFromBookingLimits.
-const busyTimes = bookings.map(({ id, startTime, endTime, eventTypeId, title, userId }) => ({ +const busyTimes = bookings.map(({ id, startTime, endTime, eventTypeId, title, userId }) => ({ start: dayjs(startTime).toDate(), end: dayjs(endTime).toDate(), title, source: `eventType-${eventTypeId}-booking-${id}`, userId, + eventTypeId, }));
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/slots/util.ts (2)
1022-1042: Seat-aware block: logic directionally correct; tighten slot filtering and avoid quadratic scans
- Correctly defers marking periods busy for seated events until seats are exhausted.
- Two improvements:
- Restrict comparisons to exact instants without constructing Dayjs repeatedly in the inner loop to avoid O(N^2) overhead from filtering the entire array per booking.
- Use a precomputed map keyed by slotStartMillis to count bookings per period for better perf.
Apply this refactor inside the period loop to pre-aggregate by slot start:
- for (const booking of busyTimesFromLimitsBookings) { + // Pre-aggregate by slot start (ms) for this period only + const periodBookings = busyTimesFromLimitsBookings.filter((b) => + isBookingWithinPeriod(b, periodStart, periodEnd, timeZone) + ); + const bySlot = new Map<number, number>(); + for (const b of periodBookings) { + const k = new Date(b.start).getTime(); + bySlot.set(k, (bySlot.get(k) ?? 0) + 1); + } + for (const booking of periodBookings) { ... - const slotBookings = busyTimesFromLimitsBookings.filter( - (b) => - dayjs(b.start).isSame(dayjs(booking.start)) && - isBookingWithinPeriod(b, periodStart, periodEnd, timeZone) - ); - const totalAttendees = slotBookings.length; + const totalAttendees = bySlot.get(new Date(booking.start).getTime()) ?? 0; ... }
1099-1119: Duplicate seat-aware logic: extract helper or share pre-aggregation to reduce complexityYou’ve replicated the same seat-aware pattern in the per-user pass. Consider factoring a small helper or reusing the pre-aggregated map suggested above to keep the two loops consistent and cheaper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/lib/getUserAvailability.ts(2 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(6 hunks)packages/trpc/server/routers/viewer/slots/util.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/trpc/server/routers/viewer/slots/util.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/trpc/server/routers/viewer/slots/util.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧬 Code graph analysis (2)
packages/trpc/server/routers/viewer/slots/util.ts (2)
packages/lib/getUserAvailability.ts (1)
GetAvailabilityUser(193-193)packages/lib/intervalLimits/utils.ts (1)
isBookingWithinPeriod(42-56)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
packages/lib/getUserAvailability.ts (1)
EventType(147-147)packages/lib/event-types/getEventTypeById.ts (1)
EventType(34-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
packages/trpc/server/routers/viewer/slots/util.ts (1)
63-65: Type alias rename is fineThe underscore prefix clarifies internal scope and avoids accidental exports. No issues.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
46-47: Good: propagate eventType into booking-limits evaluationThis enables seated-event awareness down the stack.
248-258: Good: team limits now accept eventType for seat awarenessSignature change is correct and non-breaking for callers you updated.
packages/lib/getUserAvailability.ts (3)
269-269: Internal result type rename looks goodUnderscoring avoids accidental exports and clarifies intent.
391-393: Great: pass eventType to team-limit checksThis makes yearly/period checks seat-aware in the per-user path.
Note: If initialData.teamBookingLimits is provided (from slots/util.ts), it takes precedence and can still be seat-unaware. After updating slots/util.ts as suggested, this precedence remains correct.
351-368: Timezone argument consistently propagatedI’ve verified that every call to
getPeriodStartDatesBetweeninpackages/trpc/server/routers/viewer/slots/util.tsincludes thetimeZoneparameter, ensuring parity with the fallback logic’s use ofdateFrom.tz(timeZone)/dateTo.tz(timeZone)and preventing any off-by-one period issues. No further changes needed here.
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
Could you please resolve merge conflicts and address coderabbit suggestions?
|
@kart1ka okay working on them |
|
@kart1ka i am not understanding why the type-check errors are coming can you give me little bit explanation about it |
|
This PR is being marked as stale due to inactivity. |
Devin AI is completing this stale PRThis PR by @Vansh5632 has been marked as stale. A Devin session has been created to complete the remaining work. Devin will review the PR, address any feedback, and push updates to complete this PR. |
|
|
Summary of ChangesI've completed the work on this PR to fix booking limits with seats not working (CAL-4553). The codebase had been significantly refactored since the original PR was created, with files moved from Changes Madepackages/features/busyTimes/lib/getBusyTimesFromLimits.ts:
packages/features/availability/lib/getUserAvailability.ts:
packages/types/Calendar.d.ts:
How It WorksWhen a booking limit is reached for a time slot:
This ensures that slots with remaining seats are not incorrectly blocked when booking limits are hit. Co-authored-by: @Vansh5632 (original author) |
|
Hey @Vansh5632 looks like DevinAI went a little crazy here. We'll get it fixed. Apologies... |
When booking limits are reached, check if the event type has seats configured. If seats are available, don't mark the slot as busy - only block when no seats remain. Changes: - Add eventType parameter to getBusyTimesFromBookingLimits and getBusyTimesFromTeamLimits - Add eventTypeId to EventBusyDetails type for filtering - Implement seat-aware logic in booking limits check - Pass eventType through getUserAvailability to team limits Co-authored-by: Vansh5632 <[email protected]> Co-Authored-By: unknown <>
calcom-bot
force-pushed
the
fix/booking-limits-with-seats-cal-4553
branch
from
060d737 to
152f660
Compare
Co-Authored-By: unknown <>
keithwillcode
added
run-ci
5 participants
What does this PR do?
_getBusyTimesFromBookingLimitsgetBusyTimesFromTeamLimitsto pass eventType parameter for seat checkingeventTypeIdtoEventBusyDetailstype for filtering by event typeResolves CAL-4553: Booking limits with seats not working
Updates since last revision
The codebase was significantly refactored since the original PR was created. Files moved from
packages/lib/topackages/features/locations. The branch has been reset to a clean state based on currentmainwith only the necessary changes applied to the new file locations:packages/features/busyTimes/lib/getBusyTimesFromLimits.ts(waspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts)packages/features/availability/lib/getUserAvailability.ts(waspackages/lib/getUserAvailability.ts)packages/types/Calendar.d.tsLatest update: Added unit tests for seat-aware booking limits in
packages/features/busyTimes/lib/getBusyTimesFromLimits.test.tscovering:Video Demo (if applicable):
Screencast.from.2025-08-23.13-11-44.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Run unit tests:
TZ=UTC yarn test packages/features/busyTimes/lib/getBusyTimesFromLimits.test.tsHuman Review Checklist
slotBookings.lengthcounts bookings, not attendees - confirm this is the intended behavior for seated eventseventTypeparameter is properly passed through thegetBusyTimesFromTeamLimits→getBusyTimesFromBookingLimitscall chaineventTypeIdfiltering in the seat-aware check correctly filters bookings for the same event typeLink to Devin run: https://app.devin.ai/sessions/1f701af3d3174cc399f6d08a6883cf6c
Co-authored-by: Vansh5632 [email protected]