From 0110eebb7802d188292be5a87c60a1888a57ca26 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 27 Sep 2026 09:54:03 +0200 Subject: [PATCH] fix: upload an edited event another client moved out of synced calendars #34 When another client moved an event into a calendar this device does not sync while the device had an unsent edit of it, the calendar planner refused to merge the server's version, since the event is in no synced calendar any more. The edit then waited on every run as a planner error, and the event never left the device. The server's version now merges into the rows where they are, so the edit uploads against it without undoing the other client's changes, and the rows go once the server has taken the edit. Checked with a planner test and a run through the engine and the fake server: the series rescheduled on the device keeps the occurrence the other client added, and no rows are left. --- .../__tests__/calendar/download.test.ts | 22 +++++++++++++++++++ .../__tests__/engine/real-planners.test.ts | 8 +++---- src/device-sync/calendar/planner.ts | 17 +++++++++----- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/device-sync/__tests__/calendar/download.test.ts b/src/device-sync/__tests__/calendar/download.test.ts index ab00eea5..b4884312 100644 --- a/src/device-sync/__tests__/calendar/download.test.ts +++ b/src/device-sync/__tests__/calendar/download.test.ts @@ -235,6 +235,28 @@ describe('calendar planner: downloads', () => { expect(() => calendarPlanner.planDownload({ ...single(), '@type': undefined, due: '2026-10-01T00:00:00' } as unknown as CalendarEventWire, null, h.ctx)).toThrow(/tasks/); }); + it('merges an edited event another client moved out of every synced calendar where its rows are', async () => { + const h = await new Harness().setup(); + await h.download(single()); + const id = Number(h.masterRow('e1')._id); + h.fake.user.updateEvent(id, { [Events.TITLE]: 'Team lunch' }); + // Filed in a calendar this device does not sync, with a new description. + const moved = single({ calendarIds: { archive: true }, description: 'Bring salad' }); + + const plan = calendarPlanner.planDownload(moved, await h.local('e1'), h.ctx); + await h.apply(plan.ops); + + expect(plan).toMatchObject({ effect: 'update', stillDirty: true, conflicts: 0 }); + expect(h.masterRow('e1')).toMatchObject({ [Events.CALENDAR_ID]: h.rowIds.get('b'), [Events.TITLE]: 'Team lunch', [Events.DESCRIPTION]: 'Bring salad', [Events.DIRTY]: 1 }); + const local = (await h.local('e1'))!; + expect(local.shadow).toEqual(moved); + // Only the device's edit goes up (no calendar move), and the accepted version takes the rows away. + const plan2 = calendarPlanner.planUpload(local, h.ctx); + expect(plan2.kind === 'upload' && plan2.actions).toEqual([{ kind: 'update', id: 'e1', patch: { title: 'Team lunch' } }]); + await h.apply(calendarPlanner.planAccepted(local, { ...moved, title: 'Team lunch' }, h.ctx).ops); + expect(await h.events()).toEqual([]); + }); + it('places an event in several calendars in the first selected one, and keeps it where it is', async () => { const h = await new Harness().setup(); await h.download(single({ calendarIds: { w: true, b: true } })); diff --git a/src/device-sync/__tests__/engine/real-planners.test.ts b/src/device-sync/__tests__/engine/real-planners.test.ts index cae21f0c..1f95bb6c 100644 --- a/src/device-sync/__tests__/engine/real-planners.test.ts +++ b/src/device-sync/__tests__/engine/real-planners.test.ts @@ -574,14 +574,14 @@ describe('device sync engine with the real planners', () => { 'recurrenceOverrides/2026-10-01T09:00:00': { title: 'Retro' }, }); - expect((await h.run(CALENDAR_AUTHORITY)).outcome).toBe('ok'); + expect(await h.run(CALENDAR_AUTHORITY)).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { updated: 1 } } }); const event = h.server.get('CalendarEvent', 'a', series)!; const titles = Object.values((event.recurrenceOverrides ?? {}) as Record).map((o) => o.title).sort(); expect(titles).toEqual(['Planning', 'Retro']); - // The move went up, or it still waits on the device: it is not lost. - const row = h.events().find((e) => Number(e._id) === Number(master._id)); - expect(event.start === '2026-09-28T10:00:00' || (Number(row?.dirty) === 1 && Number(row?.[Events.DTSTART]) === later)).toBe(true); + // The move went up against the server's version, and the rows went with the event. + expect(event).toMatchObject({ start: '2026-09-28T10:00:00', calendarIds: { [archive]: true } }); + expect(h.events()).toEqual([]); }); }); diff --git a/src/device-sync/calendar/planner.ts b/src/device-sync/calendar/planner.ts index 8348e6ef..5db04219 100644 --- a/src/device-sync/calendar/planner.ts +++ b/src/device-sync/calendar/planner.ts @@ -31,7 +31,7 @@ import { claimOps, claimUid, claimedUid, computeCreate, createTarget, deleteActi import { decodeCalendar, decodeEvents, isExceptionRow, isNewMaster } from './decode'; import { isExcluded, overridesOf } from './exceptions'; import { eventImage } from './image'; -import { insertEvent, isRemovedInstance, linkedExceptionCount, pickCalendar, writeEvent } from './merge'; +import { insertEvent, isRemovedInstance, linkedExceptionCount, pickCalendar, writeEvent, type CalendarTarget } from './merge'; import { pairKind, pairOps, pairPatch } from './pairs'; import { GroupBuilder, assertExceptionCount, assertRow, deleteRow, writesOf } from './rows'; import { eventZone } from './timing'; @@ -52,6 +52,12 @@ function refOf(local: LocalEventRow | null, event?: CalendarEventWire, jmapAccou return local ? `row:${local.eventId}` : 'new'; } +/** The calendar a row is in, when it is one of ours. */ +function rowCalendar(local: LocalEventRow, ctx: CalendarContext): CalendarTarget | null { + const cal = ctx.calendarIdOfRow(local.calendarRowId); + return cal ? { calendarRowId: local.calendarRowId, calendarId: cal.calendarId } : null; +} + // ─── Downloads ────────────────────────────────────────── function planDownload(event: CalendarEventWire, local: LocalEvent | null, ctx: CalendarContext): DownloadPlan { @@ -59,7 +65,9 @@ function planDownload(event: CalendarEventWire, local: LocalEvent | null, ctx: C refuseTask(local?.shadow); const ref = refOf(local, event, ctx.jmapAccountId); const none = (stillDirty: boolean): DownloadPlan => ({ ops: { ref, ops: [] }, conflicts: 0, stillDirty, effect: 'none', writes: 0 }); - const target = pickCalendar(event, local, ctx); + // An event another client moved out of every synced calendar still merges into its rows where they are + // (they wait for an upload; the accepted write then removes them). + const target = pickCalendar(event, local, ctx) ?? (local ? rowCalendar(local, ctx) : null); if (!target) throw new Error(`calendar planner: ${ref} is in no synced calendar (the engine removes such rows)`); const image = eventImage(event, target.calendarRowId, target.calendarId, ctx); // Stalwart drops a start it can't resolve (a DST gap or overlap): nothing to show. @@ -210,10 +218,7 @@ function purgeGroup(local: LocalEvent): OpGroup { function revertGroup(local: LocalEvent, ctx: CalendarContext): OpGroup | null { const shadow = local.shadow; if (!shadow) return null; - const target = pickCalendar(shadow, local, ctx) ?? (() => { - const cal = ctx.calendarIdOfRow(local.calendarRowId); - return cal ? { calendarRowId: local.calendarRowId, calendarId: cal.calendarId } : null; - })(); + const target = pickCalendar(shadow, local, ctx) ?? rowCalendar(local, ctx); if (!target) return null; const image = eventImage(shadow, target.calendarRowId, target.calendarId, ctx); if (!image) return null;