feat: Round Robin groups #22296
Merged
feat: Round Robin groups #22296
Conversation
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
added 20 commits
June 24, 2025 13:15
|
✅ No security or compliance issues detected. Reviewed everything up to 8363d04. Security Overview
Detected Code ChangesThe diff is too large to display a summary of code changes. Reply to this PR with |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
* check for attendee emails * fix RR with same host with fixed hosts and RR groups * code clean up * return hosts instead of empty array --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/lib/bookings/filterHostsBySameRoundRobinHost.test.ts (5)
69-69: Rename the describe to avoid implying fixed-hosts or RR-group supportPer the documented limitation (CAL-6134), this helper isn’t group-aware and shouldn’t be framed as such. These tests validate organizer/attendee-based filtering only. Rename to prevent confusion and avoid overclaiming coverage.
-describe("Fixed hosts and round robin groups support", () => { +describe("Organizer and attendee host filtering (rescheduleWithSameRoundRobinHost)", () => {
87-93: Assert prisma was called when the filter appliesMake the test intention explicit by asserting the call to prisma, including key parts of the query. Keeps the test robust if early-return conditions change.
const result = await filterHostsBySameRoundRobinHost({ hosts, rescheduleUid: "some-uid", rescheduleWithSameRoundRobinHost: true, routedTeamMemberIds: null, }); + + expect(prisma.booking.findFirst).toHaveBeenCalledTimes(1); + expect(prisma.booking.findFirst).toHaveBeenCalledWith({ + where: expect.objectContaining({ uid: "some-uid" }), + select: expect.objectContaining({ + userId: true, + attendees: expect.any(Object), + }), + });
95-101: Make assertion order-agnostic to reduce brittlenessfilter preserves the original order today, but asserting order here is unnecessary. Use arrayContaining + length to assert membership without coupling to order.
-// Should return organizer host (id: 1) and attendee hosts (ids: 2, 3) -expect(result).toHaveLength(3); -expect(result).toEqual([ - expect.objectContaining({ user: { id: 1, email: "host1@acme.com" } }), // organizer - expect.objectContaining({ user: { id: 2, email: "host2@acme.com" } }), // attendee - expect.objectContaining({ user: { id: 3, email: "host3@acme.com" } }), // attendee -]); +// Should return organizer host (id: 1) and attendee hosts (ids: 2, 3) +expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ user: { id: 1, email: "host1@acme.com" } }), // organizer + expect.objectContaining({ user: { id: 2, email: "host2@acme.com" } }), // attendee + expect.objectContaining({ user: { id: 3, email: "host3@acme.com" } }), // attendee + ]) +); +expect(result).toHaveLength(3);
69-69: Clarify scope in test naming and add a TODO referenceGiven CAL-6134, consider adding a TODO linking that issue to avoid confusion for future readers who might expect actual group-aware behavior to be validated here. Example:
-describe("Organizer and attendee host filtering (rescheduleWithSameRoundRobinHost)", () => { +// TODO(CAL-6134): This helper is not group-aware; tests validate organizer/attendee filtering only. +describe("Organizer and attendee host filtering (rescheduleWithSameRoundRobinHost)", () => {
124-127: Optional: Strengthen the “only organizer” caseYou can assert the prisma call here too for symmetry, and optionally assert identity to ensure filtered hosts are the same instances, not clones.
// Should return only organizer host expect(result).toHaveLength(1); expect(result[0]).toEqual(expect.objectContaining({ user: { id: 1, email: "host1@acme.com" } })); +expect(result[0]).toBe(hosts[0]); // same reference +expect(prisma.booking.findFirst).toHaveBeenCalledTimes(1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/lib/bookings/filterHostsBySameRoundRobinHost.test.ts(1 hunks)packages/lib/bookings/filterHostsBySameRoundRobinHost.ts(2 hunks)packages/lib/bookings/findQualifiedHostsWithDelegationCredentials.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/lib/bookings/filterHostsBySameRoundRobinHost.ts
- packages/lib/bookings/findQualifiedHostsWithDelegationCredentials.ts
🧰 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/lib/bookings/filterHostsBySameRoundRobinHost.test.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/bookings/filterHostsBySameRoundRobinHost.test.ts
🧠 Learnings (2)
📓 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.
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx:514-523
Timestamp: 2025-07-22T10:45:26.651Z
Learning: In the Round Robin groups feature, group names are only used within the event type settings UI and are not rendered elsewhere in the application. The user CarinaWolli confirmed this limited scope when asked about potential XSS concerns with direct input handling in the group name field.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.
📚 Learning: 2025-07-22T11:42:47.623Z
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.
Applied to files:
packages/lib/bookings/filterHostsBySameRoundRobinHost.test.ts
🧬 Code Graph Analysis (1)
packages/lib/bookings/filterHostsBySameRoundRobinHost.test.ts (1)
packages/lib/bookings/filterHostsBySameRoundRobinHost.ts (1)
filterHostsBySameRoundRobinHost(6-57)
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
eunjae-lee
approved these changes
Aug 8, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/features/bookings/lib/handleNewBooking.ts (3)
868-872: Micro-optimisation: avoid repeated org-lookup inside loop
getOrgIdFromMemberOrTeamIdis called for every host group but its inputs never change inside this loop. Fetch it once before the loop and reuse the value to save an unnecessary DB round-trip.
936-947: Redundant second call togroupHostsByGroupId
luckyUserPoolsalready groups the non-fixed hosts. CallinggroupHostsByGroupIdagain on essentially the same data wastes cycles and risks diverging behaviour if the util changes. ReuseluckyUserPoolsor keep a single grouped structure for both selection and validation.
846-850: Minor readability: shadowingusersvariableInside
Object.entries(...).map(([groupId, users]) => …)the parameterusersshadows the outerusersarray defined earlier in the scope, which makes the logged object harder to read. Renaming the inner variable (e.g.groupHosts) avoids cognitive overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/public/static/locales/en/common.json(4 hunks)apps/web/test/lib/getSchedule.test.ts(2 hunks)packages/features/bookings/lib/handleNewBooking.ts(6 hunks)packages/features/eventtypes/lib/types.ts(2 hunks)packages/lib/server/repository/eventTypeRepository.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/features/eventtypes/lib/types.ts
- packages/lib/server/repository/eventTypeRepository.ts
- apps/web/public/static/locales/en/common.json
- apps/web/test/lib/getSchedule.test.ts
🧰 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/features/bookings/lib/handleNewBooking.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/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (5)
📓 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.
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx:514-523
Timestamp: 2025-07-22T10:45:26.651Z
Learning: In the Round Robin groups feature, group names are only used within the event type settings UI and are not rendered elsewhere in the application. The user CarinaWolli confirmed this limited scope when asked about potential XSS concerns with direct input handling in the group name field.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.
📚 Learning: 2025-07-22T11:42:47.623Z
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.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-05T07:42:06.335Z
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-07-16T11:46:28.759Z
Learnt from: hariombalhara
PR: calcom/cal.com#22547
File: packages/embeds/embed-core/src/lib/eventHandlers/scrollByDistanceEventHandler.ts:11-14
Timestamp: 2025-07-16T11:46:28.759Z
Learning: In Cal.com's embed system, internal events like "__scrollByDistance" are fired by Cal.com's own code, so runtime validation of event data structure is unnecessary since TypeScript type system guarantees type safety for internal events.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
Comment on lines
+871
to
+882
| const newLuckyUser = await getLuckyUser({ | ||
| // find a lucky user that is not already in the luckyUsers array | ||
| availableUsers: freeUsers, | ||
| // only hosts from the same group | ||
| allRRHosts: ( | ||
| await enrichHostsWithDelegationCredentials({ | ||
| orgId: firstUserOrgId ?? null, | ||
| hosts: eventTypeWithUsers.hosts, | ||
| }) | ||
| ).filter((host) => !host.isFixed && userIdsSet.has(host.user.id) && host.groupId === groupId), | ||
| eventType, | ||
| routingFormResponse, |
There was a problem hiding this comment.
Bug – strict comparison between string and number drops all hosts in a group
groupId originates from Object.entries(luckyUserPools) and is therefore a string.
host.groupId comes from Prisma and is an int (number).
host.groupId === groupId will always be false, so the Round-Robin filter can end up with an empty array and getLuckyUser returns undefined, triggering RoundRobinHostsUnavailableForBooking even when suitable hosts exist.
-).filter((host) => !host.isFixed && userIdsSet.has(host.user.id) && host.groupId === groupId),
+).filter(
+ (host) =>
+ !host.isFixed &&
+ userIdsSet.has(host.user.id) &&
+ host.groupId !== null &&
+ host.groupId === Number(groupId),
+),Consider typing groupId as number in groupHostsByGroupId and/or casting once at the top of the loop to avoid repeated Number() calls.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const newLuckyUser = await getLuckyUser({ | |
| // find a lucky user that is not already in the luckyUsers array | |
| availableUsers: freeUsers, | |
| // only hosts from the same group | |
| allRRHosts: ( | |
| await enrichHostsWithDelegationCredentials({ | |
| orgId: firstUserOrgId ?? null, | |
| hosts: eventTypeWithUsers.hosts, | |
| }) | |
| ).filter((host) => !host.isFixed && userIdsSet.has(host.user.id) && host.groupId === groupId), | |
| eventType, | |
| routingFormResponse, | |
| const newLuckyUser = await getLuckyUser({ | |
| // find a lucky user that is not already in the luckyUsers array | |
| availableUsers: freeUsers, | |
| // only hosts from the same group | |
| allRRHosts: ( | |
| await enrichHostsWithDelegationCredentials({ | |
| orgId: firstUserOrgId ?? null, | |
| hosts: eventTypeWithUsers.hosts, | |
| }) | |
| ).filter( | |
| (host) => | |
| !host.isFixed && | |
| userIdsSet.has(host.user.id) && | |
| host.groupId !== null && | |
| host.groupId === Number(groupId), | |
| ), | |
| eventType, | |
| routingFormResponse, | |
| }); |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleNewBooking.ts around lines 871 to 882,
the comparison between host.groupId (number) and groupId (string) uses strict
equality, causing the filter to always fail and drop all hosts. To fix this,
ensure groupId is typed as a number before the loop or cast groupId to a number
once at the top of the loop, so the comparison host.groupId === groupId
correctly matches numeric values without repeated Number() calls.
E2E results are ready! |
2 tasks
Pallava-Joshi
pushed a commit
to Pallava-Joshi/cal.com
that referenced
this pull request
Aug 8, 2025
* add Add Group button * add host groups to schema * UI for host groups * raname groups to hostGroups * schema update * show groups in assignment tab * add no group hosts to Group 1 * add dummy group for non group hosts * fix type errors * use two dimensional array for luckyUserPools * fix empty array * group RR hosts in handleNewBooking * improve logic for grouping lucky users * find all lucky users of all groups * allow several RR hosts on booking * clean up migrations * create helper function * group hosts for slots logic * add group logic to loading available slots * adding hosts to groups * add groupId to hostSchema * disable hosts from other groups * handle groups in checkedTeamSelect * fix adding hosts to groups * remove and add groups * show hosts if there are no groups * fixing adding first group with existing hosts * show groups empty groups correctly * UI upddate fixes * fix adding hosts to existing first host group * small fixes + code clean up * add availability fix with test * create new round-robin test file * disable reassignment * fix losing fixed hosts * fix updating weights and priorities * disable load balancing with Round Robin Groups * automatically disable load balancing in update handler * allRRHosts should only include hosts from same group * fix type errors * fix type error * fix tests * fix type error * remove undefined from groupId type * type changes * add tests for hostGroups * add tests for host groups * fixes * fix type errors with undefined groupId * remove seperate host groups prop * fix editing weights * remove console.log * code clean up * improve getAggregatedAvailability tests * throw error when no available hosts in a group * add fixme comment * create constant for DEFAULT_GROUP_ID * clean up code * mock default_group_id for unit tests * don't show fixed hosts in edit weights side bar * add DEFAULT_GROUP_ID to mock test-setup * remove unused index variable * code clean up * fix updating host groups * fix imports * add default_group_id to mocks * add uuid() to zod schema * remove unused code * fix singular translation key * remove unnessary !! * Revert formatting changes * add additional tests for bookingActions * use createMany * import DEFAULT_GROUP_ID for mocks * fix mocks * clean up EventTeamAssignmentTab * fix type errors in tests * fix mocks * remove constants.example.test.ts * fix type error * add missing groupId * fix margin * clean up empty host groups * fix constants mock * useCalback * use reduce * extract handlers into seperate functions * fix handler functions * fix border radius * fix type error in CheckForEmptyAssignment * fix type error --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
This was referenced Aug 11, 2025
This was referenced Aug 18, 2025
This was referenced Aug 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
Introduces Round Robin host groups. From each group one host needs to attend the event. To find the Round Robin host to attend for each group the existing Round Robin logic is used. Empty groups are ignored, so if a group has no host's we are still showing available slots.
No-name groups:
Named groups:
Summary by cubic
Added support for Round Robin host groups, allowing event organizers to assign hosts into groups so that one host from each group is selected per booking.
New Features
Fixes Round Robin Groups #22002
Fixes CAL-5975