diff --git a/src/device-sync/__tests__/calendar/engine.test.ts b/src/device-sync/__tests__/calendar/engine.test.ts index 10418fd6..8e30e46b 100644 --- a/src/device-sync/__tests__/calendar/engine.test.ts +++ b/src/device-sync/__tests__/calendar/engine.test.ts @@ -379,6 +379,53 @@ describe('calendar device edits through the engine', () => { expect(h.server.all('CalendarEvent', 'team')).toEqual([]); }); + describe('an occurrence\'s own colour and reminders removed on the device', () => { + async function standup(extra: Record, override: Record) { + const h = real(); + const id = h.server.addEvent('a', { + uid: 'standup-uid', + title: 'Standup', + start: '2026-09-28T09:00:00', + duration: 'PT15M', + timeZone: 'Europe/Berlin', + recurrenceRule: { '@type': 'RecurrenceRule', frequency: 'daily', count: 5 }, + recurrenceOverrides: { '2026-09-29T09:00:00': { title: 'Standup (late)', ...override } }, + calendarIds: { [h.calendar]: true }, + ...extra, + }); + await h.run(CALENDAR_AUTHORITY); + const exception = () => h.events().find((e) => e[Events.ORIGINAL_SYNC_ID] === `a/${id}`)!; + const minutes = () => h.device.rows('reminders').filter((r) => Number(r[Reminders.EVENT_ID]) === Number(exception()._id)).map((r) => Number(r[Reminders.MINUTES])); + return { h, id, exception, minutes }; + } + + it('keeps a colour cleared on one occurrence cleared', async () => { + const { h, id, exception } = await standup({ color: 'steelblue' }, { color: 'red' }); + + h.device.user.updateEvent(Number(exception()._id), { [Events.EVENT_COLOR]: null }); + expect(await h.run(CALENDAR_AUTHORITY)).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { updated: 1 } } }); + + expect(exception()).toMatchObject({ [Events.EVENT_COLOR]: null, [Events.DIRTY]: 0 }); + expect((h.server.get('CalendarEvent', 'a', id)!.recurrenceOverrides as Overrides)['2026-09-29T09:00:00']).toMatchObject({ color: '' }); + }); + + it('puts back the removal of every reminder of an occurrence whose series shows its own, and reports it', async () => { + const alert = { '@type': 'Alert', trigger: { '@type': 'OffsetTrigger', offset: '-PT15M' }, action: 'display' }; + const { h, id, exception, minutes } = await standup({ alerts: { al1: alert } }, {}); + expect(minutes()).toEqual([15]); + const sets = () => h.server.requests.filter((r) => r.methods.includes('CalendarEvent/set')).length; + const before = sets(); + + h.device.user.setReminders(Number(exception()._id), []); + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report.itemErrors).toEqual([expect.objectContaining({ ref: `a/${id}`, side: 'upload', type: 'remindersNotRepresentable' })]); + expect(sets()).toBe(before); + expect(minutes()).toEqual([15]); + expect(exception()[Events.DIRTY]).toBe(0); + }); + }); + it('keeps a description and location cleared on one occurrence cleared', async () => { const h = real(); const id = h.server.addEvent('a', { diff --git a/src/device-sync/__tests__/calendar/exceptions.test.ts b/src/device-sync/__tests__/calendar/exceptions.test.ts index 2163d4c1..da473d36 100644 --- a/src/device-sync/__tests__/calendar/exceptions.test.ts +++ b/src/device-sync/__tests__/calendar/exceptions.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { Attendees, Events, Reminders } from '../../android-columns'; import { calendarPlanner } from '../../calendar/planner'; +import { cssColorToArgb } from '../../calendar/values'; import type { LocalEvent, UploadPlan } from '../../planner'; import type { CalendarEventWire } from '../../wire'; import { ACCT, Harness, ME, serverApply, single, utc, weekly } from './harness'; @@ -194,6 +195,80 @@ describe('calendar planner: recurrence edits made on the device', () => { expect(patch['recurrenceOverrides/2026-10-14T09:00:00']).toMatchObject({ useDefaultAlerts: false }); }); + it('clears an occurrence\'s own colour instead of letting the series\' come back', async () => { + const event = weekly(); + event.recurrenceOverrides!['2026-10-07T09:00:00'] = { ...event.recurrenceOverrides!['2026-10-07T09:00:00'], color: 'red' }; + const { h } = await synced(event); + const x = (await h.local('bl'))!.exceptions[0]; + expect(h.row(x.eventId)[Events.EVENT_COLOR]).toBe(cssColorToArgb('red')); + h.fake.user.updateEvent(x.eventId, { [Events.EVENT_COLOR]: null }); + const local = (await h.local('bl'))!; + + const plan = calendarPlanner.planUpload(local, h.ctx); + + // Without a colour of its own the occurrence would show the series' steelblue again: an empty one, which Stalwart keeps. + expect(patchOf(plan).patch).toEqual({ 'recurrenceOverrides/2026-10-07T09:00:00/color': '' }); + const server = await accept(h, event, local, plan); + expect(h.row(x.eventId)).toMatchObject({ [Events.EVENT_COLOR]: null, [Events.DIRTY]: 0 }); + expect(calendarPlanner.planDownload(server, (await h.local('bl'))!, h.ctx).effect).toBe('none'); + + // A series without a colour has none to show: the occurrence's own is removed. + const plain = weekly(); + delete plain.color; + plain.recurrenceOverrides!['2026-10-07T09:00:00'] = { ...plain.recurrenceOverrides!['2026-10-07T09:00:00'], color: 'red' }; + const p = await synced(plain); + const px = (await p.h.local('bl'))!.exceptions[0]; + p.h.fake.user.updateEvent(px.eventId, { [Events.EVENT_COLOR]: null }); + expect(patchOf(calendarPlanner.planUpload((await p.h.local('bl'))!, p.h.ctx)).patch).toEqual({ 'recurrenceOverrides/2026-10-07T09:00:00/color': null }); + }); + + it('removes every reminder of one occurrence of a series on the calendar\'s defaults', async () => { + const event = weekly(); + delete event.alerts; + event.useDefaultAlerts = true; + event.recurrenceOverrides!['2026-10-07T09:00:00'] = { + ...event.recurrenceOverrides!['2026-10-07T09:00:00'], + useDefaultAlerts: false, + alerts: { a60: { '@type': 'Alert', trigger: { '@type': 'OffsetTrigger', offset: '-PT1H', relativeTo: 'start' }, action: 'display' } }, + }; + const { h } = await synced(event); + const x = (await h.local('bl'))!.exceptions[0]; + h.fake.user.setReminders(x.eventId, []); + const local = (await h.local('bl'))!; + + const plan = calendarPlanner.planUpload(local, h.ctx); + const server = await accept(h, event, local, plan); + + // Stalwart drops the empty map, but the series has no alerts of its own to show instead. + expect(server.recurrenceOverrides!['2026-10-07T09:00:00']).toMatchObject({ useDefaultAlerts: false }); + expect(server.recurrenceOverrides!['2026-10-07T09:00:00']).not.toHaveProperty('alerts'); + expect(h.fake.rows('reminders', `${Reminders.EVENT_ID} = ?`, [x.eventId])).toEqual([]); + expect(h.row(x.eventId)[Events.DIRTY]).toBe(0); + expect(calendarPlanner.planDownload(server, (await h.local('bl'))!, h.ctx).effect).toBe('none'); + }); + + it('puts back and reports the removal of every reminder of one occurrence whose series would show its own again', async () => { + const event = weekly(); + event.alerts = { al1: { '@type': 'Alert', trigger: { '@type': 'OffsetTrigger', offset: '-PT15M', relativeTo: 'start' }, action: 'display' } }; + const { h } = await synced(event); + const minutes = (eventId: number) => h.fake.rows('reminders', `${Reminders.EVENT_ID} = ?`, [eventId]).map((r) => Number(r[Reminders.MINUTES])); + const x = (await h.local('bl'))!.exceptions[0]; + expect(minutes(x.eventId)).toEqual([15]); + h.fake.user.setReminders(x.eventId, []); + + // An override without alerts shows the series' (Stalwart writes no VALARM for an empty map): none can't be stored. + const plan = calendarPlanner.planUpload((await h.local('bl'))!, h.ctx); + expect(plan).toMatchObject({ kind: 'revert', reason: 'remindersNotRepresentable' }); + if (plan.kind === 'revert') await h.apply(plan.ops); + expect(minutes(x.eventId)).toEqual([15]); + expect(h.row(x.eventId)[Events.DIRTY]).toBe(0); + + // With another edit, that one goes up alone. + h.fake.user.setReminders(x.eventId, []); + h.fake.user.updateEvent(x.eventId, { [Events.TITLE]: 'Moved again' }); + expect(patchOf(calendarPlanner.planUpload((await h.local('bl'))!, h.ctx)).patch).toEqual({ 'recurrenceOverrides/2026-10-07T09:00:00/title': 'Moved again' }); + }); + it('patches an existing override one level deep, never deeper', async () => { const { h } = await synced(weekly()); const x = (await h.local('bl'))!.exceptions[0]; diff --git a/src/device-sync/__tests__/calendar/harness.ts b/src/device-sync/__tests__/calendar/harness.ts index 1ec0873c..b073e75b 100644 --- a/src/device-sync/__tests__/calendar/harness.ts +++ b/src/device-sync/__tests__/calendar/harness.ts @@ -169,6 +169,11 @@ export function serverApply(event: CalendarEventWire, patch: PatchObject): Calen for (const forbidden of ['@type', 'method', 'organizerCalendarAddress', 'privacy', 'prodId', 'recurrenceId', 'recurrenceIdTimeZone', 'sentBy', 'uid', 'recurrenceRule', 'recurrenceOverrides']) { delete (value as Record)[forbidden]; } + // An override is stored as its own VEVENT: an empty map writes nothing, so it reads back without it. + for (const map of ['participants', 'alerts', 'locations']) { + const inner = (value as Record)[map]; + if (inner && typeof inner === 'object' && !Object.keys(inner).length) delete (value as Record)[map]; + } } if (!Object.keys(overrides).length) delete next.recurrenceOverrides; } diff --git a/src/device-sync/calendar/planner.ts b/src/device-sync/calendar/planner.ts index 4ee3b26b..ac65d474 100644 --- a/src/device-sync/calendar/planner.ts +++ b/src/device-sync/calendar/planner.ts @@ -331,6 +331,11 @@ function planUpload(local: LocalEvent, ctx: CalendarContext): Plan { const ops = revertGroup(local, ctx); if (ops) return { kind: 'revert', ops, reason: SKIP.crossAccountMove }; } + // Every reminder of an occurrence removed where the server can't store none: back to what it holds, reported. + if (computation.dropped.includes('reminders')) { + const ops = revertGroup(local, ctx); + if (ops) return { kind: 'revert', ops, reason: SKIP.remindersNotRepresentable }; + } return { kind: 'clean', ops: cleanGroup(local, computation) }; } const action: UploadAction = { kind: 'update', id: ref.id, patch: computation.patch }; diff --git a/src/device-sync/calendar/upload.ts b/src/device-sync/calendar/upload.ts index f39210cc..73d63a73 100644 --- a/src/device-sync/calendar/upload.ts +++ b/src/device-sync/calendar/upload.ts @@ -50,6 +50,7 @@ import { } from './exceptions'; import { eventImage, plainInstanceImage } from './image'; import { isRemovedInstance } from './merge'; +import { reminderRows } from './reminders'; import { isRuleRepresentable, partsToRule, ruleToRRule, rruleParts, untilUtc, type RuleZone } from './rrule'; import { NOTIFYING_UNITS, sendsSchedulingMessages } from './scheduling'; import { fixedOffsetTiming, rowTiming, type RowTiming } from './timing'; @@ -90,6 +91,8 @@ export const SKIP = { rsvpRefused: 'rsvpRefused', /** Moved in place to a calendar of another JMAP account: no patch can do that. */ crossAccountMove: 'crossAccountMove', + /** Every reminder removed from one occurrence of a series with alerts of its own: the occurrence would show those. */ + remindersNotRepresentable: 'remindersNotRepresentable', } as const; export class SkipUpload extends Error { @@ -373,6 +376,7 @@ export function computeUpdate(local: LocalEvent, ctx: CalendarContext): UploadCo if (!ruleRemoved && shadow.recurrenceRule) { for (const x of local.exceptions) { const units = exceptionChanges(x, local, shadow, calendarId, zone, overrides, removedExceptions, ctx, lossy); + if (units.dropped) dropped.push(...units.dropped); if (units.units.size) { exceptionUnits.set(x.eventId, units.units); overridesChanged = true; @@ -423,7 +427,7 @@ function exceptionChanges( removed: Set, ctx: CalendarContext, lossy: boolean, -): { units: Set; rsvp: boolean; others?: boolean } { +): { units: Set; rsvp: boolean; others?: boolean; dropped?: string[] } { const units = new Set(); const key = x.recurrenceId; if (!key) return { units, rsvp: false }; @@ -478,7 +482,7 @@ function exceptionChanges( values.freeBusyStatus = availabilityFromDevice(cells[Events.AVAILABILITY]); break; case 'color': - values.color = argbToCss(cells[Events.EVENT_COLOR]); + values.color = argbToCss(cells[Events.EVENT_COLOR]) ?? clearedInOverride('color', instance, shadow); break; case 'timing': { const moved = columnUnitDiffers('timing', cur, reference); @@ -519,22 +523,30 @@ function exceptionChanges( units.add('attendees'); } } + let dropped: string[] | undefined; if (ctx.reminderOwner === 'device' && (existing || cur.reminders!.length) && remindersDiffer(cur, reference)) { - values.alerts = reminderChanges(instance, ctx.calendar(calendarId), cur.reminders!, ctx.mintKey).alerts; - // An occurrence on the calendar's defaults ignores its own alerts: it leaves them, as a series does - // (Stalwart keeps the override's `useDefaultAlerts` as a JSPROP; the series stays on the defaults). - if (instance.useDefaultAlerts) values.useDefaultAlerts = false; - units.add('reminders'); + const alerts = reminderChanges(instance, ctx.calendar(calendarId), cur.reminders!, ctx.mintKey).alerts; + if (!Object.keys(alerts).length && reminderRows(shadow.alerts).length) { + // An override is stored as a VEVENT of its own and an empty map writes no VALARM, so an override without + // alerts shows the series' again: none at all can't be stored while the series has alerts of its own. + dropped = ['reminders']; + } else { + values.alerts = alerts; + // An occurrence on the calendar's defaults ignores its own alerts: it leaves them, as a series does + // (Stalwart keeps the override's `useDefaultAlerts` as a JSPROP; the series stays on the defaults). + if (instance.useDefaultAlerts) values.useDefaultAlerts = false; + units.add('reminders'); + } } if (existing) { if (units.size) work.props.set(key, { ...(work.props.get(key) ?? {}), ...values }); - return { units, rsvp, others }; + return { units, rsvp, others, dropped }; } // A new override: nothing but its pinned time is still an override (an app saving the instance unchanged). if (!units.size) units.add('new'); work.whole.set(key, newOverride(shadow, key, values)); - return { units, rsvp, others }; + return { units, rsvp, others, dropped }; } /** Units a new exception row inherits from its series when it leaves them empty. */ @@ -547,15 +559,17 @@ function isEmptyUnit(unit: string, cells: Row): boolean { } /** - * A description or location cleared on an existing override. An override - * without one of its own shows the series' again, so while the series has - * one the override gets the empty value Stalwart keeps: the empty text, or a - * location without a name (an empty map writes no LOCATION line and reads - * back as none of its own). Null, a clean removal, when nothing would come - * back. + * A description, location or colour cleared on an existing override. An + * override without one of its own shows the series' again, so while the + * series has one the override gets the empty value Stalwart keeps: the empty + * text or colour (an empty DESCRIPTION or COLOR; an empty colour is none, the + * calendar's, on the device), or a location without a name (an empty map + * writes no LOCATION line and reads back as none of its own). Null, a clean + * removal, when nothing would come back. */ -function clearedInOverride(unit: 'description' | 'location', instance: CalendarEventWire, master: CalendarEventWire): unknown { +function clearedInOverride(unit: 'description' | 'location' | 'color', instance: CalendarEventWire, master: CalendarEventWire): unknown { if (unit === 'description') return typeof master.description === 'string' && master.description ? '' : null; + if (unit === 'color') return typeof master.color === 'string' && master.color ? '' : null; const series = Object.keys(master.locations ?? {}); if (!series.length) return null; const key = Object.keys(instance.locations ?? {})[0] ?? series[0];