From 5168920eb1d3c5b57a814868f9f2198f3b128a48 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Tue, 9 Jun 2026 10:59:19 -0400 Subject: [PATCH] fix(03): CR-01 preserve RRULE on edit-as-move (forward source rule to create row) --- apps/api/src/broker/outboxWorker.ts | 19 +++++++- apps/api/src/routes/events.ts | 23 ++++++++- apps/api/tests/broker/outboxWorker.test.ts | 52 +++++++++++++++++++++ apps/api/tests/routes/events.test.ts | 54 ++++++++++++++++++++++ 4 files changed, 146 insertions(+), 2 deletions(-) diff --git a/apps/api/src/broker/outboxWorker.ts b/apps/api/src/broker/outboxWorker.ts index ab9af4f..1d9a179 100644 --- a/apps/api/src/broker/outboxWorker.ts +++ b/apps/api/src/broker/outboxWorker.ts @@ -282,6 +282,23 @@ async function dispatchRow(row: OutboxRow): Promise { } catch { return { success: false, conflict: false, hardFail: true, transient: false, error: 'payload parse failed' } } + // CR-01: edit-as-move RRULE preservation. The same-calendar `update` branch + // preserves a recurring series' RRULE by reading rawVevent; the `create` branch + // (used for the create half of an edit-as-move, D-04) has no source for the + // original RRULE because it writes under a brand-new uid. The edit route extracts + // the source event's RRULE and stashes it on the payload as `_preservedRrule` so + // the worker can re-apply it here. An explicit `recurrence` on the payload still + // wins (deliberate user change); the preserved RRULE only fills the gap when the + // edit omitted recurrence — matching the update-branch semantics and the WR-01 fix. + const hasExplicitRecurrence = Object.prototype.hasOwnProperty.call(fields, 'recurrence') + const rruleFromPayload = + fields.recurrence && fields.recurrence !== 'none' + ? RRULE_PRESETS[fields.recurrence as string] + : undefined + const preservedRrule = + typeof fields._preservedRrule === 'string' && fields._preservedRrule.length > 0 + ? fields._preservedRrule + : undefined const { icsString } = buildVeventString({ uid: row.uid, summary: fields.title as string, @@ -290,7 +307,7 @@ async function dispatchRow(row: OutboxRow): Promise { dtend: fields.allDay ? (fields.end as string) : new Date(fields.end as string), location: fields.location as string | undefined, description: fields.description as string | undefined, - rruleString: fields.recurrence && fields.recurrence !== 'none' ? RRULE_PRESETS[fields.recurrence as string] : undefined, + rruleString: hasExplicitRecurrence ? rruleFromPayload : (preservedRrule ?? rruleFromPayload), }) // Build a minimal DAVCalendar for the write wrapper (only url is needed) const davCalendar = { url: row.calendarUrl } as Parameters[1] diff --git a/apps/api/src/routes/events.ts b/apps/api/src/routes/events.ts index 0920705..5d8184d 100644 --- a/apps/api/src/routes/events.ts +++ b/apps/api/src/routes/events.ts @@ -31,6 +31,7 @@ import { sql } from 'drizzle-orm' import { db } from '../db/client.js' import { calendarEvents, calendars, users, calendarOutbox } from '../db/schema.js' import { expandOccurrences } from '../broker/expand.js' +import { extractRruleString } from '../broker/vevent.js' import { getAuth } from '../auth/middleware.js' import { upsertUser, deriveDisplayName } from '../auth/user.js' // Side-effect import: brings in the ContextVariableMap augmentation for c.get('user') @@ -334,6 +335,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c calendarId: calendarEvents.calendarId, calendarUrl: calendars.url, userId: calendars.userId, + rawVevent: calendarEvents.rawVevent, }) .from(calendarEvents) .innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id)) @@ -373,6 +375,25 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c const newUid = `${randomUUID()}@familysync` const groupId = randomUUID() + // CR-01: carry the existing RRULE through the move. The edit payload omits + // `recurrence` (the occurrence contract does not expose it, D-03), and the + // create lands under a brand-new uid that the worker can never look up the + // original RRULE from. Unlike the same-calendar `update` branch — which reads + // rawVevent and re-applies the stored RRULE — the create branch has no source + // for it. Extract the RRULE from the source event here and stash it on the + // create payload so the worker re-applies it, preventing a recurring series + // from silently collapsing into a single occurrence on a calendar move. + // Only stash when the edit did NOT carry an explicit recurrence: an explicit + // value (including 'none') is a deliberate user change and must win. + const preservedRrule = + payload.recurrence === undefined + ? extractRruleString(eventRow.rawVevent ?? '') + : undefined + const createPayload = + preservedRrule !== undefined + ? { ...payload, _preservedRrule: preservedRrule } + : payload + await db.transaction(async (tx) => { // Delete from old calendar await tx.insert(calendarOutbox).values({ @@ -392,7 +413,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c status: 'pending', uid: newUid, calendarUrl: newCalendarUrl, - payload: JSON.stringify(payload), + payload: JSON.stringify(createPayload), groupId, }) }) diff --git a/apps/api/tests/broker/outboxWorker.test.ts b/apps/api/tests/broker/outboxWorker.test.ts index ca82e51..d47d000 100644 --- a/apps/api/tests/broker/outboxWorker.test.ts +++ b/apps/api/tests/broker/outboxWorker.test.ts @@ -281,6 +281,58 @@ describe('runOutboxDrain — ICS building from form JSON (CR-02)', () => { const setArg = mockUpdateSet.mock.calls[0]?.[0] as { status?: string } expect(setArg?.status).toBe('failed') }) + + // CR-01 (iteration 2): the edit-as-move create branch must re-apply the RRULE the + // route stashed on the payload as `_preservedRrule`, so a moved recurring series keeps + // its RRULE instead of collapsing into a single occurrence. + it('CR-01: create row re-applies _preservedRrule → emitted ICS contains RRULE:', async () => { + const { createCalendarEvent } = await import('../../src/broker/write.js') + let capturedIcsString: unknown = null + vi.mocked(createCalendarEvent).mockImplementation(async (_client, _cal, _uid, icsString) => { + capturedIcsString = icsString + return makeResponse(201) + }) + // Move payload: no explicit `recurrence`, but the route stashed the source RRULE. + const movePayload = JSON.stringify({ + title: 'Moved weekly standup', + allDay: false, + start: '2026-06-10T12:00:00', + end: '2026-06-10T13:00:00', + _preservedRrule: 'FREQ=WEEKLY;BYDAY=MO', + }) + mockPendingRows = [makeRow({ operation: 'create', payload: movePayload, groupId: 'move-grp-1' })] + + await runOutboxDrain() + + expect(typeof capturedIcsString).toBe('string') + expect(capturedIcsString as string).toContain('RRULE:') + expect(capturedIcsString as string).toContain('FREQ=WEEKLY') + }) + + // CR-01 corollary: an explicit `recurrence` on a create still wins over any preserved + // RRULE (deliberate user choice); recurrence:'none' must emit no RRULE. + it("CR-01: explicit recurrence:'none' wins → emitted ICS has no RRULE even if _preservedRrule present", async () => { + const { createCalendarEvent } = await import('../../src/broker/write.js') + let capturedIcsString: unknown = null + vi.mocked(createCalendarEvent).mockImplementation(async (_client, _cal, _uid, icsString) => { + capturedIcsString = icsString + return makeResponse(201) + }) + const payload = JSON.stringify({ + title: 'One-off', + allDay: false, + start: '2026-06-10T12:00:00', + end: '2026-06-10T13:00:00', + recurrence: 'none', + _preservedRrule: 'FREQ=WEEKLY;BYDAY=MO', + }) + mockPendingRows = [makeRow({ operation: 'create', payload })] + + await runOutboxDrain() + + expect(typeof capturedIcsString).toBe('string') + expect(capturedIcsString as string).not.toContain('RRULE:') + }) }) describe('runOutboxDrain — edit-as-move ordering (D-04)', () => { diff --git a/apps/api/tests/routes/events.test.ts b/apps/api/tests/routes/events.test.ts index 1d64e3f..ea89c44 100644 --- a/apps/api/tests/routes/events.test.ts +++ b/apps/api/tests/routes/events.test.ts @@ -413,6 +413,60 @@ describe('PATCH /api/events/:uid/edit', () => { }) expect(res.status).toBe(202) }) + + // CR-01 (iteration 2): edit-as-move must carry the source event's RRULE through + // to the create row so a moved recurring series does not silently collapse into a + // single occurrence. The edit payload omits `recurrence`; the route must extract the + // RRULE from the source event's rawVevent and stash it on the create payload as + // `_preservedRrule` so the worker re-applies it. + it('CR-01: move of a recurring event stashes the source RRULE on the create outbox row', async () => { + // Seed a recurring source event (RRULE:FREQ=WEEKLY;BYDAY=MO) on the user's calendar. + mockDbRows = [ + { + uid: 'uid-001@familysync', + etag: '"etag-abc"', + objectUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/uid-001.ics', + calendarId: 1, + calendarUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/', + userId: 1, + rawVevent: SAMPLE_VEVENT_RECURRING_TIMED, + }, + ] + const mockInnerJoinWhere = vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockImplementation(() => Promise.resolve(mockDbRows)), + }), + }) + const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere }) + mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin }) + mockSelectFn.mockReturnValue({ from: mockFromFn }) + + const { app } = await import('../../src/index.js') + const res = await app.request('/api/events/uid-001%40familysync/edit', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + title: 'Moved event', + allDay: false, + start: '2026-06-15T10:00:00Z', + end: '2026-06-15T11:00:00Z', + // Different calendarUrl than the source → triggers the edit-as-move path (D-04) + calendarUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Family/', + }), + }) + expect(res.status).toBe(202) + + // The move inserts a delete+create pair inside a transaction. Find the create row's + // payload and assert the preserved RRULE survived. + const createInsertCall = mockInsertValuesFn.mock.calls.find((call) => { + const arg = call[0] as { operation?: string; payload?: string } + return arg?.operation === 'create' + }) + expect(createInsertCall).toBeDefined() + const createArg = createInsertCall![0] as { payload: string } + const parsed = JSON.parse(createArg.payload) as { _preservedRrule?: string } + expect(parsed._preservedRrule).toBe('FREQ=WEEKLY;BYDAY=MO') + }) }) // ---------------------------------------------------------------------------