Feat/multi host round robin #15
Conversation
Add maxRoundRobinHosts config to assign up to N hosts per RR booking. Includes schema, API v2, booking logic, UI, and tRPC integration. Default (null/1) maintains existing single-host behavior feat: add multi-host assignment for round robin bookings Add maxRoundRobinHosts config to assign up to N hosts per RR booking. Includes schema, API v2, booking logic, UI, and tRPC integration. Default (null/1) maintains existing single-host behavior
- Add 4 unit tests covering multi-host assignment scenarios - Update event-types-hooks.mdx with maxRoundRobinHosts documentation - Improve API field description in DTOs for better OpenAPI docs
- Test create round robin event type with maxRoundRobinHosts - Test default value (null) when not provided - Test update maxRoundRobinHosts value
- Use Set for faster user lookups - Move userIdsSet outside the loop - Move async calls (firstUserOrgId, enrichedHosts) outside loops - Filter group hosts once per group instead of every iteration
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Rate limit exceeded@visz11 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (20)
✨ 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 |
Summary of ChangesHello @visz11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the flexibility of Round Robin event types by introducing a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
codeant-ai
bot
added
the
size:XL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the maxRoundRobinHosts feature, allowing multiple hosts to be assigned to a round-robin event. The changes span across the backend logic, database schema, API, and UI, and include comprehensive tests. The implementation looks solid. I've only found a minor issue in the documentation that could be misleading.
| schedulingType: "COLLECTIVE", | ||
| hosts: [{"userId": 1456}, {"userId": 2357}], | ||
| teamId: 1234, | ||
| maxRoundRobinHosts: 2, |
There was a problem hiding this comment.
The example for useCreateTeamEventType is demonstrating the use of maxRoundRobinHosts, which is a feature specific to Round Robin scheduling. However, the schedulingType is set to "COLLECTIVE". This is misleading. To make the example clearer and more accurate, please change the schedulingType to "ROUND_ROBIN".
schedulingType: "ROUND_ROBIN",
hosts: [{"userId": 1456}, {"userId": 2357}],
teamId: 1234,
maxRoundRobinHosts: 2,
Refacto PR SummaryImplemented multi-host assignment capability for Round Robin event types, allowing multiple hosts to be automatically assigned to a single booking instead of just one. The feature adds a configurable Key Changes:
Change HighlightsClick to expand
Sequence DiagramsequenceDiagram
participant B as Booker
participant API as Booking API
participant RR as Round Robin Service
participant DB as Database
participant H1 as Host 1
participant H2 as Host 2
participant H3 as Host 3
B->>API: Create booking request
API->>RR: Get available hosts for time slot
RR->>DB: Query event type maxRoundRobinHosts
DB-->>RR: maxRoundRobinHosts = 2
RR->>RR: Filter available hosts by group
RR->>RR: Select up to 2 hosts from pool
RR-->>API: Return [Host1, Host2] as luckyUsers
API->>DB: Create booking with multiple hosts
API->>H1: Send calendar invite
API->>H2: Send calendar invite
API-->>B: Booking confirmation with both hosts
Testing GuideClick to expand
|
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Multi-Host Round Robin ImplementationPR Confidence Score: 🟥 3 / 5👍 Well Done
📁 Selected files for review (20)
🎯 Custom Instructions
📝 Additional Comments
🧰 Additional context used
|
![refacto-visz[bot]](https://avatars.githubusercontent.com/in/1889637?s=60&v=4){kind=small}
| if ( | ||
| [...qualifiedRRUsers, ...additionalFallbackRRUsers].length > 0 && | ||
| luckyUsers.length !== (Object.keys(nonEmptyHostGroups).length || 1) | ||
| luckyUsers.length < (Object.keys(nonEmptyHostGroups).length || 1) |
There was a problem hiding this comment.
Round Robin Failure Logic
Modified condition allows booking creation with insufficient hosts when maxRoundRobinHosts > available hosts. System accepts bookings that cannot be properly assigned causing service degradation and booking failures from host assignment logic inconsistency.
luckyUsers.length < Math.min(maxHostsPerGroup * Object.keys(nonEmptyHostGroups).length, Object.keys(nonEmptyHostGroups).length || 1)
Commitable Suggestion
| luckyUsers.length < (Object.keys(nonEmptyHostGroups).length || 1) | |
| luckyUsers.length < Math.min(maxHostsPerGroup * Object.keys(nonEmptyHostGroups).length, Object.keys(nonEmptyHostGroups).length || 1) |
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
- SRE-Error-Handling
Context References
packages/features/bookings/lib/service/RegularBookingService.ts-ErrorCode.RoundRobinHostsUnavailableForBooking - validates proper error handling exists
| while (luckyUserPool.length > 0 && hostsFoundInGroup < maxHostsPerGroup) { | ||
| const freeUsers = luckyUserPool.filter((user) => !excludedUserIds.has(user.id)); |
There was a problem hiding this comment.
Nested Loop Complexity
Nested while loop with async getLuckyUser call creates O(n*m) complexity where n is host groups and m is hosts per group. Each iteration performs expensive user filtering and async service calls causing performance degradation multiplicatively with increased host count.
const batchSize = Math.min(maxHostsPerGroup, luckyUserPool.length);
const candidateUsers = luckyUserPool.filter((user) => !excludedUserIds.has(user.id)).slice(0, batchSize * 2);
for (let i = 0; i < candidateUsers.length && hostsFoundInGroup < maxHostsPerGroup; i++) {
const newLuckyUser = await deps.luckyUserService.getLuckyUser({
availableUsers: [candidateUsers[i]],
allRRHosts: groupRRHosts,
eventType,
routingFormResponse,
meetingStartTime: new Date(reqBody.start),
});
Commitable Suggestion
| while (luckyUserPool.length > 0 && hostsFoundInGroup < maxHostsPerGroup) { | |
| const freeUsers = luckyUserPool.filter((user) => !excludedUserIds.has(user.id)); | |
| const batchSize = Math.min(maxHostsPerGroup, luckyUserPool.length); | |
| const candidateUsers = luckyUserPool.filter((user) => !excludedUserIds.has(user.id)).slice(0, batchSize * 2); | |
| for (let i = 0; i < candidateUsers.length && hostsFoundInGroup < maxHostsPerGroup; i++) { | |
| const newLuckyUser = await deps.luckyUserService.getLuckyUser({ | |
| availableUsers: [candidateUsers[i]], | |
| allRRHosts: groupRRHosts, | |
| eventType, | |
| routingFormResponse, | |
| meetingStartTime: new Date(reqBody.start), | |
| }); |
Standards
- ISO-IEC-25010-Performance-Efficiency-Time-Behavior
- Algorithmic-Complexity-Nested-Loop-Optimization
- Optimization-Pattern-Async-Batching
Context References
packages/features/bookings/lib/getLuckyUser.ts-LuckyUserService.getLuckyUser - async database/computation operation
| @IsInt() | ||
| @Min(1) | ||
| @IsOptional() | ||
| @DocsPropertyOptional({ | ||
| description: "Only relevant for round robin event types. Specifies the maximum number of hosts to automatically assign per booking. When a booking is created, the system assigns up to this number of available hosts. If fewer hosts are available than the configured maximum, all available hosts are assigned. Minimum value is 1, defaults to 1.", | ||
| example: 1, | ||
| }) | ||
| maxRoundRobinHosts?: number; |
There was a problem hiding this comment.
Missing Validation Logic
The validation decorators allow maxRoundRobinHosts to be set for any event type, but business rule states it's only relevant for round robin scheduling. Missing conditional validation causes configuration inconsistencies when field is set for non-round-robin event types.
Standards
- Business-Rule-Conditional-Validation
- Logic-Verification-Input-Constraints
Context References
packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts-Same validation pattern repeated without scheduling type constraint
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| ### 5. `useCreateTeamEventType` | ||
| The useCreateTeamEventType hook allows you to create a new team event type. This hook returns a mutation function that handles the event type creation process. The mutation function accepts an object with the following properties: ***lengthInMinutes*** which is the length of the event in minutes, ***title*** which is the title of the event, ***slug*** which is the slug of the event, ***description*** which is the description of the event, schedulingType which can be either ***COLLECTIVE***, ***ROUND_ROBIN*** or ***MANAGED***, ***hosts*** which is an array of hosts for the event and the ***teamId*** which is the id of the team. | ||
| The useCreateTeamEventType hook allows you to create a new team event type. This hook returns a mutation function that handles the event type creation process. The mutation function accepts an object with the following properties: ***lengthInMinutes*** which is the length of the event in minutes, ***title*** which is the title of the event, ***slug*** which is the slug of the event, ***description*** which is the description of the event, schedulingType which can be either ***COLLECTIVE***, ***ROUND_ROBIN*** or ***MANAGED***, ***hosts*** which is an array of hosts for the event and the ***teamId*** which is the id of the team, and optionally ***maxRoundRobinHosts*** which specifies the maximum number of hosts to assign per booking for Round Robin events (defaults to 1). |
There was a problem hiding this comment.
Suggestion: The description of schedulingType lists the allowed values as COLLECTIVE, ROUND_ROBIN, and MANAGED, but the underlying API input class documents the string values as collective, roundRobin, and managed, so keeping the docs aligned with the API avoids confusing consumers about the expected literals. [logic error]
Severity Level: Minor
| The useCreateTeamEventType hook allows you to create a new team event type. This hook returns a mutation function that handles the event type creation process. The mutation function accepts an object with the following properties: ***lengthInMinutes*** which is the length of the event in minutes, ***title*** which is the title of the event, ***slug*** which is the slug of the event, ***description*** which is the description of the event, schedulingType which can be either ***COLLECTIVE***, ***ROUND_ROBIN*** or ***MANAGED***, ***hosts*** which is an array of hosts for the event and the ***teamId*** which is the id of the team, and optionally ***maxRoundRobinHosts*** which specifies the maximum number of hosts to assign per booking for Round Robin events (defaults to 1). | |
| The useCreateTeamEventType hook allows you to create a new team event type. This hook returns a mutation function that handles the event type creation process. The mutation function accepts an object with the following properties: ***lengthInMinutes*** which is the length of the event in minutes, ***title*** which is the title of the event, ***slug*** which is the slug of the event, ***description*** which is the description of the event, ***schedulingType*** which can be either ***collective***, ***roundRobin*** or ***managed***, ***hosts*** which is an array of hosts for the event and the ***teamId*** which is the id of the team, and optionally ***maxRoundRobinHosts*** which specifies the maximum number of hosts to assign per booking for Round Robin events (defaults to 1). |
Why it matters? ⭐
Verified in the codebase: the CreateTeamEventTypeInput transforms incoming string literals "collective", "roundRobin", and "managed" into the platform SchedulingType enum and its DocsProperty enum lists ["collective","roundRobin","managed"]. The docs currently show uppercase enum values (COLLECTIVE/ROUND_ROBIN/MANAGED) which are the server-side enum constants but not the API input strings users should send. Aligning the docs to show the API literal strings prevents real confusion and is a bugfix for the docs rather than mere style.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/platform/event-types-hooks.mdx
**Line:** 133:133
**Comment:**
*Logic Error: The description of `schedulingType` lists the allowed values as `COLLECTIVE`, `ROUND_ROBIN`, and `MANAGED`, but the underlying API input class documents the string values as `collective`, `roundRobin`, and `managed`, so keeping the docs aligned with the API avoids confusing consumers about the expected literals.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| schedulingType: "COLLECTIVE", | ||
| hosts: [{"userId": 1456}, {"userId": 2357}], | ||
| teamId: 1234, | ||
| maxRoundRobinHosts: 2, |
There was a problem hiding this comment.
Suggestion: The example config sets maxRoundRobinHosts while using a non–Round Robin scheduling type, which can mislead users into thinking the option affects collective events, so the line should explicitly indicate that it is only effective when schedulingType is set to a round-robin mode. [logic error]
Severity Level: Minor
| maxRoundRobinHosts: 2, | |
| maxRoundRobinHosts: 2, // Only relevant when schedulingType is "ROUND_ROBIN" |
Why it matters? ⭐
The CreateTeamEventTypeInput docs and implementation explain maxRoundRobinHosts is "Only relevant for round robin event types." The example in the MDX sets schedulingType to COLLECTIVE while also providing maxRoundRobinHosts, which misleads readers. Adding an inline comment (or better: changing the example to use a round-robin schedulingType) clarifies intent and prevents users from thinking the field applies to other scheduling types.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/platform/event-types-hooks.mdx
**Line:** 163:163
**Comment:**
*Logic Error: The example config sets `maxRoundRobinHosts` while using a non–Round Robin scheduling type, which can mislead users into thinking the option affects collective events, so the line should explicitly indicate that it is only effective when `schedulingType` is set to a round-robin mode.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| durationLimits: null, | ||
| assignAllTeamMembers: false, | ||
| rescheduleWithSameRoundRobinHost: false, | ||
| maxRoundRobinHosts: 1, |
There was a problem hiding this comment.
Suggestion: The test builder sets maxRoundRobinHosts to 1, whereas the schema and default event configuration treat this field as nullable with a default of null; this discrepancy can cause tests to assume a non-null, explicit value where production data would have null, masking bugs that depend on the distinction between an unset value and 1. [logic error]
Severity Level: Minor
Why it matters? ⭐
The Prisma schema shows maxRoundRobinHosts is Int? (nullable) and the project's defaultEvents explicitly sets maxRoundRobinHosts: null. The test builder setting it to 1 injects a concrete value where production/seed/default configuration expects null, which can mask behavior that depends on "unset" vs "set to 1". Changing the builder to null preserves fidelity between test fixtures and production defaults and reduces the chance of false positives in tests that assume the field is optional.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/lib/test/builder.ts
**Line:** 143:143
**Comment:**
*Logic Error: The test builder sets `maxRoundRobinHosts` to `1`, whereas the schema and default event configuration treat this field as nullable with a default of `null`; this discrepancy can cause tests to assume a non-null, explicit value where production data would have `null`, masking bugs that depend on the distinction between an unset value and `1`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| @Min(1) | ||
| @IsOptional() | ||
| @DocsPropertyOptional({ | ||
| description: "Only relevant for round robin event types. Specifies the maximum number of hosts to automatically assign per booking. When a booking is created, the system assigns up to this number of available hosts. If fewer hosts are available than the configured maximum, all available hosts are assigned. Minimum value is 1, defaults to 1.", |
There was a problem hiding this comment.
Suggestion: The description for the new maxRoundRobinHosts field states it limits the maximum number of hosts per booking, but the existing assignment logic in RegularBookingService actually treats this as a per-host-group limit (maxHostsPerGroup), so the API contract is misleading and integrators may assume a global cap while the system assigns more hosts when multiple groups are configured. To avoid incorrect assumptions and surprising behavior, the description should be updated to reflect that this value is applied per host group (and defaults to a global cap only when there are no groups). [logic error]
Severity Level: Minor
| description: "Only relevant for round robin event types. Specifies the maximum number of hosts to automatically assign per booking. When a booking is created, the system assigns up to this number of available hosts. If fewer hosts are available than the configured maximum, all available hosts are assigned. Minimum value is 1, defaults to 1.", | |
| description: | |
| "Only relevant for round robin event types. Specifies the maximum number of non-fixed hosts to automatically assign per host group for each booking. When a booking is created, the system assigns up to this number of available hosts in each group. If fewer hosts are available in a group than the configured maximum, all available hosts in that group are assigned. Minimum value is 1; when not set, the system behaves as if it were 1.", |
Why it matters? ⭐
The suggestion correctly identifies a mismatch between the API docs and the runtime behavior. In RegularBookingService the code treats eventType.maxRoundRobinHosts as a per-host-group cap (see maxHostsPerGroup and the loop that enforces hostsFoundInGroup < maxHostsPerGroup). The current docs say "per booking" which is misleading because multiple host groups can each yield hosts up to that cap, yielding more hosts overall. Updating the description to state "per host group" (and note default/semantic) fixes the API contract and prevents integrator confusion. This is a documentation fix that aligns the spec with verified implementation.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts
**Line:** 646:646
**Comment:**
*Logic Error: The description for the new `maxRoundRobinHosts` field states it limits the maximum number of hosts per booking, but the existing assignment logic in `RegularBookingService` actually treats this as a per-host-group limit (`maxHostsPerGroup`), so the API contract is misleading and integrators may assume a global cap while the system assigns more hosts when multiple groups are configured. To avoid incorrect assumptions and surprising behavior, the description should be updated to reflect that this value is applied per host group (and defaults to a global cap only when there are no groups).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| }) | ||
| maxRoundRobinHosts?: number; |
There was a problem hiding this comment.
Suggestion: The output DTO defines maxRoundRobinHosts as a non-nullable number and does not mark it as nullable in the Swagger schema, but the Prisma model and E2E tests expect this field to be null when not set; this mismatch can cause generated API clients and platform type consumers to treat null responses as invalid and breaks the documented response contract. [type error]
Severity Level: Minor
| }) | |
| maxRoundRobinHosts?: number; | |
| nullable: true, | |
| }) | |
| maxRoundRobinHosts?: number | null; |
Why it matters? ⭐
The DTO currently exposes maxRoundRobinHosts as an optional number but not nullable. The codebase, Prisma schema (EventType.maxRoundRobinHosts Int?), related TS types (packages/features/eventtypes/lib/types.ts) and E2E tests expect this field to be nullable (number | null) and assert null when not provided. Leaving the output DTO non-nullable will generate Swagger schemas and client types that disallow null and contradict runtime behaviour (DB may return null). Making the property nullable in the API docs and type (number | null) aligns the contract with Prisma, tests, and existing runtime handling.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts
**Line:** 560:561
**Comment:**
*Type Error: The output DTO defines `maxRoundRobinHosts` as a non-nullable number and does not mark it as nullable in the Swagger schema, but the Prisma model and E2E tests expect this field to be `null` when not set; this mismatch can cause generated API clients and platform type consumers to treat `null` responses as invalid and breaks the documented response contract.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Nitpicks 🔍
|
CodeAnt-AI Description
Allow assigning multiple hosts per Round Robin booking (maxRoundRobinHosts)
What Changed
Impact
✅ Multiple hosts assigned per Round Robin booking✅ Fewer booking failures due to an unavailable single host✅ API and UI support to configure hosts-per-booking💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.