Skip to content

fix: increment iCalSequence when changing booking location #22847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 31, 2025

Conversation

joeauyeung
Copy link
Contributor

fix: increment iCalSequence when changing booking location

Summary

Fixes an issue where changing a booking location doesn't increment the ICS sequence number, preventing calendar applications from properly updating existing events instead of creating duplicates.

Key Changes:

  • Modified editLocation.handler.ts to increment iCalSequence by 1 when location changes
  • Updated buildCalEventFromBooking.ts to accept and include iCalSequence in CalendarEvent objects
  • Updated database to store the incremented sequence value
  • Fixed related tests to account for new CalendarEvent fields

Root Cause: The editLocation handler was updating the booking location but not incrementing the ICS sequence number, which violates the ICS specification requirement that sequence numbers must increment when event details change. This caused calendar applications to treat location changes as separate events rather than updates to existing events.

Review & Testing Checklist for Human

  • End-to-end testing: Test location changes with actual calendar applications (Outlook, Google Calendar, Apple Calendar) to verify events update rather than create duplicates
  • Database schema verification: Confirm that the iCalSequence field exists in the Booking model and database schema
  • Regression testing: Test existing location change functionality to ensure no breaking changes
  • Edge case testing: Test rapid multiple location changes and verify sequence increments correctly
  • ICS specification compliance: Verify that incrementing by 1 follows ICS specification requirements for SEQUENCE field

Recommended Test Plan:

  1. Create a booking and add it to a calendar application
  2. Change the booking location through Cal.com
  3. Verify the calendar application updates the existing event (same UID) rather than creating a new event
  4. Check that the ICS file in the email has SEQUENCE:1 (or higher for subsequent changes)

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    editLocationHandler["packages/trpc/server/routers/<br/>viewer/bookings/<br/>editLocation.handler.ts"]:::major-edit
    buildCalEvent["packages/lib/<br/>buildCalEventFromBooking.ts"]:::major-edit
    testFile["packages/lib/__tests__/<br/>buildCalEventFromBooking.test.ts"]:::minor-edit
    
    
    emailManager["packages/emails/<br/>email-manager.ts"]:::context
    generateIcs["packages/emails/lib/<br/>generateIcsString.ts"]:::context
    prismaSchema["packages/prisma/<br/>schema.prisma"]:::context
    
    editLocationHandler -->|"calls with<br/>incremented sequence"| buildCalEvent
    buildCalEvent -->|"returns CalendarEvent<br/>with iCalSequence"| editLocationHandler
    editLocationHandler -->|"updates database<br/>with new sequence"| prismaSchema
    editLocationHandler -->|"sends emails with<br/>updated CalendarEvent"| emailManager
    emailManager -->|"generates ICS with<br/>sequence number"| generateIcs
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit  
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB  
    classDef context fill:#FFFFFF
Loading

Notes

  • This fix follows the same pattern used by cancel and reschedule operations in the codebase
  • The iCalSequence field already exists in the Prisma schema with a default value of 0
  • Type checking and unit tests pass, but comprehensive end-to-end testing with calendar applications is needed
  • The change is backward compatible as iCalSequence is optional in buildCalEventFromBooking

Link to Devin run: https://app.devin.ai/sessions/3b85205fc9494243a3f7bdf8abb066a5
Requested by: @joeauyeung

- Add iCalSequence parameter to buildCalEventFromBooking function
- Update editLocation handler to increment sequence by 1 in CalendarEvent
- Update database with incremented iCalSequence when location changes
- Update buildCalEventFromBooking test to include new fields
- Ensures ICS files properly update existing calendar events instead of creating duplicates

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Contributor

coderabbitai bot commented Jul 31, 2025

Walkthrough

The changes introduce and propagate two new fields, iCalUID and iCalSequence, to the booking domain model and related logic. The Booking type is extended to include these fields, and the buildCalEventFromBooking function now outputs them as part of the calendar event object. Test cases and booking object mocks are updated to include and verify these properties. The booking repository update logic is modified to optionally handle iCalSequence and responses during booking updates. The handler for updating booking locations now uses the repository method and increments iCalSequence as part of the update process.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15–20 minutes

  • Complexity arises from changes to type definitions, data propagation, update logic, and test coverage across several files, but the changes are localized and follow a clear pattern.

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d1a242 and 8e3d06b.

