fix(slots): include guest availability when rescheduling round-robin bookings#28806
Open
dhelland wants to merge 2 commits intocalcom:mainfrom
Open
fix(slots): include guest availability when rescheduling round-robin bookings#28806dhelland wants to merge 2 commits intocalcom:mainfrom
dhelland wants to merge 2 commits intocalcom:mainfrom
Conversation
…bookings (calcom#16378) When a guest (attendee who is also a Cal.com user) reschedules a round-robin booking, their availability was not being checked, which could lead to double-bookings. Changes: - Add UserRepository.findAvailabilityUserByEmail() to look up a Cal.com user by email including their credentials and selected calendars - Add AvailableSlotsService._getRescheduledGuestUser() which: - Returns null for collective events (all attendees must attend, so guest availability is irrelevant for slot selection) - Finds the non-organizer attendee from the original booking - Looks up that attendee as a Cal.com user - Returns null if they are not a Cal.com user or their account is locked - Include the rescheduled guest as a fixed host in slot availability calculations (both the main two-week window and the fallback RR path) - Add unit tests covering all edge cases of _getRescheduledGuestUser
|
|
github-actions
bot
added
$200
bookings
Contributor
📝 WalkthroughWalkthroughThe changes implement rescheduled guest user inclusion in slot availability calculations. A new repository method queries users by email with availability data. Utility logic detects rescheduled guest requirements and retrieves the guest's availability information, excluding organizer attendees. The guest is conditionally injected into host calculations for non-collective scheduling types. Test coverage validates the guest lookup method's behavior across various scenarios including missing reschedule IDs, collective scheduling, locked users, and absent attendees, and confirms guest integration into slot calculations during primary and fallback path executions. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/slots/_getAvailableSlots.rescheduleGuest.test.ts (1)
65-121: Avoidas anyin this test harness.The repeated
(service as any)casts throw away the type coverage this test should give us. Please cast once to a narrow test-only shape or use typed bracket access for the private members you need to stub/call. As per coding guidelines, "Never useas any- use proper type-safe solutions instead."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/server/routers/viewer/slots/_getAvailableSlots.rescheduleGuest.test.ts` around lines 65 - 121, The test currently uses repeated (service as any) casts which lose type safety; create a single narrow test-only typed reference (e.g., const svc = service as unknown as Partial<AvailableSlotsService> & { getRegularOrDynamicEventType(...): Promise<any>; resolveOrganizationIdForBlocking(...): Promise<any>; getRescheduledGuestUser(...): Promise<any>; checkRestrictionScheduleEnabled(...): Promise<any>; _getReservedSlotsAndCleanupExpired(...): Promise<any>; calculateHostsAndAvailabilities(...): Promise<any>; _getAvailableSlots(...): Promise<any>; }) and use svc to assign vi.fn() stubs and call _getAvailableSlots, or alternatively use typed bracket access (service['calculateHostsAndAvailabilities'] = ...) for each private member; replace every (service as any) occurrence with that single typed reference so the test keeps static typing while still stubbing getRegularOrDynamicEventType, resolveOrganizationIdForBlocking, getRescheduledGuestUser, checkRestrictionScheduleEnabled, _getReservedSlotsAndCleanupExpired, calculateHostsAndAvailabilities and calling _getAvailableSlots.packages/trpc/server/routers/viewer/slots/_getRescheduledGuestUser.test.ts (1)
33-124: Please remove theas anycalls here as well.These casts make the test less useful by opting out of type checking around the exact helper contract you're validating. A small test-only interface for
_getRescheduledGuestUserwould keep the setup explicit without dropping toany. As per coding guidelines, "Never useas any- use proper type-safe solutions instead."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/server/routers/viewer/slots/_getRescheduledGuestUser.test.ts` around lines 33 - 124, Remove the unsafe "as any" casts around service when calling _getRescheduledGuestUser; instead define a small test-only interface/type that declares _getRescheduledGuestUser's exact signature (e.g., TestService { _getRescheduledGuestUser(args: { rescheduleUid: string | null; organizerEmails: string[]; schedulingType: SchedulingType }): Promise<User | null> }) and cast service to that TestService type for the calls in these tests, then replace every `(service as any)._getRescheduledGuestUser` use with `(service as TestService)._getRescheduledGuestUser`; this preserves type checking while keeping the test explicit about the helper contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/trpc/server/routers/viewer/slots/util.ts`:
- Around line 1299-1306: The organizerEmails array is currently built only from
allHosts so hosts that exist solely in eligibleFallbackRRHosts are omitted,
causing getRescheduledGuestUser (this.getRescheduledGuestUser) to misidentify
attendees; update the organizerEmails construction to include emails from every
possible organizer host by combining allHosts and eligibleFallbackRRHosts (and
any other host collections used earlier), then map to
host.user.email.toLowerCase(), filter falsy values, and dedupe with Set before
passing organizerEmails into this.getRescheduledGuestUser({ rescheduleUid:
input.rescheduleUid, organizerEmails, schedulingType: eventType.schedulingType
}).
---
Nitpick comments:
In
`@packages/trpc/server/routers/viewer/slots/_getAvailableSlots.rescheduleGuest.test.ts`:
- Around line 65-121: The test currently uses repeated (service as any) casts
which lose type safety; create a single narrow test-only typed reference (e.g.,
const svc = service as unknown as Partial<AvailableSlotsService> & {
getRegularOrDynamicEventType(...): Promise<any>;
resolveOrganizationIdForBlocking(...): Promise<any>;
getRescheduledGuestUser(...): Promise<any>;
checkRestrictionScheduleEnabled(...): Promise<any>;
_getReservedSlotsAndCleanupExpired(...): Promise<any>;
calculateHostsAndAvailabilities(...): Promise<any>; _getAvailableSlots(...):
Promise<any>; }) and use svc to assign vi.fn() stubs and call
_getAvailableSlots, or alternatively use typed bracket access
(service['calculateHostsAndAvailabilities'] = ...) for each private member;
replace every (service as any) occurrence with that single typed reference so
the test keeps static typing while still stubbing getRegularOrDynamicEventType,
resolveOrganizationIdForBlocking, getRescheduledGuestUser,
checkRestrictionScheduleEnabled, _getReservedSlotsAndCleanupExpired,
calculateHostsAndAvailabilities and calling _getAvailableSlots.
In `@packages/trpc/server/routers/viewer/slots/_getRescheduledGuestUser.test.ts`:
- Around line 33-124: Remove the unsafe "as any" casts around service when
calling _getRescheduledGuestUser; instead define a small test-only
interface/type that declares _getRescheduledGuestUser's exact signature (e.g.,
TestService { _getRescheduledGuestUser(args: { rescheduleUid: string | null;
organizerEmails: string[]; schedulingType: SchedulingType }): Promise<User |
null> }) and cast service to that TestService type for the calls in these tests,
then replace every `(service as any)._getRescheduledGuestUser` use with
`(service as TestService)._getRescheduledGuestUser`; this preserves type
checking while keeping the test explicit about the helper contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57385b23-3821-41da-a639-e8d4749ffb04
📒 Files selected for processing (4)
packages/features/users/repositories/UserRepository.tspackages/trpc/server/routers/viewer/slots/_getAvailableSlots.rescheduleGuest.test.tspackages/trpc/server/routers/viewer/slots/_getRescheduledGuestUser.test.tspackages/trpc/server/routers/viewer/slots/util.ts
Comment on lines
+1299
to
+1306
| const organizerEmails = Array.from( | ||
| new Set(allHosts.map((host) => host.user.email.toLowerCase()).filter(Boolean)) | ||
| ); | ||
| const rescheduledGuestUser = await this.getRescheduledGuestUser({ | ||
| rescheduleUid: input.rescheduleUid, | ||
| organizerEmails, | ||
| schedulingType: eventType.schedulingType, | ||
| }); |
Contributor
There was a problem hiding this comment.
Build organizerEmails from every possible organizer host.
Right now this list comes from allHosts, so a round-robin organizer who only exists in eligibleFallbackRRHosts (or was filtered out before this point) is invisible to _getRescheduledGuestUser(). Then their attendee row can be mistaken for the guest, and the host gets appended again as a fixed participant. That breaks exactly the fallback path this PR is trying to fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/server/routers/viewer/slots/util.ts` around lines 1299 - 1306,
The organizerEmails array is currently built only from allHosts so hosts that
exist solely in eligibleFallbackRRHosts are omitted, causing
getRescheduledGuestUser (this.getRescheduledGuestUser) to misidentify attendees;
update the organizerEmails construction to include emails from every possible
organizer host by combining allHosts and eligibleFallbackRRHosts (and any other
host collections used earlier), then map to host.user.email.toLowerCase(),
filter falsy values, and dedupe with Set before passing organizerEmails into
this.getRescheduledGuestUser({ rescheduleUid: input.rescheduleUid,
organizerEmails, schedulingType: eventType.schedulingType }).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #16378
This updates the reschedule availability flow so that when a host reschedules a booking, the attendee's availability is also considered if the attendee is a Cal.com user.
Instead of checking only existing guest bookings, this adds the guest into the normal availability pipeline as a fixed participant. That means their calendar conflicts, working hours, OOO, and booking limits are all respected during slot calculation.
It also applies through the round-robin fallback paths, so guest availability is not dropped when fallback host selection is used.
Tests added:
_getRescheduledGuestUsercoverage for lookup and guard cases/claim #16378