mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: keep an occurrence's colour cleared, and report reminders that can't go #34
Clearing the colour of one occurrence of a coloured series removed the occurrence's own colour, so it showed the series' colour again after the sync. The occurrence now gets an empty colour, which Stalwart keeps and which shows as the calendar's colour, as on the phone. Removing every reminder of one occurrence sent an empty set of alerts, which Stalwart does not store: the occurrence then showed the series' reminders again, without a word. When the series uses the calendar's default reminders, the occurrence leaves the defaults and has none, as before. When the series has reminders of its own, the occurrence would show them whatever is sent, so the change is put back and the report says why (`remindersNotRepresentable`). Checked against calcard 0.3.14, with planner tests and runs through the engine and the fake server.
This commit is contained in:
@@ -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<string, unknown>, override: Record<string, unknown>) {
|
||||
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', {
|
||||
|
||||
@@ -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];
|
||||
|
||||
@@ -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<string, unknown>)[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<string, unknown>)[map];
|
||||
if (inner && typeof inner === 'object' && !Object.keys(inner).length) delete (value as Record<string, unknown>)[map];
|
||||
}
|
||||
}
|
||||
if (!Object.keys(overrides).length) delete next.recurrenceOverrides;
|
||||
}
|
||||
|
||||
@@ -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<CalendarEventWire> = { kind: 'update', id: ref.id, patch: computation.patch };
|
||||
|
||||
@@ -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<number>,
|
||||
ctx: CalendarContext,
|
||||
lossy: boolean,
|
||||
): { units: Set<string>; rsvp: boolean; others?: boolean } {
|
||||
): { units: Set<string>; rsvp: boolean; others?: boolean; dropped?: string[] } {
|
||||
const units = new Set<string>();
|
||||
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];
|
||||
|
||||
Reference in New Issue
Block a user