fix(11-05): CR-01 surface reminderIsCustom to preserve custom VALARMs on edit
- expand.ts: add reminderIsCustom:boolean to CalendarOccurrence interface; derived from classifyValarms kind==='custom'; propagated to both non-recurring and recurring occurrence branches - client.ts: mirror reminderIsCustom on CalendarOccurrence (atomic mirror) - EventForm.tsx: extend deriveReminderValue to accept isCustom flag; returns '__custom__' when true, making the existing D-08 preserve branch live — editing a custom-alarm event now omits reminderLeadMinutes from the payload so outboxWorker extractValarms keeps the original VALARM - Fix existing test fixtures (EventForm.test.tsx, EventDetailPopover.test.tsx) to include reminderIsCustom:false on all CalendarOccurrence literals Fixes CAL-14 Pitfall 1: Apple Calendar absolute DATE-TIME / multi-VALARM alarms no longer silently stripped on any edit round-trip from the PWA.
This commit is contained in:
@@ -80,6 +80,13 @@ export interface CalendarOccurrence {
|
|||||||
* positive — N minutes before event start (timed) or N/1440 days before (all-day)
|
* positive — N minutes before event start (timed) or N/1440 days before (all-day)
|
||||||
*/
|
*/
|
||||||
reminderLeadMinutes: number | null;
|
reminderLeadMinutes: number | null;
|
||||||
|
/**
|
||||||
|
* True when the master event's alarm is a custom/absolute/multi-VALARM not reducible
|
||||||
|
* to a single before-event lead (CAL-14, CR-01, Phase 11 Plan 05).
|
||||||
|
* When true, reminderLeadMinutes is always null and the edit form must initialize the
|
||||||
|
* picker to '__custom__' to emit an absent payload field and preserve the original VALARM.
|
||||||
|
*/
|
||||||
|
reminderIsCustom: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -241,9 +248,14 @@ export function expandOccurrences(
|
|||||||
// event's VALARM via classifyValarms. All occurrences inherit this value (series-level, D-10).
|
// event's VALARM via classifyValarms. All occurrences inherit this value (series-level, D-10).
|
||||||
// preset/offlist → specific leadMinutes; custom/none → null (D-07/NOTIF-05).
|
// preset/offlist → specific leadMinutes; custom/none → null (D-07/NOTIF-05).
|
||||||
// classifyValarms wraps ICAL.parse in try/catch (T-11-07 safe); safe on parse failure → null.
|
// classifyValarms wraps ICAL.parse in try/catch (T-11-07 safe); safe on parse failure → null.
|
||||||
|
//
|
||||||
|
// Phase 11 Plan 05 (CR-01): reminderIsCustom — surface the 'custom' classification so the
|
||||||
|
// edit form can initialize the picker to '__custom__' and emit an absent payload field,
|
||||||
|
// preserving the original VALARM via the outboxWorker D-08 preserve path.
|
||||||
const alarmClass = classifyValarms(rawVevent);
|
const alarmClass = classifyValarms(rawVevent);
|
||||||
const reminderLeadMinutes: number | null =
|
const reminderLeadMinutes: number | null =
|
||||||
alarmClass.kind === 'preset' || alarmClass.kind === 'offlist' ? alarmClass.leadMinutes : null;
|
alarmClass.kind === 'preset' || alarmClass.kind === 'offlist' ? alarmClass.leadMinutes : null;
|
||||||
|
const reminderIsCustom: boolean = alarmClass.kind === 'custom';
|
||||||
|
|
||||||
// --- 4. Non-recurring event: single occurrence check ---
|
// --- 4. Non-recurring event: single occurrence check ---
|
||||||
if (!isRecurring) {
|
if (!isRecurring) {
|
||||||
@@ -281,6 +293,7 @@ export function expandOccurrences(
|
|||||||
description: event.description ?? null,
|
description: event.description ?? null,
|
||||||
hasRrule: isRecurring, // always false in the non-recurring branch
|
hasRrule: isRecurring, // always false in the non-recurring branch
|
||||||
reminderLeadMinutes, // series-level (D-10)
|
reminderLeadMinutes, // series-level (D-10)
|
||||||
|
reminderIsCustom, // CR-01 (Plan 05): true when alarm is absolute/multi-VALARM
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
return occurrences;
|
return occurrences;
|
||||||
@@ -329,6 +342,7 @@ export function expandOccurrences(
|
|||||||
description: event.description ?? null,
|
description: event.description ?? null,
|
||||||
hasRrule: isRecurring, // always true in the recurring branch
|
hasRrule: isRecurring, // always true in the recurring branch
|
||||||
reminderLeadMinutes, // series-level — all occurrences inherit the master's value (D-10)
|
reminderLeadMinutes, // series-level — all occurrences inherit the master's value (D-10)
|
||||||
|
reminderIsCustom, // CR-01 (Plan 05): true when alarm is absolute/multi-VALARM
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -139,6 +139,14 @@ export interface CalendarOccurrence {
|
|||||||
* (atomic mirror, Plan 11-03).
|
* (atomic mirror, Plan 11-03).
|
||||||
*/
|
*/
|
||||||
reminderLeadMinutes: number | null;
|
reminderLeadMinutes: number | null;
|
||||||
|
/**
|
||||||
|
* True when the event's alarm is custom/absolute/multi-VALARM (not reducible to a
|
||||||
|
* single before-event lead). When true, reminderLeadMinutes is always null and the
|
||||||
|
* form must initialize to '__custom__' to preserve the VALARM on edit (CR-01, Plan 11-05).
|
||||||
|
* Mirrors CalendarOccurrence.reminderIsCustom in apps/api/src/broker/expand.ts
|
||||||
|
* (atomic mirror, Plan 11-05).
|
||||||
|
*/
|
||||||
|
reminderIsCustom: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface OccurrencesResponse {
|
export interface OccurrencesResponse {
|
||||||
|
|||||||
@@ -60,6 +60,7 @@ const TIMED_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: 'Daily team sync meeting',
|
description: 'Daily team sync meeting',
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: null,
|
reminderLeadMinutes: null,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
const OCCURRENCE_WITH_HTML: CalendarOccurrence = {
|
const OCCURRENCE_WITH_HTML: CalendarOccurrence = {
|
||||||
@@ -88,6 +89,7 @@ const ALLDAY_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: null,
|
description: null,
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: null,
|
reminderLeadMinutes: null,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
// ── Import component (after mocks are declared) ───────────────────────────────
|
// ── Import component (after mocks are declared) ───────────────────────────────
|
||||||
|
|||||||
@@ -125,6 +125,7 @@ const EDIT_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: 'Weekly sync',
|
description: 'Weekly sync',
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: null,
|
reminderLeadMinutes: null,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
// ── Import component (after mocks) ────────────────────────────────────────────
|
// ── Import component (after mocks) ────────────────────────────────────────────
|
||||||
@@ -511,6 +512,7 @@ const RECURRING_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: null,
|
description: null,
|
||||||
hasRrule: true,
|
hasRrule: true,
|
||||||
reminderLeadMinutes: null,
|
reminderLeadMinutes: null,
|
||||||
|
reminderIsCustom: false,
|
||||||
// @ts-expect-error — recurrence is not on CalendarOccurrence type yet; the reset
|
// @ts-expect-error — recurrence is not on CalendarOccurrence type yet; the reset
|
||||||
// effect reads it if present and defaults to 'none' when absent (WR-03, v1 comment)
|
// effect reads it if present and defaults to 'none' when absent (WR-03, v1 comment)
|
||||||
recurrence: 'weekly',
|
recurrence: 'weekly',
|
||||||
@@ -538,6 +540,7 @@ const LATE_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: null,
|
description: null,
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: null,
|
reminderLeadMinutes: null,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
describe('EventForm — Plan 03-12 gap closures', () => {
|
describe('EventForm — Plan 03-12 gap closures', () => {
|
||||||
@@ -786,6 +789,7 @@ const ALL_DAY_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: null,
|
description: null,
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: null,
|
reminderLeadMinutes: null,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
describe('EventForm — Plan 06-06 end-tracking + recurrence-bound', () => {
|
describe('EventForm — Plan 06-06 end-tracking + recurrence-bound', () => {
|
||||||
@@ -1007,6 +1011,7 @@ const TIMED_REMINDER_OCCURRENCE: CalendarOccurrence = {
|
|||||||
title: 'Meeting with reminder',
|
title: 'Meeting with reminder',
|
||||||
start: '2026-06-15T10:00:00-04:00',
|
start: '2026-06-15T10:00:00-04:00',
|
||||||
end: '2026-06-15T11:00:00-04:00',
|
end: '2026-06-15T11:00:00-04:00',
|
||||||
|
reminderIsCustom: false,
|
||||||
allDay: false,
|
allDay: false,
|
||||||
location: null,
|
location: null,
|
||||||
description: null,
|
description: null,
|
||||||
@@ -1034,6 +1039,7 @@ const ALLDAY_REMINDER_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: null,
|
description: null,
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: 1440,
|
reminderLeadMinutes: 1440,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -1056,6 +1062,7 @@ const OFFLIST_REMINDER_OCCURRENCE: CalendarOccurrence = {
|
|||||||
description: null,
|
description: null,
|
||||||
hasRrule: false,
|
hasRrule: false,
|
||||||
reminderLeadMinutes: 45,
|
reminderLeadMinutes: 45,
|
||||||
|
reminderIsCustom: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
describe('EventForm — Phase 11 reminder picker (Plan 04)', () => {
|
describe('EventForm — Phase 11 reminder picker (Plan 04)', () => {
|
||||||
@@ -1315,10 +1322,8 @@ describe('EventForm — Phase 11 Plan 05 CR-01: custom alarm round-trip', () =>
|
|||||||
const callPayload = mockUpdateEvent.mock.calls[0][1] as Record<string, unknown>;
|
const callPayload = mockUpdateEvent.mock.calls[0][1] as Record<string, unknown>;
|
||||||
// MUST be absent: the presence of reminderLeadMinutes:null would cause the outbox
|
// MUST be absent: the presence of reminderLeadMinutes:null would cause the outbox
|
||||||
// worker to clear the VALARM — the CR-01 data-loss bug.
|
// worker to clear the VALARM — the CR-01 data-loss bug.
|
||||||
expect(Object.prototype.hasOwnProperty.call(callPayload, 'reminderLeadMinutes')).toBe(
|
// D-08: field must be absent — presence of reminderLeadMinutes:null clears the VALARM (CR-01)
|
||||||
false,
|
expect(Object.prototype.hasOwnProperty.call(callPayload, 'reminderLeadMinutes')).toBe(false);
|
||||||
'reminderLeadMinutes must be absent from payload when alarm is custom (D-08 preserve path)',
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -75,12 +75,22 @@ function humanizeReminderLead(minutes: number): string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Derive the initial reminder picker value from an occurrence's reminderLeadMinutes.
|
* Derive the initial reminder picker value from an occurrence's reminder fields.
|
||||||
* Returns '__none__' for null, the matching preset string for a preset, or the numeric
|
*
|
||||||
* string for an off-list value (synthetic option will be rendered for this case).
|
* CR-01 (Plan 11-05): When reminderIsCustom is true, the event carries an absolute
|
||||||
* There is no '__custom__' path here — the occurrence only carries number|null.
|
* DATE-TIME trigger or multiple VALARMs that cannot be reduced to a single lead.
|
||||||
|
* Returning '__custom__' makes the existing preserve branch live: the form emits an
|
||||||
|
* absent reminderLeadMinutes field, and the outboxWorker's D-08 preserve path keeps
|
||||||
|
* the original VALARM intact (no silent data loss on edit).
|
||||||
|
*
|
||||||
|
* Precedence: custom → '__custom__'; null → '__none__'; else preset/off-list string.
|
||||||
*/
|
*/
|
||||||
function deriveReminderValue(leadMinutes: number | null, isAllDay: boolean): string {
|
function deriveReminderValue(
|
||||||
|
leadMinutes: number | null,
|
||||||
|
isAllDay: boolean,
|
||||||
|
isCustom = false,
|
||||||
|
): string {
|
||||||
|
if (isCustom) return '__custom__';
|
||||||
if (leadMinutes === null) return '__none__';
|
if (leadMinutes === null) return '__none__';
|
||||||
const presets = isAllDay ? ALLDAY_REMINDER_PRESETS : TIMED_REMINDER_PRESETS;
|
const presets = isAllDay ? ALLDAY_REMINDER_PRESETS : TIMED_REMINDER_PRESETS;
|
||||||
if (presets.has(leadMinutes)) return String(leadMinutes);
|
if (presets.has(leadMinutes)) return String(leadMinutes);
|
||||||
@@ -317,8 +327,13 @@ export function EventForm() {
|
|||||||
setLocation(occurrence?.location ?? '');
|
setLocation(occurrence?.location ?? '');
|
||||||
setDescription(occurrence?.description ?? '');
|
setDescription(occurrence?.description ?? '');
|
||||||
// Phase 11: derive reminder picker value from occurrence (edit-mode pre-population, D-01/D-07)
|
// Phase 11: derive reminder picker value from occurrence (edit-mode pre-population, D-01/D-07)
|
||||||
|
// CR-01 (Plan 11-05): pass reminderIsCustom so custom alarms initialize to '__custom__'
|
||||||
|
// instead of '__none__', making the D-08 preserve path reachable on edit.
|
||||||
const occAllDay = occurrence?.allDay ?? false;
|
const occAllDay = occurrence?.allDay ?? false;
|
||||||
setReminderValue(deriveReminderValue(occurrence?.reminderLeadMinutes ?? null, occAllDay));
|
const occIsCustom = occurrence?.reminderIsCustom ?? false;
|
||||||
|
setReminderValue(
|
||||||
|
deriveReminderValue(occurrence?.reminderLeadMinutes ?? null, occAllDay, occIsCustom),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}, [eventFormOpen, eventFormMode, eventFormUid, occurrence?.uid]); // eslint-disable-line react-hooks/exhaustive-deps
|
}, [eventFormOpen, eventFormMode, eventFormUid, occurrence?.uid]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user