From 58a6f45c15b3baabc121ebf0ba349ba0ee88acde Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:45:43 +0200 Subject: [PATCH] fix: save all-day events with the right length, a time zone and explicit nulls for cleared fields (#805) --- docs/parity/05-calendar.md | 6 +-- src/api/types.ts | 27 ++++++----- src/components/calendar/EventModal.tsx | 66 +++++++++++++++++++------- src/lib/calendar-timezone.ts | 65 +++++++++++++++++++++++++ src/lib/recurrence.ts | 5 +- src/stores/settings-store.ts | 4 ++ 6 files changed, 140 insertions(+), 33 deletions(-) create mode 100644 src/lib/calendar-timezone.ts diff --git a/docs/parity/05-calendar.md b/docs/parity/05-calendar.md index c1835549..66941b29 100644 --- a/docs/parity/05-calendar.md +++ b/docs/parity/05-calendar.md @@ -7,17 +7,17 @@ RN has a solid read path (month/week/agenda, shared-calendar namespacing, per-vi ### Recurrence -- [ ] **Recurrence scope dialog is a stub: "This event" / "This and following" behave as "All events"** — `P1` — `missing` +- [x] **Recurrence scope dialog is a stub: "This event" / "This and following" behave as "All events"** — `P1` — `missing` — fixed in 161baf1 - What WEB does: `handleScopeSelect` implements all three scopes: "this" writes a `recurrenceOverrides/` patch (or the synthetic id on Stalwart >= 0.16.20), "this_and_future" truncates the master with `until` and creates a new series, "all" patches the master; delete does the mirror (exclude / truncate / destroy) and refetches the range (ref `components/calendar/calendar-app.tsx:842-998`, `lib/recurrence-overrides.ts:21-32`, changelog 1.9.0 "Edit a single recurring occurrence via a one-shot override patch"). - What RN does: `handleScopeSelect` ignores the chosen scope with a TODO ("treat all scopes as 'all'") and either opens the edit modal on the occurrence or deletes `action.event.id`, which the store resolves to the master via `originalId` (ref `src/screens/CalendarScreen.tsx:347-363`, `src/stores/calendar-store.ts:306-329`). Choosing "This event" on delete destroys the entire series. - Fix hint: port WEB's `handleScopeSelect`/`truncateRecurrenceAtEvent`/`findMasterEvent` into CalendarScreen; add `buildRecurrenceOverridePatch` (WEB `lib/recurrence-overrides.ts`) and `RECURRENCE_OVERRIDE_IMMUTABLE_KEYS`; for "this" write `{ [`recurrenceOverrides/${recurrenceId}`]: override }` on `originalId`; for delete-this write `{ excluded: true }`; refresh after. -- [ ] **Editing an expanded occurrence rewrites the master's `start` to the occurrence date (series shifts / earlier occurrences vanish)** — `P1` — `bug` +- [x] **Editing an expanded occurrence rewrites the master's `start` to the occurrence date (series shifts / earlier occurrences vanish)** — `P1` — `bug` — fixed in 161baf1 - What WEB does: for scope "all" the updates go to the master but `start` from an occurrence is never applied to the master unless the user changed it; scope "this" goes into an override (ref `components/calendar/calendar-app.tsx:930-943`). - What RN does: `EventModal` seeds `start` from the occurrence (`getEventStartDate(event)`, `src/components/calendar/EventModal.tsx:197`) and `handleSave` always sends `start` + `duration` (`EventModal.tsx:262-281`); `updateEvent` resolves the occurrence to `originalId` and PATCHes the master (`src/stores/calendar-store.ts:306-318`). Any occurrence edit moves the whole series to that occurrence's date. - Fix hint: part of the scope work above; when scope is "all", drop `start` from the patch unless the user actually changed the date/time relative to the occurrence, or compute the delta and apply it to the master's `start`. -- [ ] **Store's optimistic merge after an occurrence edit only updates one occurrence** — `P2` — `bug` +- [x] **Store's optimistic merge after an occurrence edit only updates one occurrence** — `P2` — `bug` — fixed in 161baf1 - What WEB does: after any occurrence or master mutation with expanded occurrences in view it refetches the visible range (`stores/calendar-store.ts:186-206, 749-751`). - What RN does: `updateEvent` maps only `e.id === id` (`src/stores/calendar-store.ts:315-317`), so sibling occurrences keep the old title/time until a pull-to-refresh; `deleteEvent` removes only the tapped occurrence from state while the server destroyed the master (`:320-329`). - Fix hint: after mutating a `recurrenceId`/`recurrenceRules` event call `get().refresh()` (RN already has it) like WEB's `refetchAfterOccurrenceMutation`. diff --git a/src/api/types.ts b/src/api/types.ts index a5042c6d..134bffa4 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -452,30 +452,33 @@ export interface CalendarEvent { description?: string; start: string; duration?: string; - timeZone?: string; + // `null` on write clears a field the server holds (JSON drops `undefined`, + // so an omitted key keeps the old value). All-day events carry a null zone. + timeZone?: string | null; showWithoutTime?: boolean; utcStart?: string; utcEnd?: string; status?: string; freeBusyStatus?: string; - participants?: Record; - // Scheduling (iTIP/iMIP). `replyTo` tells the server where RSVP replies go; - // Stalwart additionally exposes the organizer as `organizerCalendarAddress`. - replyTo?: Record; - organizerCalendarAddress?: string; + participants?: Record | null; + // Scheduling (iTIP/iMIP). Stalwart derives the iCalendar ORGANIZER solely + // from `organizerCalendarAddress` (a `mailto:` URI); RFC 8984's `replyTo` is + // retired in jscalendarbis and only kept here so old values round-trip. + replyTo?: Record | null; + organizerCalendarAddress?: string | null; sequence?: number; - recurrenceRules?: RecurrenceRule[]; - recurrenceOverrides?: Record>; - excludedRecurrenceRules?: RecurrenceRule[]; + recurrenceRules?: RecurrenceRule[] | null; + recurrenceOverrides?: Record> | null; + excludedRecurrenceRules?: RecurrenceRule[] | null; recurrenceId?: string; originalId?: string; // Client-only: the event's server-side calendarIds (raw, unprefixed), kept // when `calendarIds` is remapped to namespaced store ids for shared events. originalCalendarIds?: Record; useDefaultAlerts?: boolean; - alerts?: Record; - locations?: Record; - virtualLocations?: Record; + alerts?: Record | null; + locations?: Record | null; + virtualLocations?: Record | null; links?: Record; created?: string; updated?: string; diff --git a/src/components/calendar/EventModal.tsx b/src/components/calendar/EventModal.tsx index 23fd78f4..ee158a07 100644 --- a/src/components/calendar/EventModal.tsx +++ b/src/components/calendar/EventModal.tsx @@ -39,10 +39,12 @@ import { useColors } from '../../theme/colors'; import { buildAllDayDuration, getCalendarColor, + getEventDisplayEndDate, getEventEndDate, getEventStartDate, getPrimaryCalendarId, } from '../../lib/calendar-utils'; +import { getEffectiveTimeZone } from '../../lib/calendar-timezone'; import { alertsToReminders, remindersToAlerts, @@ -195,7 +197,10 @@ export function EventModal({ setDescription(event.description || ''); setAllDay(!!event.showWithoutTime); setStart(getEventStartDate(event)); - setEnd(getEventEndDate(event)); + // All-day events store an exclusive end (start + P1D = next day 00:00); + // the editor works with the inclusive last day, otherwise every re-save + // grows the event by one day. + setEnd(event.showWithoutTime ? getEventDisplayEndDate(event) : getEventEndDate(event)); setCalendarId(getPrimaryCalendarId(event) || calendars[0]?.id || ''); setParticipants(event.participants || {}); const detected = detectRecurrence(event); @@ -230,11 +235,13 @@ export function EventModal({ React.useEffect(() => { if (allDay) { + // `end` is the inclusive last day for all-day events, so a one-day + // event has end === start (not the next day). const s = new Date(start); s.setHours(0, 0, 0, 0); const e = new Date(end); e.setHours(0, 0, 0, 0); - if (e <= s) e.setDate(s.getDate() + 1); + if (e < s) e.setTime(s.getTime()); setStart(s); setEnd(e); } @@ -244,11 +251,13 @@ export function EventModal({ const updateStart = (next: Date) => { const diff = end.getTime() - start.getTime(); setStart(next); - setEnd(new Date(next.getTime() + Math.max(diff, 15 * 60_000))); + setEnd(new Date(next.getTime() + (allDay ? Math.max(diff, 0) : Math.max(diff, 15 * 60_000)))); }; const updateEnd = (next: Date) => { - if (next <= start) { + if (allDay) { + setEnd(next < start ? new Date(start) : next); + } else if (next <= start) { setEnd(addMinutes(start, 15)); } else { setEnd(next); @@ -259,26 +268,51 @@ export function EventModal({ if (!title.trim() || !calendarId) return; setSaving(true); try { + // JSON drops `undefined`, so a cleared field must be sent as an explicit + // `null` when editing or the server keeps the old value. + const had = (key: keyof CalendarEvent): boolean => { + const v = event?.[key]; + return !!v && (typeof v !== 'object' || Object.keys(v as object).length > 0); + }; + const clearedOr = (value: T | undefined, key: keyof CalendarEvent): T | null | undefined => + value !== undefined ? value : isEdit && had(key) ? null : undefined; + + const recurrenceRules = + recurrence === 'custom' + ? customRule + ? [customRule] + : undefined + : recurrenceFromOption(recurrence); const data: Partial = { title: title.trim(), - description: description.trim() || undefined, - showWithoutTime: allDay || undefined, + description: description.trim() || (isEdit && event?.description ? '' : undefined), + showWithoutTime: allDay || (isEdit ? false : undefined), start: allDay ? format(start, "yyyy-MM-dd'T'00:00:00") : format(start, "yyyy-MM-dd'T'HH:mm:ss"), duration: allDay ? buildAllDayDuration(start, end) : buildDuration(start, end), - participants: Object.keys(participants).length > 0 ? participants : undefined, - recurrenceRules: - recurrence === 'custom' - ? customRule - ? [customRule] - : undefined - : recurrenceFromOption(recurrence), - alerts: remindersToAlerts(reminders), + // A floating (zone-less) start renders at a different instant for + // every viewer; label the wall-clock with the zone it was entered in. + // All-day events are date-only and carry no zone. + timeZone: allDay ? null : getEffectiveTimeZone(), + participants: clearedOr( + Object.keys(participants).length > 0 ? participants : undefined, + 'participants', + ), + recurrenceRules: clearedOr(recurrenceRules, 'recurrenceRules'), + alerts: clearedOr(remindersToAlerts(reminders), 'alerts'), useDefaultAlerts: reminders.length > 0 ? false : undefined, - locations: buildLocations(location), - virtualLocations: buildVirtualLocations(videoUrl), + locations: clearedOr(buildLocations(location), 'locations'), + virtualLocations: clearedOr(buildVirtualLocations(videoUrl), 'virtualLocations'), }; + if (data.recurrenceRules === null) { + // Dropping the rule leaves per-occurrence data dangling. + if (had('recurrenceOverrides')) data.recurrenceOverrides = null; + if (had('excludedRecurrenceRules')) data.excludedRecurrenceRules = null; + } + for (const key of Object.keys(data) as (keyof CalendarEvent)[]) { + if (data[key] === undefined) delete data[key]; + } await onSave(data, calendarId); onClose(); } finally { diff --git a/src/lib/calendar-timezone.ts b/src/lib/calendar-timezone.ts new file mode 100644 index 00000000..3d9a44cf --- /dev/null +++ b/src/lib/calendar-timezone.ts @@ -0,0 +1,65 @@ +/** + * The IANA time zone the calendar works in. + * + * Timed events must be saved with a `timeZone` so other clients, CalDAV + * syncs and invitees resolve the wall-clock correctly; a floating DTSTART + * shifts whenever the viewer's zone differs from the author's. Mirrors the + * webmail's lib/timezone.ts: the user's override when set (#755), otherwise + * the device zone. + */ + +import { useSettingsStore } from '../stores/settings-store'; + +/** Sentinel for "follow the device" — the default. */ +export const AUTO_TIME_ZONE = 'auto'; + +/** The zone the device reports; `UTC` when detection fails. */ +export function getDeviceTimeZone(): string { + try { + return Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC'; + } catch { + return 'UTC'; + } +} + +const validityCache = new Map(); + +/** True when `Intl` accepts `tz` as a time zone identifier. */ +export function isValidTimeZone(tz: unknown): tz is string { + if (typeof tz !== 'string' || !tz) return false; + let valid = validityCache.get(tz); + if (valid === undefined) { + try { + new Intl.DateTimeFormat('en-US', { timeZone: tz }); + valid = true; + } catch { + valid = false; + } + validityCache.set(tz, valid); + } + return valid; +} + +/** + * Resolve a stored `timeZone` setting to a concrete IANA zone. `auto`, empty + * or unknown values (a zone synced from a client with a newer tz database) + * fall back to the device zone instead of throwing later inside Intl. + */ +export function resolveTimeZone( + setting: string | null | undefined, + deviceTimeZone = getDeviceTimeZone(), +): string { + if (!setting || setting === AUTO_TIME_ZONE) return deviceTimeZone; + return isValidTimeZone(setting) ? setting : deviceTimeZone; +} + +/** The zone new timed events are saved in and JMAP queries are interpreted in. */ +export function getEffectiveTimeZone(): string { + let setting: string | null | undefined; + try { + setting = useSettingsStore.getState().calendarTimeZone; + } catch { + setting = undefined; + } + return resolveTimeZone(setting); +} diff --git a/src/lib/recurrence.ts b/src/lib/recurrence.ts index 5ca183f7..9ff14af1 100644 --- a/src/lib/recurrence.ts +++ b/src/lib/recurrence.ts @@ -217,8 +217,9 @@ export function buildRuleFromEditorValue( '@type': 'RecurrenceRule', frequency: value.frequency, interval: Math.max(1, value.interval), - rscale: 'gregorian', - skip: 'omit', + // No rscale/skip: Stalwart serialises them into the RRULE as + // RSCALE=GREGORIAN;SKIP=OMIT, which DAVx5 rejects and which breaks + // Android CalDAV sync for the whole calendar (#805). firstDayOfWeek: 'mo', }; diff --git a/src/stores/settings-store.ts b/src/stores/settings-store.ts index fab3abf9..a5f06d56 100644 --- a/src/stores/settings-store.ts +++ b/src/stores/settings-store.ts @@ -169,6 +169,9 @@ interface PersistedSettings { calendarShowTimeInMonth: boolean; calendarShowWeekNumbers: boolean; calendarHoverPreview: CalendarHoverPreview; + // IANA zone the calendar works in, or 'auto' to follow the device (#755). + // Same key semantics as the webmail's `timeZone` setting. + calendarTimeZone: string; showBirthdayCalendar: boolean; enableCalendarTasks: boolean; showTasksOnCalendar: boolean; @@ -281,6 +284,7 @@ const DEFAULT_PERSISTED: PersistedSettings = { calendarShowTimeInMonth: true, calendarShowWeekNumbers: false, calendarHoverPreview: 'delay-500ms', + calendarTimeZone: 'auto', showBirthdayCalendar: true, enableCalendarTasks: false, showTasksOnCalendar: true,