fix: calendar invite organizer reassignment#24395
fix: calendar invite organizer reassignment#24395husniabad wants to merge 2 commits intocalcom:mainfrom
Conversation
|
@husniabad is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
Walkthrough
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/ee/round-robin/roundRobinManualReassignment.test.ts (1)
655-665: Align the expectation with an organizer change.Here the booking is reassigned from the Google Meet host to the Zoom host, so the organizer switches. The test should assert
changedOrganizer: true; keeping itfalsehides the bug and will fail once the implementation is corrected.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/ee/round-robin/handleRescheduleEventManager.ts(1 hunks)packages/features/ee/round-robin/roundRobinManualReassignment.test.ts(5 hunks)packages/features/ee/round-robin/roundRobinManualReassignment.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/ee/round-robin/roundRobinManualReassignment.tspackages/features/ee/round-robin/handleRescheduleEventManager.tspackages/features/ee/round-robin/roundRobinManualReassignment.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/ee/round-robin/roundRobinManualReassignment.tspackages/features/ee/round-robin/handleRescheduleEventManager.tspackages/features/ee/round-robin/roundRobinManualReassignment.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/round-robin/roundRobinManualReassignment.tspackages/features/ee/round-robin/handleRescheduleEventManager.tspackages/features/ee/round-robin/roundRobinManualReassignment.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
🧬 Code graph analysis (2)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
packages/features/ee/round-robin/handleRescheduleEventManager.ts (1)
handleRescheduleEventManager(28-207)
packages/features/ee/round-robin/roundRobinManualReassignment.test.ts (1)
apps/web/test/utils/bookingScenario/expects.ts (1)
expectBookingToBeInDatabase(418-435)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
| expectEventManagerCalledWith(eventManagerRescheduleSpy, { | ||
| uid: bookingToReassignUid, | ||
| changedOrganizer: true, | ||
| changedOrganizer: false, | ||
| destinationCalendars: expect.arrayContaining([expect.objectContaining(testDestinationCalendar)]), | ||
| }); |
There was a problem hiding this comment.
Expect changedOrganizer to be true when the host actually changes.
In this scenario the booking organizer moves from the original host to newHost, so hasOrganizerChanged evaluates to true. Forcing the expectation to false masks regressions and contradicts the intended behavior once we pass the correct flag to the rescheduler. Please update the assertion to expect true (and adjust mocks accordingly).
🤖 Prompt for AI Agents
In packages/features/ee/round-robin/roundRobinManualReassignment.test.ts around
lines 302 to 306, the test currently asserts changedOrganizer: false even though
the booking organizer is moved to newHost; update the expectation to
changedOrganizer: true to match hasOrganizerChanged being true, and ensure any
related mocks/spies that construct or validate the rescheduler payload are
updated to reflect the organizer change (e.g., mock
destinationCalendar/organizer fields and any helper that computes
hasOrganizerChanged) so the test consistently reflects the new organizer state.
| const { evtWithAdditionalInfo } = await handleRescheduleEventManager({ | ||
| evt, | ||
| rescheduleUid: booking.uid, | ||
| newBookingId: undefined, | ||
| changedOrganizer: hasOrganizerChanged, | ||
| changedOrganizer: false, | ||
| previousHostDestinationCalendar: previousHostDestinationCalendar ? [previousHostDestinationCalendar] : [], |
There was a problem hiding this comment.
Restore hasOrganizerChanged when calling handleRescheduleEventManager.
Hard-coding changedOrganizer: false means the reschedule flow is never told that the host/organizer switched. Two concrete regressions follow:
- Inside
handleRescheduleEventManager,skipDeleteEventsAndMeetingsis derived from this flag, so we go back to deleting/recreating the event instead of updating it—exactly what the PR set out to avoid. EventManager.reschedulenever receives the organizer-change signal, so connectors (Google/Outlook) keep the old organizer, reintroducing the original bug.
Please pass hasOrganizerChanged as before:
newBookingId: undefined,
- changedOrganizer: false,
+ changedOrganizer: hasOrganizerChanged,This keeps the deletion skip in place and ensures external calendars get the organizer update.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { evtWithAdditionalInfo } = await handleRescheduleEventManager({ | |
| evt, | |
| rescheduleUid: booking.uid, | |
| newBookingId: undefined, | |
| changedOrganizer: hasOrganizerChanged, | |
| changedOrganizer: false, | |
| previousHostDestinationCalendar: previousHostDestinationCalendar ? [previousHostDestinationCalendar] : [], | |
| const { evtWithAdditionalInfo } = await handleRescheduleEventManager({ | |
| evt, | |
| rescheduleUid: booking.uid, | |
| newBookingId: undefined, | |
| changedOrganizer: hasOrganizerChanged, | |
| previousHostDestinationCalendar: previousHostDestinationCalendar ? [previousHostDestinationCalendar] : [], |
🤖 Prompt for AI Agents
In packages/features/ee/round-robin/roundRobinManualReassignment.ts around lines
334 to 339, the call to handleRescheduleEventManager hard-codes
changedOrganizer: false which prevents signaling organizer changes; replace the
hard-coded false with the existing hasOrganizerChanged variable (i.e., pass
changedOrganizer: hasOrganizerChanged) so skipDeleteEventsAndMeetings logic and
EventManager.reschedule receive the correct organizer-change flag.
|
Hi @husniabad, |
|
Hi @ibex088 thanks for your valuable feedback When I re-assign booking, the event got removed from host's A linked calendar and recreate it in host's B linked calendar using update method. how update method works:
Key PointBy keeping the same iCalUID and using UPDATE, the calendar system handles the migration atomically. DELETE+CREATE would use different UIDs and risk duplicates/orphans. |
|
cc: @CarinaWolli |
Problem: This causes the event to be updated in the old host’s calendar instead of being deleted there(old host's calendar) and created in the new host’s calendar -- which is not the intended behavior. |
What does this PR do?
Fixes the calendar invite owner not updating when a booking is reassigned to a different team member. After reassignment, the calendar invite now correctly reflects the new host as the owner in both Google Calendar and Outlook.
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
#Before
#24357-before - Watch Video
#After
#24357-after - Watch Video
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
##Key changes:
Update organizer field to use new user's details when host changes
Preserve original iCalUID to prevent duplicate calendar events
Update existing calendar event instead of delete+create pattern
Remove blocking video integration validation (consistent with initial booking behavior)
Show individual host name instead of team name in event title
How should this be tested?
#Prerequisites:
#Test steps:
#Expected outcome:
Calendar event organizer updates to new host
Single event (no duplicates)
Event title shows individual host name
Reassignment completes without errors
Are there environment variables that should be set?
What are the minimal test data to have?
What is expected (happy path) to have (input and output)?
Any other important info that could help to test that PR
Checklist