📒 Files selected for processing (4)
  • packages/lib/__tests__/buildCalEventFromBooking.test.ts (5 hunks)
  • packages/lib/buildCalEventFromBooking.ts (2 hunks)
  • packages/lib/server/repository/booking.ts (2 hunks)
  • packages/trpc/server/routers/viewer/bookings/editLocation.handler.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/lib/server/repository/booking.ts
  • packages/lib/__tests__/buildCalEventFromBooking.test.ts
  • packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts
  • packages/lib/buildCalEventFromBooking.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.ts
  • packages/lib/__tests__/buildCalEventFromBooking.test.ts
  • packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts
  • packages/lib/buildCalEventFromBooking.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.

Applied to files:

  • packages/lib/__tests__/buildCalEventFromBooking.test.ts
  • packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts
📚 Learning: the office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is spec...
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/__tests__/buildCalEventFromBooking.test.ts
📚 Learning: the filterhostsbysameroundrobinhost function in packages/lib/bookings/filterhostsbysameroundrobinhos...
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/trpc/server/routers/viewer/bookings/editLocation.handler.ts
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
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/trpc/server/routers/viewer/bookings/editLocation.handler.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class

Applied to files:

  • packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts
📚 Learning: the outlook calendar integration in cal.com intentionally reuses subscription ids across multiple ev...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.

Applied to files:

  • packages/lib/buildCalEventFromBooking.ts
📚 Learning: in cal.com's calendar integration, both google calendar and outlook calendar are designed to allow m...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.

Applied to files:

  • packages/lib/buildCalEventFromBooking.ts
🧬 Code Graph Analysis (1)
packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts (1)
packages/lib/server/repository/booking.ts (1)
  • BookingRepository (116-872)
⏰ 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: Detect changes
🔇 Additional comments (10)
packages/lib/buildCalEventFromBooking.ts (2)

58-59: LGTM! Type extension aligns with iCal requirements.

The addition of iCalUID and iCalSequence fields to the Booking type is correct and necessary for proper iCal sequence handling in calendar applications.


115-116: LGTM! Proper fallback logic for iCal fields.

The fallback logic is sound - using booking.uid when iCalUID is null ensures a unique identifier is always available, and defaulting iCalSequence to 0 follows the iCal specification.

packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts (3)

13-13: LGTM! Good use of repository pattern.

Adding the BookingRepository import supports better data access abstraction compared to direct Prisma usage.


101-103: LGTM! Proper repository instantiation and method usage.

The BookingRepository instantiation and method call follow the correct pattern and match the repository interface.


118-118: LGTM! Correct iCal sequence increment implementation.

The logic (evt.iCalSequence || 0) + 1 properly increments the sequence number as required by the iCal specification when event details change. This will ensure calendar applications update existing events instead of creating duplicates.

packages/lib/server/repository/booking.ts (2)

525-525: LGTM! Well-typed optional parameters.

The addition of optional responses and iCalSequence parameters with appropriate types enhances the method's flexibility while maintaining type safety.

Also applies to: 532-533


543-544: LGTM! Proper conditional field inclusion.

The conditional spreading logic is well-implemented - using !== undefined for iCalSequence correctly handles the case where 0 is a valid value, while the truthy check for responses is appropriate for object types.

packages/lib/__tests__/buildCalEventFromBooking.test.ts (3)

55-56: LGTM! Consistent test data with appropriate defaults.

Adding iCalSequence: 0 and iCalUID: "icaluid" to the createBooking helper ensures consistent test data across all test cases with sensible default values.


126-129: LGTM! Accurate test expectations for new fields.

The expected values correctly validate the new iCal fields - iCalSequence: 0 matches the mock data, and iCalUID: booking.iCalUID properly tests the field propagation.


144-145: LGTM! Comprehensive test coverage for edge cases.

The consistent inclusion of iCal fields across all test scenarios (missing fields, null destination calendar) ensures robust validation of the new functionality under various conditions.

Also applies to: 181-184, 207-208

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1753973426-fix-ics-sequence-location-change

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Jul 31, 2025
Copy link

vercel bot commented Jul 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jul 31, 2025 5:56pm
cal-eu ⬜️ Ignored (Inspect) Jul 31, 2025 5:56pm

@joeauyeung joeauyeung marked this pull request as ready for review July 31, 2025 17:56
@graphite-app graphite-app bot requested a review from a team July 31, 2025 17:56
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Jul 31, 2025
Copy link

graphite-app bot commented Jul 31, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (07/31/25)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link
Contributor

E2E results are ready!

Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 lgtm

@emrysal emrysal merged commit f81bb92 into main Jul 31, 2025
82 of 85 checks passed
@emrysal emrysal deleted the devin/1753973426-fix-ics-sequence-location-change branch July 31, 2025 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants