fix: resolve credential mismatch in round robin reschedule causing 404 errors #22993
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
WalkthroughThe changes introduce logic to handle cases where the organizer of a rescheduled booking in a round robin event type has changed. In the booking handler, the code now retrieves and refreshes credentials for the original organizer, creates an EventManager instance with those credentials, and deletes associated calendar events and meetings for the previous booking. The username and timeFormat properties are also included in the email data for original booking members. In the EventManager class, the deletion logic during rescheduling is adjusted to skip deletion when the organizer changes and the event requires confirmation, and the deleteEventsAndMeetings method is made public. In the booking repository, credential selection is refactored to use a centralized selector, and the username and timeFormat fields are included in user selections. A new test case verifies that the original host's credentials are used for deletion and the new host's credentials for creation during a round robin reschedule with a changed organizer. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/09/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/11/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/lib/server/repository/booking.ts (1)
658-688: Replaceincludewithselectto meet Prisma selection guidelinesRepo guideline: for Prisma queries, only select what you need; never use include. This method still uses include. Suggest converting to select for tighter payloads and clearer contracts.
Example refactor:
- include: { + select: { attendees: { select: { name: true, email: true, locale: true, timeZone: true, phoneNumber: true, ...(seatsEventType && { bookingSeat: true, id: true }), }, }, user: { select: { id: true, name: true, username: true, email: true, locale: true, timeZone: true, destinationCalendar: true, credentials: { select: credentialForCalendarServiceSelect, }, }, }, - destinationCalendar: true, - payment: true, - references: true, - workflowReminders: true, + destinationCalendar: true, + payment: true, + references: true, + workflowReminders: true, },
🧹 Nitpick comments (2)
packages/lib/EventManager.ts (1)
687-746: Public deleteEventsAndMeetings: add explicit types to avoid implicit any[]Making the method public is necessary for external cleanup. Minor nit: initialize arrays with explicit types for clarity and stricter typing.
public async deleteEventsAndMeetings({ @@ - const log = logger.getSubLogger({ prefix: [`[deleteEventsAndMeetings]: ${event?.uid}`] }); - const calendarReferences = [], - videoReferences = [], - crmReferences = [], - allPromises = []; + const log = logger.getSubLogger({ prefix: [`[deleteEventsAndMeetings]: ${event?.uid}`] }); + const calendarReferences: PartialReference[] = []; + const videoReferences: PartialReference[] = []; + const crmReferences: PartialReference[] = []; + const allPromises: Array<Promise<unknown>> = [];packages/features/bookings/lib/handleNewBooking.ts (1)
1570-1576: Perform old host cleanup before mutating evt identifiers (optional ordering tweak)Great fix to delete previous host’s events using the original host’s refreshed credentials. Consider moving the iCalUID/videoCallData resets after the deletion to preserve maximum context for providers that might read event metadata during delete (even though references are primarily used).
- if (changedOrganizer) { - // location might changed and will be new created in eventManager.create (organizer default location) - evt.videoCallData = undefined; - // To prevent "The requested identifier already exists" error while updating event, we need to remove iCalUID - evt.iCalUID = undefined; - } - - if (changedOrganizer && originalRescheduledBooking?.user) { + if (changedOrganizer && originalRescheduledBooking?.user) { // Create EventManager with original host's credentials for deletion operations const originalHostCredentials = await getAllCredentialsIncludeServiceAccountKey( originalRescheduledBooking.user, eventType ); const refreshedOriginalHostCredentials = await refreshCredentials(originalHostCredentials); const originalHostEventManager = new EventManager( { ...originalRescheduledBooking.user, credentials: refreshedOriginalHostCredentials }, apps ); log.debug("RescheduleOrganizerChanged: Deleting Event and Meeting for previous booking"); await originalHostEventManager.deleteEventsAndMeetings({ event: { ...evt, destinationCalendar: previousHostDestinationCalendar }, bookingReferences: originalRescheduledBooking.references, }); } + + if (changedOrganizer) { + // location might change and will be recreated in eventManager.create (organizer default location) + evt.videoCallData = undefined; + // Prevent "The requested identifier already exists" on update + evt.iCalUID = undefined; + }Also applies to: 1578-1594
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/features/bookings/lib/handleNewBooking.ts(2 hunks)packages/lib/EventManager.ts(2 hunks)packages/lib/server/repository/booking.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/repository/booking.tspackages/lib/EventManager.tspackages/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/lib/server/repository/booking.tspackages/lib/EventManager.tspackages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.352Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
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: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/lib/server/repository/booking.tspackages/lib/EventManager.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-08T09:27:23.870Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/lib/server/repository/booking.tspackages/lib/EventManager.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-08T09:29:11.665Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/lib/server/repository/booking.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-07-24T08:39:06.185Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
packages/lib/server/repository/booking.ts
📚 Learning: 2025-08-08T09:12:08.269Z
Learnt from: hariombalhara
PR: calcom/cal.com#22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.269Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.
Applied to files:
packages/lib/server/repository/booking.ts
📚 Learning: 2025-07-18T17:57:16.395Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Applied to files:
packages/lib/EventManager.ts
📚 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
🧬 Code Graph Analysis (1)
packages/lib/server/repository/booking.ts (1)
packages/prisma/selects/credential.ts (1)
credentialForCalendarServiceSelect(3-17)
⏰ 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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (3)
packages/lib/server/repository/booking.ts (2)
10-10: Good reuse: centralized credential selectImporting credentialForCalendarServiceSelect reduces duplication and keeps credential shape consistent across queries.
670-681: Add username + unify credential selection — OK; ensure no leakage of credential.key
- Adding user.username will help downstream (emails, logs).
- Swapping inline credential selection with credentialForCalendarServiceSelect is cleaner.
Note: credentialForCalendarServiceSelect includes credential.key. This repository method is server-side, but please double-check that objects returned from findOriginalRescheduledBooking are not serialized to client or exposed via tRPC/API responses.
packages/lib/EventManager.ts (1)
601-608: Correctness: skip deletion when organizer changed on requiresConfirmationChanging the condition to if (evt.requiresConfirmation && !changedOrganizer) prevents deleting previous events with the new organizer’s credentials. This aligns with the new external cleanup flow and fixes the credential mismatch during RR reschedules.
…hedule - Verify original host credentials used for deletion operations - Verify new host credentials used for creation operations - Test dual EventManager pattern implementation - Fix timeFormat property access issue Co-Authored-By: anik@cal.com <adhabal2002@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (2)
2916-2928: Make host selection deterministic to avoid flakes.The host change currently relies on schedule shifts (morning vs evening). To harden the test, explicitly route to the new host so future schedule fixtures don’t accidentally select the original host.
Apply this diff:
const mockBookingData = getMockRequestDataForBooking({ data: { eventTypeId: 1, rescheduleUid: uidOfBookingToBeRescheduled, start: `${plus1DateString}T14:00:00.000Z`, end: `${plus1DateString}T14:15:00.000Z`, + // Ensure host selection is deterministic for this test + routedTeamMemberIds: [newHost.id], responses: { email: booker.email, name: booker.name, location: { optionValue: "", value: BookingLocations.CalVideo }, }, }, });
2934-2948: Also assert call counts for stronger guarantees.You already assert one create call. Consider mirroring that for delete to ensure exactly one deletion happened as part of reschedule.
Apply this diff:
expectSuccessfulCalendarEventDeletionInCalendar(calendarMock, { externalCalendarId: "MOCK_EXTERNAL_CALENDAR_ID", calEvent: { organizer: expect.objectContaining({ email: originalHost.email, }), }, uid: "MOCK_ID", }); // Verify that creation occurred with new host credentials +expect(calendarMock.deleteEventCalls.length).toBe(1); expect(calendarMock.createEventCalls.length).toBe(1); const createCall = calendarMock.createEventCalls[0]; expect(createCall.args.calEvent.organizer.email).toBe(newHost.email);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/features/bookings/lib/handleNewBooking.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/bookings/lib/handleNewBooking.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/test/reschedule.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/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.352Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
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: vijayraghav-io
PR: calcom/cal.com#21072
File: .env.example:354-357
Timestamp: 2025-07-18T08:49:18.779Z
Learning: For E2E integration tests that require real third-party service credentials (like Outlook calendar), it's acceptable to temporarily include actual test account credentials in .env.example during feature development and validation, provided there's a clear plan to replace them with placeholder values before final release. Test credentials should be for dedicated test tenants/accounts, not production systems.
📚 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/test/reschedule.test.ts
📚 Learning: 2025-07-18T08:49:18.779Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: .env.example:354-357
Timestamp: 2025-07-18T08:49:18.779Z
Learning: For E2E integration tests that require real third-party service credentials (like Outlook calendar), it's acceptable to temporarily include actual test account credentials in .env.example during feature development and validation, provided there's a clear plan to replace them with placeholder values before final release. Test credentials should be for dedicated test tenants/accounts, not production systems.
Applied to files:
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.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
🔇 Additional comments (1)
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (1)
2831-2954: Great targeted test covering the credential split between delete and create.This validates the core regression: deletion uses the original host’s credentials and creation uses the new host’s credentials. It directly aligns with the PR’s objective and mirrors the expected EventManager behavior. Nice job asserting organizer email on both paths and that the new booking is assigned to the new host.
Devanshusharma2005
approved these changes
Aug 11, 2025
zomars
approved these changes
Aug 11, 2025
E2E results are ready! |
This was referenced Aug 14, 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.
Fix credential mismatch in round robin reschedule causing 404 errors
What does this PR do?
Resolves a critical issue where round robin reschedule operations fail with Google Calendar API 404 errors when the host changes. The root cause was a credential mismatch: the system was using the new host's credentials to delete events from the original host's calendar, which lacks proper access permissions.