fix(emails): increment iCalendar SEQUENCE on cancellation per RFC 5546 (#29706) - #29926
fix(emails): increment iCalendar SEQUENCE on cancellation per RFC 5546 (#29706)#29926Samarth1306w wants to merge 5 commits into
Conversation
…s in availability (calcom#29869)
|
Welcome to Cal.diy, @Samarth1306w! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThe change updates cancellation ICS generation to increment event sequences and verifies the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/lib/CalendarService.test.ts (1)
700-716: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an explicit
DECLINEDfixture.The fixture omits
PARTSTAT, so it tests only the defaultNEEDS-ACTIONpath. Add an attendee withPARTSTAT=DECLINEDand assert thatgetAvailabilityalso returns no busy interval.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/CalendarService.test.ts` around lines 700 - 716, Add an explicit CalDAV event fixture in the test beginning “skips CalDAV events…” with an attendee for user@example.com having PARTSTAT=DECLINED, then verify getAvailability returns no busy interval for that event as well as the existing NEEDS-ACTION case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/emails/lib/generateIcsString.test.ts`:
- Around line 96-97: Update the ICS test assertions around the existing
METHOD:CANCEL and SEQUENCE checks to validate exact lines rather than
substrings, preventing partial sequence matches. Add a test case that sets
event.iCalSequence to 2 and verify the generated ICS contains the exact line
SEQUENCE:3, while preserving coverage of the default sequence behavior.
In `@packages/lib/CalendarService.test.ts`:
- Around line 702-725: Update the test setup around getAvailability to provide
the user email through the TestCalendarService constructor instead of assigning
the private BaseCalendarService.credential member. Replace the remaining as any
casts by using typed CalDAV object and calendar fixture values, while preserving
the unanswered-invite scenario and existing availability assertions.
---
Nitpick comments:
In `@packages/lib/CalendarService.test.ts`:
- Around line 700-716: Add an explicit CalDAV event fixture in the test
beginning “skips CalDAV events…” with an attendee for user@example.com having
PARTSTAT=DECLINED, then verify getAvailability returns no busy interval for that
event as well as the existing NEEDS-ACTION case.
🪄 Autofix
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 Plus
Run ID: 6347034e-3aea-4ca0-881e-a3a8d2b47251
📒 Files selected for processing (6)
packages/emails/lib/generateIcsString.test.tspackages/emails/lib/generateIcsString.tspackages/lib/CalendarService.test.tspackages/lib/CalendarService.tspackages/lib/timezone.test.tspackages/lib/timezone.ts
| expect(assertedIcsString).toContain("METHOD:CANCEL"); | ||
| expect(assertedIcsString).toContain("SEQUENCE:1"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the ICS assertions exact and cover a non-zero sequence.
Lines 96-97 use substring matching. A value such as SEQUENCE:10 would satisfy the SEQUENCE:1 assertion. Replace both assertions with exact-line checks. Add a case with event.iCalSequence = 2 and assert SEQUENCE:3; the current test only covers the default sequence path.
Proposed assertion fix
- expect(assertedIcsString).toContain("METHOD:CANCEL");
- expect(assertedIcsString).toContain("SEQUENCE:1");
+ expect(assertedIcsString).toMatch(/(?:^|\r?\n)METHOD:CANCEL(?:\r?\n|$)/);
+ expect(assertedIcsString).toMatch(/(?:^|\r?\n)SEQUENCE:1(?:\r?\n|$)/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/emails/lib/generateIcsString.test.ts` around lines 96 - 97, Update
the ICS test assertions around the existing METHOD:CANCEL and SEQUENCE checks to
validate exact lines rather than substrings, preventing partial sequence
matches. Add a test case that sets event.iCalSequence to 2 and verify the
generated ICS contains the exact line SEQUENCE:3, while preserving coverage of
the default sequence behavior.
| service.credential.user = { email: "user@example.com" } as any; | ||
|
|
||
| const objects = [ | ||
| { | ||
| data: `BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| BEGIN:VEVENT | ||
| UID:unanswered-1 | ||
| SUMMARY:Unanswered Invite | ||
| DTSTART:20260729T100000Z | ||
| DTEND:20260729T110000Z | ||
| ORGANIZER;EMAIL=organizer@example.com:mailto:organizer@example.com | ||
| ATTENDEE;EMAIL=user@example.com;X-APPLE-NEEDS-REPLY=TRUE:mailto:user@example.com | ||
| END:VEVENT | ||
| END:VCALENDAR`, | ||
| }, | ||
| ]; | ||
|
|
||
| vi.mocked(fetchCalendarObjects).mockResolvedValue(objects as any); | ||
|
|
||
| const busy = await service.getAvailability({ | ||
| dateFrom: "2026-07-29T00:00:00Z", | ||
| dateTo: "2026-07-29T23:59:59Z", | ||
| selectedCalendars: [{ externalId: "cal1", integration: "caldav_calendar" }] as any, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the private member, the test access, and TypeScript test/typecheck configuration.
rg -n -C 3 'private credential|service\.credential|as any' \
packages/lib/CalendarService.ts \
packages/lib/CalendarService.test.ts
fd -HI -t f '^(tsconfig.*\.json|package\.json)$' . -x sh -c '
echo "=== $1 ==="
rg -n "include|exclude|typecheck|test|vitest" "$1" || true
' sh {}Repository: calcom/cal.diy
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== target file relevant lines ==="
sed -n '680,735p' packages/lib/CalendarService.test.ts
echo "=== BaseCalendarService credential area ==="
rg -n -C 5 'credential|constructor|class BaseCalendarService|class TestCalendarService' packages/lib/CalendarService.ts
echo "=== top-level tsconfig/package test/typecheck references (repo root only) ==="
fd -HI -t f '^(tsconfig.*\.json|package\.json)$' . \
-x sh -c 'echo "=== $1 ==="; rg -n "include|exclude|typecheck|test|vitest" "$1" || true' sh {} \
| sed -n '1,240p'
echo "=== deterministic TypeScript private-member behavior probe ==="
node - <<'JS'
console.log("skip runtime ts-node probe if unavailable")
JSRepository: calcom/cal.diy
Length of output: 23217
Configure the test credential through TestCalendarService.
BaseCalendarService.credential is private, so service.credential = ... as any is blocked by private-member access rules and the test will fail type-checking. Pass the user email through the test service constructor and use typed CalDAV object and calendar fixtures instead of the remaining as any casts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/CalendarService.test.ts` around lines 702 - 725, Update the test
setup around getAvailability to provide the user email through the
TestCalendarService constructor instead of assigning the private
BaseCalendarService.credential member. Replace the remaining as any casts by
using typed CalDAV object and calendar fixture values, while preserving the
unanswered-invite scenario and existing availability assertions.
Source: Coding guidelines
|
|
2 participants
Fixes #29706
Summary
Per RFC 5546 §3.2.5, when a cancellation iCalendar packet () is issued, the number MUST be incremented above the original event's sequence.
Without a bumped , recipient calendar clients (such as Outlook, Apple Calendar, and Thunderbird) treat the cancellation payload as an obsolete or duplicate revision and fail to remove the event from attendee calendars.
Updated to increment on cancellation ().