KAN-1 Feat/multi host round robin #21
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 · |
Summary of ChangesHello @ayushjrathod-dd, 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 implements a significant new feature that allows round-robin event types to support multiple hosts per booking. This change provides greater control over how team members are assigned to events, moving beyond the previous single-host-per-booking model. The update involves modifications to the API, database schema, core scheduling logic, user interface, and documentation to ensure a seamless and robust experience. 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
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Service
participant Svc as OutputService
participant DB as Database
participant Logic as RegularBookingService
Client->>API: POST /event-types (maxRoundRobinHosts: 2)
API->>Svc: Validate & Save
Svc->>DB: Create EventType with maxRoundRobinHosts
DB-->>Svc: EventType persisted
Svc-->>API: Response with maxRoundRobinHosts field
API-->>Client: 201 Created
Client->>API: POST /bookings (for Round Robin event)
API->>Logic: Process booking (fetch eventType with maxRoundRobinHosts)
Logic->>DB: Retrieve Round Robin event type
DB-->>Logic: EventType {<br/> maxRoundRobinHosts: 2,<br/> team members: [Alice, Bob, Charlie]<br/>}
rect rgb(100, 150, 200)
Note over Logic: Apply maxRoundRobinHosts filtering
Logic->>Logic: Select up to 2 hosts from team<br/>(excludedUserIds tracking)
Logic->>Logic: Assign Alice & Bob<br/>(cap at maxRoundRobinHosts)
end
Logic-->>API: Booking with assigned hosts
API-->>Client: 200 OK {attendees: [Alice, Bob]}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
codeant-ai
bot
added
the
size:XL
Refacto PR SummaryImplemented multi-host Round Robin booking capability by adding a configurable Key Changes:
Change HighlightsClick to expand
Sequence DiagramsequenceDiagram
participant U as User
participant API as Booking API
participant RR as Round Robin Service
participant DB as Database
participant Cal as Calendar Service
U->>API: Create Round Robin booking
API->>DB: Get event type with maxRoundRobinHosts
DB-->>API: EventType (maxRoundRobinHosts: 3)
API->>RR: Find available hosts
RR->>RR: Filter by availability & groups
RR->>RR: Select up to 3 hosts using lucky algorithm
RR-->>API: Selected hosts [Host1, Host2, Host3]
API->>Cal: Create calendar events for all hosts
Cal-->>API: Calendar events created
API->>DB: Save booking with multiple attendees
API-->>U: Booking confirmed with 3 hosts
Testing GuideClick to expand
|
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
![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 for round-robin event types. The changes are comprehensive, touching the database schema, API endpoints, UI components, and the core booking logic. E2E and unit tests have been added to cover the new functionality. My review identified a minor documentation issue and a potential bug in the booking creation logic that could prevent bookings from being made under certain conditions. Overall, the implementation is solid, but I recommend addressing the feedback to ensure correctness and clarity.
| 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.
The condition to throw RoundRobinHostsUnavailableForBooking seems incorrect with the introduction of maxRoundRobinHosts. The current logic luckyUsers.length < (Object.keys(nonEmptyHostGroups).length || 1) appears to require at least one host from each group if groups are present, which may not be the desired behavior. A booking should likely succeed even if hosts are found in only some groups, as long as at least one host is found overall.
Consider changing this condition to luckyUsers.length === 0 to throw an error only when no hosts are available at all. This would align with the feature's goal of assigning up to a certain number of hosts.
| luckyUsers.length < (Object.keys(nonEmptyHostGroups).length || 1) | |
| luckyUsers.length === 0 |
| title: "Daily standup", | ||
| slug: "daily-standup", | ||
| description: "Daily standup for the team", | ||
| schedulingType: "COLLECTIVE", |
There was a problem hiding this comment.
The example for useCreateTeamEventType uses schedulingType: "COLLECTIVE" but also includes maxRoundRobinHosts. The documentation correctly states that maxRoundRobinHosts is for Round Robin events. To avoid confusion, the example should be updated to use schedulingType: "ROUND_ROBIN".
schedulingType: "ROUND_ROBIN",
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| schedulingType: "COLLECTIVE", | ||
| hosts: [{"userId": 1456}, {"userId": 2357}], | ||
| teamId: 1234, | ||
| maxRoundRobinHosts: 2, |
There was a problem hiding this comment.
Suggestion: The example adds maxRoundRobinHosts to a collective event type even though this field only has an effect for round robin scheduling, which can mislead users into thinking it affects other scheduling types as well. [possible bug]
Severity Level: Critical 🚨
| maxRoundRobinHosts: 2, | |
| maxRoundRobinHosts: 2, // Only add this when schedulingType is "roundRobin" |
Why it matters? ⭐
This is a valid, actionable doc-improvement: the example sets schedulingType to "COLLECTIVE" but includes maxRoundRobinHosts which only matters for round-robin scheduling. That can mislead readers. Adding a short comment or moving the property behind a roundRobin example clarifies intent and avoids confusion.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/platform/event-types-hooks.mdx
**Line:** 163:163
**Comment:**
*Possible Bug: The example adds `maxRoundRobinHosts` to a collective event type even though this field only has an effect for round robin scheduling, which can mislead users into thinking it affects other scheduling types as well.
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.|
|
||
| const { setValue, getValues, control, formState } = useFormContext<FormValues>(); | ||
| const { setValue, getValues, control } = useFormContext<FormValues>(); | ||
| const assignRRMembersUsingSegment = getValues("assignRRMembersUsingSegment"); |
There was a problem hiding this comment.
Suggestion: The value of assignRRMembersUsingSegment is read once via getValues and passed into EditWeightsForAllTeamMembers, so toggling the "filter by attributes" state will not cause RoundRobinHosts to re-render with the updated value; using useWatch for this field keeps the prop in sync with the form state and prevents stale behavior. [possible bug]
Severity Level: Critical 🚨
| const assignRRMembersUsingSegment = getValues("assignRRMembersUsingSegment"); | |
| const assignRRMembersUsingSegment = useWatch({ | |
| control, | |
| name: "assignRRMembersUsingSegment", | |
| }); |
Why it matters? ⭐
The file currently reads assignRRMembersUsingSegment once via getValues and then passes it to EditWeightsForAllTeamMembers. That value will not update when the form field changes, causing stale prop issues and incorrect UI behavior. Switching to useWatch for that field makes RoundRobinHosts re-render with the current form state and fixes a real synchronization bug rather than being a stylistic change.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
**Line:** 305:305
**Comment:**
*Possible Bug: The value of `assignRRMembersUsingSegment` is read once via `getValues` and passed into `EditWeightsForAllTeamMembers`, so toggling the "filter by attributes" state will not cause `RoundRobinHosts` to re-render with the updated value; using `useWatch` for this field keeps the prop in sync with the form state and prevents stale behavior.
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.| )} | ||
| /> | ||
| </> | ||
| {!hostGroups.length ? ( |
There was a problem hiding this comment.
Suggestion: hostGroups is read with .length without a null/undefined guard, so if the form value for hostGroups is ever undefined (for instance when no groups are configured yet), this will throw a runtime error when rendering the Round Robin hosts section; guarding for a falsy hostGroups before accessing .length avoids this crash. [null pointer]
Severity Level: Minor
| {!hostGroups.length ? ( | |
| {!hostGroups || hostGroups.length === 0 ? ( | |
| <AddMembersWithSwitchComponent groupId={hostGroups?.[0]?.id ?? null} /> |
Why it matters? ⭐
In the final file hostGroups is populated with useWatch(...) and can be undefined. The current code dereferences .length without a guard which will throw if hostGroups is undefined. The suggested improved_code (guarding for falsy hostGroups) fixes a real runtime crash risk during render. This is a small, targeted safety fix and not merely cosmetic.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
**Line:** 543:543
**Comment:**
*Null Pointer: `hostGroups` is read with `.length` without a null/undefined guard, so if the form value for `hostGroups` is ever `undefined` (for instance when no groups are configured yet), this will throw a runtime error when rendering the Round Robin hosts section; guarding for a falsy `hostGroups` before accessing `.length` avoids this crash.
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 helper now sets maxRoundRobinHosts to 1 by default, but in the real system this field is nullable (Int? in Prisma) and other defaults (e.g. defaultEvents) use null, with the runtime logic interpreting a null/undefined value as 1 via eventType.maxRoundRobinHosts ?? 1; hardcoding 1 here makes tests build event types that no longer reflect the actual default (missing/nullable) value, which can cause tests to miss regressions around the "not provided" case and diverge from production behavior. [logic error]
Severity Level: Minor
| maxRoundRobinHosts: 1, | |
| maxRoundRobinHosts: null, |
Why it matters? ⭐
The Prisma schema (packages/prisma/schema.prisma) declares maxRoundRobinHosts as Int? (nullable). The default event fixtures in packages/features/eventtypes/lib/defaultEvents.ts also set maxRoundRobinHosts: null. The PR's test builder hardcodes 1 which diverges from production/default fixtures and can mask behavior when the value is omitted vs set to 1 (the application often uses eventType.maxRoundRobinHosts ?? 1). Changing the builder to use null keeps test objects faithful to production shape and avoids hiding regressions that depend on the "not provided" path.
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 helper now sets `maxRoundRobinHosts` to `1` by default, but in the real system this field is nullable (`Int?` in Prisma) and other defaults (e.g. `defaultEvents`) use `null`, with the runtime logic interpreting a `null`/undefined value as `1` via `eventType.maxRoundRobinHosts ?? 1`; hardcoding `1` here makes tests build event types that no longer reflect the actual default (missing/nullable) value, which can cause tests to miss regressions around the "not provided" case and diverge from production behavior.
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 declares maxRoundRobinHosts as an optional number without null, and the OpenAPI metadata does not mark it as nullable, but the service actually returns null when the value is not set (as verified by the e2e tests and Prisma schema). This creates an API contract mismatch: generated clients and internal consumers using @calcom/platform-types or OpenAPI will think the field is either omitted or a positive integer, while they can receive null at runtime, which can cause runtime errors or incorrect handling in strictly typed or generated clients. [logic error]
Severity Level: Minor
| }) | |
| maxRoundRobinHosts?: number; | |
| nullable: true, | |
| }) | |
| maxRoundRobinHosts?: number | null; |
Why it matters? ⭐
The Prisma schema defines EventType.maxRoundRobinHosts as Int? (nullable) and E2E tests explicitly assert the API returns null when the value is not set. The DTO currently declares maxRoundRobinHosts?: number (no null) and the ApiPropertyOptional metadata doesn't mark nullable: true — this creates a real API contract mismatch for generated clients and strict TypeScript consumers. Changing the property to number | null and setting nullable: true in ApiPropertyOptional fixes a runtime/typing mismatch rather than being merely cosmetic.
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:**
*Logic Error: The output DTO declares `maxRoundRobinHosts` as an optional `number` without `null`, and the OpenAPI metadata does not mark it as nullable, but the service actually returns `null` when the value is not set (as verified by the e2e tests and Prisma schema). This creates an API contract mismatch: generated clients and internal consumers using `@calcom/platform-types` or OpenAPI will think the field is either omitted or a positive integer, while they can receive `null` at runtime, which can cause runtime errors or incorrect handling in strictly typed or generated clients.
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.
Code Review: Multi-Host Round Robin EnhancementPR Confidence Score: 🟨 4 / 5👍 Well Done
📁 Selected files for review (20)
🎯 Custom Instructions
📝 Additional Comments
|
![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) | ||
| ) { | ||
| throw new Error(ErrorCode.RoundRobinHostsUnavailableForBooking); |
There was a problem hiding this comment.
Incomplete Group Coverage Validation
The validation luckyUsers.length < numGroups is insufficient when maxRoundRobinHosts > 1. If one group provides multiple hosts while another required group provides none, the total count might satisfy the length check despite a required group being unrepresented, violating collective scheduling requirements.
const requiredGroups = Object.keys(nonEmptyHostGroups);
const representedGroups = new Set();
// Track which groups have assigned hosts
for (const luckyUser of luckyUsers) {
const userGroupId = enrichedHosts.find(host => host.user.id === luckyUser.id)?.groupId || DEFAULT_GROUP_ID;
representedGroups.add(userGroupId);
}
if (
[...qualifiedRRUsers, ...additionalFallbackRRUsers].length > 0 &&
representedGroups.size < requiredGroups.length
) {
throw new Error(ErrorCode.RoundRobinHostsUnavailableForBooking);
}
Commitable Suggestion
| if ( | |
| [...qualifiedRRUsers, ...additionalFallbackRRUsers].length > 0 && | |
| luckyUsers.length !== (Object.keys(nonEmptyHostGroups).length || 1) | |
| luckyUsers.length < (Object.keys(nonEmptyHostGroups).length || 1) | |
| ) { | |
| throw new Error(ErrorCode.RoundRobinHostsUnavailableForBooking); | |
| const requiredGroups = Object.keys(nonEmptyHostGroups); | |
| const representedGroups = new Set(); | |
| // Track which groups have assigned hosts | |
| for (const luckyUser of luckyUsers) { | |
| const userGroupId = enrichedHosts.find(host => host.user.id === luckyUser.id)?.groupId || DEFAULT_GROUP_ID; | |
| representedGroups.add(userGroupId); | |
| } | |
| if ( | |
| [...qualifiedRRUsers, ...additionalFallbackRRUsers].length > 0 && | |
| representedGroups.size < requiredGroups.length | |
| ) { | |
| throw new Error(ErrorCode.RoundRobinHostsUnavailableForBooking); | |
| } |
Standards
- Business-Rule-Completeness
- Algorithm-Correctness-Validation
| @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 Maximum Limit Validation
Missing upper bound validation (@max) for maxRoundRobinHosts. A large value can trigger excessive synchronous external API calls (calendar syncs, emails) during booking creation, leading to request timeouts and resource exhaustion.
Standards
- ISO-IEC-25010-Performance-Efficiency-Capacity
- SRE-Resource-Limits
![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
🧹 Nitpick comments (9)
packages/features/bookings/lib/service/RegularBookingService.ts (2)
1047-1072: Clarify semantics ofmaxRoundRobinHosts(per group vs per booking)The new logic:
- Derives
maxHostsPerGroup = eventType.maxRoundRobinHosts ?? 1.- Loops per host group, with a per‑group
hostsFoundInGroupcounter.- Tracks
excludedUserIdsglobally so a user is not selected twice.- Passes a per‑group
groupRRHostsintoluckyUserService.getLuckyUser.This effectively enforces a per‑group cap while allowing up to
numberOfGroups * maxRoundRobinHostshosts per booking. However, the field name and docs describe “maximum number of hosts to assign per booking for Round Robin events”, which reads as a global cap.If the intended behavior is truly global, you’ll likely want a
totalHostsFoundcounter and to break outer iteration oncetotalHostsFound >= maxRoundRobinHosts, plus adjust the final validation that currently comparesluckyUsers.lengthonly to the number of non‑empty host groups.If the per‑group semantics are intentional, consider documenting that explicitly (and perhaps renaming the internal variable) to avoid confusion between implementation and the public description.
Also applies to: 1081-1082, 1087-1089, 1122-1124, 1126-1127, 1133-1135, 1156-1159
1051-1059: Host enrichment with delegation credentials is wired correctly but adds another org lookupUsing
getOrgIdFromMemberOrTeamIdwithmemberId: eventTypeWithUsers.users[0]?.idandteamId: eventType.teamIdto obtainorgIdforenrichHostsWithDelegationCredentials, then enrichingeventTypeWithUsers.hosts, is consistent with howgetOrgIdFromMemberOrTeamIdis used later in this file.The extra org lookup per booking is probably acceptable, but if you see this on a hot path under high load you might want to reuse
eventOrganizationId/orgIdrather than re‑querying.docs/platform/event-types-hooks.mdx (1)
133-135: AlignmaxRoundRobinHostsdocs and example with Round Robin behaviorThe text correctly describes
maxRoundRobinHostsas affecting Round Robin events, but the example usesschedulingType: "COLLECTIVE"while still passingmaxRoundRobinHosts: 2.To avoid confusion for integrators, consider either:
- Switching the example to
schedulingType: "ROUND_ROBIN", or- Explicitly stating that
maxRoundRobinHostsis only honored for Round Robin scheduling types and ignored otherwise.Also applies to: 155-164
apps/api/v2/src/modules/organizations/event-types/services/output.service.ts (1)
102-144: Properly wiredmaxRoundRobinHoststhrough the responseDestructuring
maxRoundRobinHostsfromdatabaseEventTypeand returning it alongsiderescheduleWithSameRoundRobinHostcleanly exposes the new field on the team event type response. Behavior remains backward compatible since the field is optional.If you ever revisit this function, consider grouping “RR-related” flags (
rescheduleWithSameRoundRobinHost,maxRoundRobinHosts, distribution settings) in a small nested object to reduce top-level clutter, but this is non-blocking.apps/api/v2/src/modules/organizations/event-types/max-round-robin-hosts.e2e-spec.ts (1)
83-88: Consider removing the@ts-ignorecomment by using a type assertion.The
@ts-ignoresuppresses type checking but masks potential issues. Consider using a proper type assertion or casting instead.🔎 Suggested fix
const body: CreateTeamEventTypeInput_2024_06_14 = { title: "Round Robin Event", slug: `max-rr-hosts-${randomString()}`, lengthInMinutes: 30, - // @ts-ignore - schedulingType accepts string - schedulingType: "roundRobin", + schedulingType: "roundRobin" as unknown as keyof typeof import("@calcom/prisma/enums").SchedulingType, assignAllTeamMembers: true, ...overrides, };Alternatively, verify if the input type definition can be updated to accept the string literal directly.
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
517-541: Consider adding a visual indicator for the default behavior when the field is empty.The input correctly handles null values and validates that the number is at least 1. However, the UX might be confusing since:
- An empty field means
null, which defaults to assigning a single host- The placeholder shows "1" but doesn't explicitly communicate this is the default behavior
The description at line 525 (
t("max_round_robin_hosts_description")) should clarify this, but consider adding helper text or adjusting the placeholder.🔎 Potential improvement
<NumberInput min={1} - placeholder="1" + placeholder={t("default_one")} className="w-24" value={value ?? ""} onChange={(e: React.ChangeEvent<HTMLInputElement>) => { const val = e.target.value === "" ? null : parseInt(e.target.value, 10); if (val === null || (!isNaN(val) && val >= 1)) { onChange(val); } }} /> + <p className="text-muted text-xs mt-1">{t("leave_empty_for_default")}</p>This would require adding appropriate localization strings.
docs/api-reference/v2/openapi.json (3)
19667-19670: Use"type": "integer"with aminimumconstraint.Since
maxRoundRobinHostsrepresents a discrete count of hosts, it should be typed as"integer"rather than"number". Additionally, the description states "Minimum value is 1" but the schema lacks the correspondingminimumconstraint for validation.🔎 Proposed fix
"maxRoundRobinHosts": { - "type": "number", + "type": "integer", + "minimum": 1, "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 }
21685-21688: Apply the same fix: use"type": "integer"withminimum: 1.Same issue as the previous occurrence.
🔎 Proposed fix
"maxRoundRobinHosts": { - "type": "number", + "type": "integer", + "minimum": 1, "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 }
22153-22156: Apply the same fix: use"type": "integer"withminimum: 1.Same issue as the previous occurrences.
🔎 Proposed fix
"maxRoundRobinHosts": { - "type": "number", + "type": "integer", + "minimum": 1, "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 }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.tsapps/api/v2/src/modules/organizations/event-types/max-round-robin-hosts.e2e-spec.tsapps/api/v2/src/modules/organizations/event-types/services/output.service.tsapps/web/public/static/locales/en/common.jsondocs/api-reference/v2/openapi.jsondocs/platform/event-types-hooks.mdxpackages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsxpackages/features/eventtypes/lib/defaultEvents.tspackages/features/eventtypes/lib/types.tspackages/features/eventtypes/repositories/eventTypeRepository.tspackages/lib/test/builder.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.tspackages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.tspackages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.tspackages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.tspackages/prisma/migrations/20251208104340_add_max_round_robin_hosts_optional/migration.sqlpackages/prisma/schema.prisma
🧰 Additional context used
🧬 Code graph analysis (3)
packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (6)
apps/web/test/utils/bookingScenario/bookingScenario.ts (10)
getBooker(2260-2274)getOrganizer(1543-1608)TestData(1262-1534)getGoogleCalendarCredential(1215-1223)Timezones(317-321)createBookingScenario(1002-1033)getScenarioData(1610-1698)mockSuccessfulVideoMeetingCreation(2115-2133)mockCalendarToHaveNoBusySlots(1999-2012)getDate(1116-1163)packages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.ts (1)
Timezones(33-36)packages/features/bookings/lib/handleNewBooking/test/date-overrides.test.ts (1)
Timezones(33-36)packages/platform/libraries/index.ts (1)
SchedulingType(29-29)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(15-17)apps/web/test/utils/bookingScenario/getMockRequestDataForBooking.ts (1)
getMockRequestDataForBooking(42-56)
apps/api/v2/src/modules/organizations/event-types/max-round-robin-hosts.e2e-spec.ts (3)
packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts (1)
CreateTeamEventTypeInput_2024_06_14(566-650)packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts (1)
TeamEventTypeOutput_2024_06_14(494-562)packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts (1)
UpdateTeamEventTypeInput_2024_06_14(481-564)
packages/features/bookings/lib/service/RegularBookingService.ts (5)
packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-62)packages/app-store/delegationCredential.ts (1)
enrichHostsWithDelegationCredentials(425-465)packages/lib/constants.ts (1)
DEFAULT_GROUP_ID(226-226)packages/lib/CalEventParser.ts (1)
getVideoCallUrlFromCalEvent(427-444)packages/platform/atoms/prisma-types/index.ts (1)
JsonValue(5-5)
🔇 Additional comments (35)
packages/lib/test/builder.ts (1)
85-170: DefaultmaxRoundRobinHostsin test builder is consistent with service logicSetting
maxRoundRobinHosts: 1inbuildEventTypematches the runtime fallback (eventType.maxRoundRobinHosts ?? 1) and gives deterministic test fixtures. Looks good.packages/features/bookings/lib/service/RegularBookingService.ts (7)
39-39: Async booking email/SMS tasker wiring looks soundInjecting
BookingEmailAndSmsTaskerviaIBookingServiceDependenciesand deferring to it underENABLE_ASYNC_TASKERwith abookingEmailsAndSmsTaskerActionderived from the actual flow (requested/confirmed/rescheduled) is consistent and side‑effect free for dry runs or when the feature flag/team feature is off. No issues spotted.Also applies to: 459-469, 531-531, 2671-2695
1315-1321: Static conferencing URLs: avoiding credentials looks correctBranching to:
const { bookingLocation, conferenceCredentialId } = organizerOrFirstDynamicGroupMemberDefaultLocationUrl ? { bookingLocation: organizerOrFirstDynamicGroupMemberDefaultLocationUrl, conferenceCredentialId: undefined } : getLocationValueForDB(...);ensures static URLs (pre‑resolved links) are treated as plain locations and don’t try to use conferencing credentials. The updated
destinationCalendarfallback to organizer’s destination calendar if the event type doesn’t specify one also preserves expected behavior.Also applies to: 1363-1367
1986-1995: NewEventManager.reschedulecall signatureThe extended
eventManager.reschedulecall now passeschangedOrganizer,previousHostDestinationCalendar,isBookingRequestedReschedule, andskipDeleteEventsAndMeetings, which matches the nearby logic that computes these flags and inputs.Assuming
EventManager.reschedulehas been updated to accept this expanded signature in its own module, this looks consistent.
2293-2297: Video call URL handling now consistently prefers the canonical event URLUsing:
const metadata = videoCallUrl ? { videoCallUrl: getVideoCallUrlFromCalEvent(evt) || videoCallUrl } : undefined;and then threading
metadata.videoCallUrlinto both the booking response and stored booking metadata unifies how the video URL is derived (videoCallData → integration hangoutLink → HTTP location → fallback). This should reduce drift between what’s actually on the calendar event and what’s exposed to consumers.Also applies to: 2465-2467, 2581-2583
2308-2316:oldBookingpayload addition for reschedulesIncluding an
oldBookingobject withstartTimeandendTimeinBookingRescheduledPayloadis a useful addition for downstream handlers/auditing.Just ensure the
BookingRescheduledPayloadtype inonBookingEventsis updated accordingly (witholdBookingoptional) so there’s no type drift.
2391-2393: CastingeventType.metadata.appstoPrisma.JsonValuebefore paymentThe change:
metadata: eventType.metadata ? { ...eventType.metadata, apps: eventType.metadata?.apps as Prisma.JsonValue } : {},is a reasonable way to satisfy Prisma’s
JsonValuetyping forappswithout mutating the original metadata object. This should help keep the payment flow type‑safe.
2727-2754: RegularBookingService constructor & overloads unchanged in behaviorThe DI constructor and the three public methods (
createBooking,rescheduleBooking,createBookingForApiV1) remain thin wrappers overhandler, just wiring throughbookingMetaand an optional schema getter. No behavioral changes beyond the internal handler updates.packages/prisma/migrations/20251208104340_add_max_round_robin_hosts_optional/migration.sql (1)
1-2: Migration formaxRoundRobinHostsis straightforwardAdding a nullable
INTEGERcolumnmaxRoundRobinHoststo"public"."EventType"aligns with the Prisma model (Int?) and service‑level defaulting. No issues here.packages/features/eventtypes/lib/types.ts (1)
84-182: FormValuesmaxRoundRobinHostsfield matches backend shapeAdding
maxRoundRobinHosts: number | nulltoFormValuesaligns with the PrismaInt?field and the rest of the PR (defaults set viadefaultEvents). As long as the form always initializes this to eithernullor a positive integer, this type is appropriate.packages/prisma/schema.prisma (1)
245-252:EventType.maxRoundRobinHostsmodel field is consistent with migrationDefining
maxRoundRobinHosts Int?onEventTypematches the migration and how the service code handles it (?? 1). Any bounds (e.g., enforcing>= 1) are correctly left to higher‑level validation (zod / API layer), so nothing else is needed in the schema.packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
18-89: SelectingmaxRoundRobinHostsfor booking flow is correctAdding
maxRoundRobinHosts: truetogetEventTypesFromDBSelectensures the booking flow sees this field and can apply the new round‑robin logic. The rest of the transformation already spreads all scalar fields viarestEventType, so this wires through cleanly.packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (1)
122-122: LGTM! Field correctly integrated into form defaults.The
maxRoundRobinHostsfield is properly added to the form's default values, sourced fromeventType.maxRoundRobinHosts. The placement alongside other round-robin configuration fields is logical.packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts (1)
642-649: LGTM! Input validation is properly configured.The field validation is consistent with the output type. The
@Min(1)validator correctly ensures that when a value is provided, it must be at least 1, while@IsOptional()allows the field to be omitted entirely.apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts (1)
97-97: LGTM! Type extension for team event type support.Adding
maxRoundRobinHoststo theInputtype extends theDatabaseEventTypeinterface to support this field system-wide. While this service'sgetResponseEventTypemethod returnsEventTypeOutput_2024_06_14(which doesn't include this field), the type extension ensures the field is available for team event type services that returnTeamEventTypeOutput_2024_06_14.packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts (1)
556-563: LGTM! Update input consistent with create input.The field definition matches the create input type, maintaining consistency across the API surface. The validation correctly allows the field to be omitted (optional) or provided with a minimum value of 1.
apps/api/v2/src/modules/organizations/event-types/services/output.service.ts (1)
32-92: Input type extension formaxRoundRobinHostslooks correctIncluding
maxRoundRobinHostsin theInputpick keeps this service aligned with the EventType model and other plumbing in the PR. No issues from a typing or usage perspective.packages/features/eventtypes/lib/defaultEvents.ts (1)
63-156: Sane default formaxRoundRobinHostsincommonsAdding
maxRoundRobinHosts: nulltocommonskeeps the default event shape in sync with the new field while deferring the actual defaulting (to 1) to business logic/UI. This is consistent with the rest of the event defaults.apps/web/public/static/locales/en/common.json (1)
4108-4110: New locale strings for max round robin hosts are clear and consistentThe label and description accurately describe the feature (“Maximum hosts per booking”, defaulting to 1) and follow existing copy style. No i18n issues spotted.
packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (2)
1089-1170: Well-structured test helpers for maxRoundRobinHosts scenarios.The helper functions (
createBooker,createOrganizer,createTeamMember,createTeamMembers,setupScenario,createBooking) effectively reduce code duplication and improve test readability. The conditional spread on line 1139 correctly handles the optionalmaxRoundRobinHostsparameter.
1191-1236: Good test coverage for maxRoundRobinHosts core scenarios.The tests cover the essential cases:
- Assigning exactly
maxRoundRobinHostswhen enough hosts are available- Gracefully handling fewer available hosts than the max
- Default behavior when
maxRoundRobinHostsis undefined- Explicit
maxRoundRobinHosts: 1behaviorConsider adding a test for the scenario where
maxRoundRobinHostsis set but some hosts are busy (existing bookings), to verify the system correctly assigns only available hosts up to the max.apps/api/v2/src/modules/organizations/event-types/max-round-robin-hosts.e2e-spec.ts (2)
207-230: Good E2E coverage for create and update scenarios.The tests validate:
- Setting
maxRoundRobinHostson creation- Default
nullwhen not provided- Updating the value
Consider adding a test to verify that
maxRoundRobinHostscan be cleared (set back tonull) after being set, to ensure the full update lifecycle is covered.
199-205: No changes needed. The cleanup implementation is correct—cascade deletion is properly configured in the Prisma schema to handle all dependent entities when the organization is deleted, including managedTeam and all event types created during tests.Likely an incorrect or invalid review comment.
packages/features/eventtypes/repositories/eventTypeRepository.ts (3)
606-606: Correctly addedmaxRoundRobinHoststo the event type select.This ensures the field is included when fetching event type data for editing and management.
905-905: Consistent withfindById- org admin access includesmaxRoundRobinHosts.This mirrors the change in
findById, ensuring organization admins have access to the same data.
1265-1265: Essential for slot calculation logic.Including
maxRoundRobinHostsinfindForSlotsis necessary for the round-robin host selection algorithm to respect the configured maximum hosts per booking.packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx (5)
30-30: LGTM - Import consolidation.Adding
NumberInputto the existing import from@calcom/ui/components/formkeeps imports organized.
104-110: Defensive null check for onChange.The explicit
if (onChange)guard prevents potential runtime errors whenonChangeis not provided, making the component more robust.
282-282: Unused parameter prefixed with underscore.Prefixing
orgIdwith_indicates it's intentionally unused while maintaining the interface contract. This follows common conventions for signaling unused parameters.
752-752: Simplified conditional is cleaner.Removing the double-negation (
!!) from the truthiness check improves readability without changing behavior.
918-921: Clearer conditional logic for tooltip content.The direct evaluation of
eventType.team?.rrTimestampBasis && eventType.team?.rrTimestampBasis !== RRTimestampBasis.CREATED_ATis more readable than the previous double-negation approach.docs/api-reference/v2/openapi.json (4)
13844-13844: LGTM!The enhanced description clearly documents the slot format options, seated slot properties, and empty response behavior.
14455-14455: LGTM!
29460-29463: LGTM!Good practice to proactively notify API consumers about upcoming authentication requirements.
29491-29494: LGTM!
| @IsInt() | ||
| @Min(1) | ||
| @IsOptional() | ||
| @ApiPropertyOptional({ | ||
| 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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Prisma schema for default value
echo "=== Checking Prisma schema for maxRoundRobinHosts default ==="
rg "maxRoundRobinHosts" packages/prisma/schema.prisma -A 2 -B 2
# Check service layer for default value handling
echo -e "\n=== Checking service layer for default value handling ==="
find apps -type f -name "*.ts" | xargs rg "maxRoundRobinHosts" -B 3 -A 3Repository: coderabbit-test/cal.com
Length of output: 7486
Fix misleading documentation about default value.
The description states "defaults to 1", but the Prisma schema has no default value defined for this field (maxRoundRobinHosts Int?), and e2e tests confirm the field returns null when not provided. Update the description to accurately reflect that the field is optional and defaults to null when not set, or implement an actual database-level default if that was the intent.
Nitpicks 🔍
|
CodeAnt-AI Description
Support assigning multiple hosts per Round Robin booking with maxRoundRobinHosts
What Changed
Impact
✅ Assign multiple hosts per round-robin booking✅ Fewer booking failures when multiple hosts are available✅ Clearer admin control over host count 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.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.