fix: duplicate event created in attendee calendar due to incorrect icalUid#23658
fix: duplicate event created in attendee calendar due to incorrect icalUid#23658anikdhabal merged 4 commits intocalcom:mainfrom
Conversation
WalkthroughAdds propagation of booking iCal UID into CalendarEvent payloads and a fallback for reschedule handling: two reassignment entry points now set Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. Comment |
|
@saurabhraghuvanshii is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
Low priority
dosubot
bot
added
the
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/features/ee/round-robin/roundRobinReassignment.ts (1)
342-353: Avoid Prisma include; rely on top-level organizer email or use selectWe already have organizer.email. The extra include of user.email on credentials is unnecessary and violates our “no include, prefer select” rule for Prisma.
Apply:
- const credentials = await prisma.credential.findMany({ - where: { - userId: organizer.id, - }, - include: { - user: { - select: { - email: true, - }, - }, - }, - }); + const credentials = await prisma.credential.findMany({ + where: { userId: organizer.id }, + });Optionally, further tighten with a select of only the credential fields consumed by enrichUserWithDelegationCredentialsIncludeServiceAccountKey.
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
311-314: Drop Prisma include; prefer select or rely on existing organizer contextIncluding user.email on credentials is unnecessary; you already have organizer/newUser email at the top level. Remove include to comply with our “no include” rule. If you need specific credential fields, add an explicit select instead.
- const credentials = await prisma.credential.findMany({ - where: { userId: newUser.id }, - include: { user: { select: { email: true } } }, - }); + const credentials = await prisma.credential.findMany({ + where: { userId: newUser.id }, + });
🧹 Nitpick comments (4)
packages/features/ee/round-robin/roundRobinReassignment.ts (2)
470-471: Prefer named export over default exportProject guideline favors named exports for better tree-shaking and refactors.
-export default roundRobinReassignment; +export { roundRobinReassignment };
298-306: Fetch-all calendars is correct; consider narrowing Prisma selection and confirm manager expects an array
- Switching to findMany ensures all destination calendars are considered for cleanup; passing an array into the rescheduler matches the intent.
- Recommendation: follow our Prisma guideline and select only fields needed by the cleanup (rather than returning full rows). If the rescheduler only needs identifiers (e.g., id, externalCalendarId, credentialId, integration), add a select clause to reduce payload and risk surface.
Run to confirm the rescheduler now expects an array and what fields it uses:
#!/bin/bash # Show handleRescheduleEventManager signature and usages of previousHostDestinationCalendar rg -nP -C3 '(export\s+)?(async\s+)?(function|const)\s+handleRescheduleEventManager\b' rg -nP -C3 'previousHostDestinationCalendar\b' packages | sed -n '1,120p'Also applies to: 364-365
packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)
570-571: Prefer named exportSwitch to named export to follow repo conventions.
-export default roundRobinManualReassignment; +export { roundRobinManualReassignment };
320-325: Good: fetch all previous host calendars and pass array; narrow selection for PrismafindMany + passing an array fixes multi-calendar cleanup. To align with our Prisma guideline, select only fields needed by the rescheduler rather than returning full rows.
Verify the rescheduler’s parameter type and accessed fields before adding select:
#!/bin/bash # Confirm expected type is DestinationCalendar[] and discover used fields rg -nP -C3 '(export\s+)?(async\s+)?(function|const)\s+handleRescheduleEventManager\b' rg -nP -C3 'previousHostDestinationCalendar\b' packages | head -n 200Also applies to: 331-332
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
packages/features/ee/round-robin/roundRobinManualReassignment.ts(3 hunks)packages/features/ee/round-robin/roundRobinReassignment.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/roundRobinReassignment.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/roundRobinReassignment.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/roundRobinReassignment.ts
🧠 Learnings (2)
📓 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.
Learnt from: ShashwatPS
PR: calcom/cal.com#23638
File: packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts:198-199
Timestamp: 2025-09-06T11:00:34.348Z
Learning: In calcom/cal.com PR #23638, the maintainer ShashwatPS determined that authorization checks in the setDestinationReminder handler are "not applicable" when CodeRabbit suggested adding user-scoped WHERE clauses to prevent users from modifying other users' destination calendar reminder settings.
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.
📚 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/ee/round-robin/roundRobinManualReassignment.tspackages/features/ee/round-robin/roundRobinReassignment.ts
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
packages/features/ee/round-robin/roundRobinReassignment.ts (2)
268-280: Good guard to prevent null attendee updateWrapping the attendee update in an existence check avoids a runtime error when the previous RR host isn’t an attendee. Looks correct.
375-375: Intentional omission of cancellationReason is fineRenaming via destructuring to drop cancellationReason from outbound emails is clear and lint-friendly. LGTM.
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
342-342: Clean omission of cancellationReasonThe destructuring rename to _cancellationReason cleanly excludes it from outbound emails. Looks good.
anikdhabal
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/features/ee/round-robin/handleRescheduleEventManager.ts (3)
163-166: Prefer nullish coalescing and include createdEvent; normalize null/undefined for iCalUIDUse ?? instead of || to avoid falling back on empty-string UIDs, also consider createdEvent as a fallback, and avoid assigning null to evt.iCalUID. This keeps types consistent and improves resilience when providers return sparse payloads.
- evt.iCalUID = Array.isArray(calendarResult?.updatedEvent) - ? calendarResult?.updatedEvent[0]?.iCalUID || bookingICalUID - : calendarResult?.updatedEvent?.iCalUID || bookingICalUID || undefined; + const updatedEvt = calendarResult?.updatedEvent; + const createdEvt = calendarResult?.createdEvent; + const resolvedICalUID = + Array.isArray(updatedEvt) ? updatedEvt[0]?.iCalUID : (updatedEvt?.iCalUID ?? createdEvt?.iCalUID); + evt.iCalUID = resolvedICalUID ?? bookingICalUID ?? undefined;
151-160: Derive createdOrUpdatedEvent from the calendar result, not results[0]results[0] may be a non-calendar app result (e.g., video). Using the calendar result prevents accidental undefined reads and aligns metadata with the actual calendar event.
- const createdOrUpdatedEvent = Array.isArray(results[0]?.updatedEvent) - ? results[0]?.updatedEvent[0] - : results[0]?.updatedEvent ?? results[0]?.createdEvent; + const calRes = results.find((r) => r.type.includes("_calendar")); + const createdOrUpdatedEvent = Array.isArray(calRes?.updatedEvent) + ? calRes?.updatedEvent[0] + : calRes?.updatedEvent ?? calRes?.createdEvent;
186-195: Avoid passing undefined to Prisma update; include iCalUID conditionallyPrisma generally ignores omitted fields but an explicit undefined can be noisy or rejected by typings. Include iCalUID only when it’s resolved.
- await prisma.booking.update({ + const iCalUIDToPersist = evt.iCalUID ?? bookingICalUID; + await prisma.booking.update({ where: { id: bookingId, }, data: { location: bookingLocation, - iCalUID: evt.iCalUID !== bookingICalUID ? evt.iCalUID : bookingICalUID, + ...(iCalUIDToPersist !== undefined ? { iCalUID: iCalUIDToPersist } : {}), metadata: { ...(typeof bookingMetadata === "object" && bookingMetadata), ...newBookingMetaData }, }, });Would you confirm the Booking.iCalUID column is nullable and Prisma’s generated type marks it optional/null? If not, I can adjust the snippet accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- 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.ts(1 hunks)packages/features/ee/round-robin/roundRobinReassignment.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/features/ee/round-robin/roundRobinManualReassignment.ts
- packages/features/ee/round-robin/roundRobinReassignment.ts
🧰 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/handleRescheduleEventManager.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/handleRescheduleEventManager.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/handleRescheduleEventManager.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.
⏰ 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). (8)
- GitHub Check: Production builds / Build Web App
- GitHub Check: Production builds / Build Docs
- GitHub Check: Production builds / Build API v2
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Production builds / Build API v1
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
E2E results are ready! |
2 participants
What does this PR do?
duplicate event created in attendee calendar due to incorrect icalUid