From 643d704b7450e7c2d453260200d52b8480a9052c Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 14 Jun 2026 14:46:15 +0200 Subject: [PATCH] fix: namespace shared calendar ids to avoid collisions with own calendars --- src/api/calendar.ts | 16 +++- src/api/types.ts | 9 ++ src/stores/__tests__/calendar-store.test.ts | 36 +++++++ src/stores/calendar-store.ts | 100 ++++++++++++++------ 4 files changed, 129 insertions(+), 32 deletions(-) diff --git a/src/api/calendar.ts b/src/api/calendar.ts index fbfa246f..3581033d 100644 --- a/src/api/calendar.ts +++ b/src/api/calendar.ts @@ -89,7 +89,18 @@ export async function getCalendars(): Promise { USING, ); const list = methodResult<{ list: Calendar[] }>(sharedRes).list ?? []; - return list.map((cal) => ({ ...cal, accountId: sharedAccountId, isShared: true })); + // JMAP calendar ids are only unique within an account, so a shared + // "default" calendar collides with the user's own "default". Namespace + // the id (keeping the real id in originalId) so visibility toggles, + // event-query routing, and create/default mutations don't cross wires. + // Mirrors webmail's getAllCalendars. + return list.map((cal) => ({ + ...cal, + id: `${sharedAccountId}:${cal.id}`, + originalId: cal.id, + accountId: sharedAccountId, + isShared: true, + })); } catch { return []; } @@ -189,9 +200,10 @@ export async function createEvent( export async function batchCreateEvents( events: Partial[], calendarId: string, + targetAccountId?: string, ): Promise { if (events.length === 0) return 0; - const accountId = jmapClient.accountId; + const accountId = targetAccountId || jmapClient.accountId; const create: Record> = {}; events.forEach((e, i) => { create[`evt-${i}`] = { ...e, calendarIds: { [calendarId]: true } }; diff --git a/src/api/types.ts b/src/api/types.ts index 3d2e4be3..a686639d 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -455,6 +455,9 @@ export interface CalendarEvent { excludedRecurrenceRules?: RecurrenceRule[]; 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; @@ -474,6 +477,8 @@ export interface CalendarEvent { // primary account; set for events on calendars shared with the user so // mutations can be routed to the owning account. accountId?: string; + // Client-only: true when the event lives on a calendar shared with the user. + isShared?: boolean; } export interface CalendarRights { @@ -502,6 +507,10 @@ export interface Calendar { // Client-only: JMAP account the calendar was fetched from. Absent for the // primary account; set for calendars shared with the user. accountId?: string; + // Client-only: the real JMAP id of a shared calendar, before its `id` was + // namespaced as `${accountId}:${id}` to stay unique in the store. Mutations + // and event queries must use this raw id against the owning account. + originalId?: string; // Client-only: true when the calendar lives in another account (shared // with the user by its owner). isShared?: boolean; diff --git a/src/stores/__tests__/calendar-store.test.ts b/src/stores/__tests__/calendar-store.test.ts index 50d594f9..ed420d2d 100644 --- a/src/stores/__tests__/calendar-store.test.ts +++ b/src/stores/__tests__/calendar-store.test.ts @@ -129,6 +129,42 @@ describe('calendar-store', () => { expect(stored[0].originalId).toBe('ev1'); expect(stored[0].accountId).toBe('acc-2'); }); + + it('routes queries by account when own and shared calendars share a raw id', async () => { + // The user's own "default" calendar and a family group's "default" + // calendar collide on the raw JMAP id. The shared one is namespaced + // (`acc-2:default`, originalId "default"); the query must use the raw id + // against acc-2, and the returned event's calendarIds must be remapped to + // the namespaced store id so it isn't hidden with the personal calendar. + useCalendarStore.setState({ + calendars: [ + { id: 'default', name: 'Personal' } as any, + { id: 'acc-2:default', originalId: 'default', name: 'Family', accountId: 'acc-2', isShared: true } as any, + ], + }); + const queried: Record = {}; + mockQueryEvents.mockImplementation(async (ids: string[], _a: string, _b: string, accountId?: string) => { + queried[accountId ?? 'primary'] = ids; + return accountId === 'acc-2' ? ['fam1'] : []; + }); + mockGetEvents.mockImplementation(async (_ids: string[], accountId?: string) => + accountId === 'acc-2' + ? [{ id: 'fam1', title: 'Soccer', start: '2026-03-20T15:00:00', calendarIds: { default: true } }] + : [], + ); + + await useCalendarStore.getState().fetchEvents(['default', 'acc-2:default'], '2026-03-01', '2026-03-31'); + + // The shared account is queried with the raw calendar id, not the prefix. + expect(queried['acc-2']).toEqual(['default']); + const ev = useCalendarStore.getState().events[0]; + expect(ev.calendarIds).toEqual({ 'acc-2:default': true }); + + // Hiding the personal "default" must not hide the family event. + useCalendarStore.setState({ hiddenCalendarIds: ['default'] }); + const visible = selectVisibleEvents(useCalendarStore.getState()); + expect(visible.map((e) => e.id)).toContain('acc-2:fam1'); + }); }); describe('ensureRange', () => { diff --git a/src/stores/calendar-store.ts b/src/stores/calendar-store.ts index 25f2c45b..a5085316 100644 --- a/src/stores/calendar-store.ts +++ b/src/stores/calendar-store.ts @@ -25,6 +25,37 @@ function hasSchedulingParticipants(event?: Partial | null): boole const HIDDEN_CALENDARS_STORAGE_KEY = 'webmail:calendar:hidden:v1'; +// Map a server CalendarEvent onto its store representation. For events from a +// shared/group account the JMAP id is only unique within that account, so we +// namespace `id` (stashing the real id in originalId) and remap the event's +// `calendarIds` from raw server ids onto the namespaced store calendar ids so +// visibility filtering and per-calendar colour lookup line up. Events from the +// primary account are returned unchanged. Mirrors webmail's +// mapServerEventToStoreEvent. +function mapServerEventToStoreEvent( + event: CalendarEvent, + calendars: Calendar[], + accountId?: string, +): CalendarEvent { + if (!accountId) return event; + const mapped: Record = {}; + for (const [calId, included] of Object.entries(event.calendarIds || {})) { + const cal = calendars.find( + (c) => (c.originalId || c.id) === calId && c.accountId === accountId, + ); + mapped[cal?.id || calId] = included; + } + return { + ...event, + id: `${accountId}:${event.id}`, + originalId: event.id, + originalCalendarIds: event.calendarIds, + calendarIds: Object.keys(mapped).length > 0 ? mapped : event.calendarIds, + accountId, + isShared: true, + }; +} + export interface LoadedRange { after: string; before: string; @@ -135,14 +166,20 @@ export const useCalendarStore = create()( try { // Group the requested calendars by owning account: the primary account // (calendars without an accountId tag) plus one group per shared - // account, since CalendarEvent/query is scoped to a single account. + // account, since CalendarEvent/query is scoped to a single account. The + // incoming ids are store ids (shared calendars are namespaced + // `${accountId}:${id}`); map them back to the raw server ids the query + // filter expects. const calendars = get().calendars; + const byId = new Map(calendars.map((c) => [c.id, c])); const groups = new Map(); for (const id of calendarIds) { - const accountId = calendars.find((c) => c.id === id)?.accountId; + const cal = byId.get(id); + const accountId = cal?.accountId; + const serverId = cal?.originalId || id; const group = groups.get(accountId); - if (group) group.push(id); - else groups.set(accountId, [id]); + if (group) group.push(serverId); + else groups.set(accountId, [serverId]); } if (groups.size === 0) groups.set(undefined, []); @@ -152,17 +189,7 @@ export const useCalendarStore = create()( const eventIds = (await queryEvents(ids, after, before, accountId)) ?? []; if (eventIds.length === 0) continue; const fetched = (await fetchEvents(eventIds, accountId)) ?? []; - // Shared/group accounts: JMAP object ids are only unique *within* an - // account, so two accounts can return events with the same id. - // Namespace shared ids with the owning accountId (stashing the real id - // in originalId for mutations) so they don't collide with the user's - // own events in the store / React keys. Mirrors webmail's - // mapServerEventToStoreEvent. - raw.push( - ...(accountId - ? fetched.map((e) => ({ ...e, accountId, id: `${accountId}:${e.id}`, originalId: e.id })) - : fetched), - ); + raw.push(...fetched.map((e) => mapServerEventToStoreEvent(e, calendars, accountId))); } catch (err) { // A failing shared account must not hide the user's own events. if (!accountId) throw err; @@ -235,20 +262,22 @@ export const useCalendarStore = create()( }, createEvent: async (event, calendarId) => { - // Shared calendars live in the owner's account — route the create there. - const accountId = get().calendars.find((c) => c.id === calendarId)?.accountId; + // Shared calendars live in the owner's account — route the create there, + // and against the calendar's raw server id (calendarId is the namespaced + // store id for shared calendars). + const calendars = get().calendars; + const cal = calendars.find((c) => c.id === calendarId); + const accountId = cal?.accountId; const created = await apiCreateEvent( event, - calendarId, + cal?.originalId || calendarId, hasSchedulingParticipants(event) ? true : undefined, accountId, ); - // Namespace shared/group event ids the same way fetchEvents does so the - // optimistic insert doesn't collide with the user's own events. - const storeCreated = accountId - ? { ...created, accountId, id: `${accountId}:${created.id}`, originalId: created.id } - : created; - set({ events: [...get().events, storeCreated] }); + // Map the same way fetchEvents does so the optimistic insert doesn't + // collide with the user's own events and stays visible under the right + // calendar filter. + set({ events: [...get().events, mapServerEventToStoreEvent(created, calendars, accountId)] }); return created; }, @@ -300,20 +329,26 @@ export const useCalendarStore = create()( importEvents: async (events, calendarId) => { if (events.length === 0) return 0; + // Shared calendars live in the owner's account and carry a namespaced + // store id — resolve the raw server id + owning account so dedup and + // create target the right place. + const cal = get().calendars.find((c) => c.id === calendarId); + const accountId = cal?.accountId; + const serverCalendarId = cal?.originalId || calendarId; // Stalwart enforces UID uniqueness across calendars. Skip events whose UID // already exists; create the rest. (We don't attempt cross-calendar linking // on mobile — a duplicate is simply skipped.) let toCreate = events; try { - const existingIds = await queryEvents([], '', ''); - const existing = existingIds.length > 0 ? await fetchEvents(existingIds) : []; + const existingIds = await queryEvents([], '', '', accountId); + const existing = existingIds.length > 0 ? await fetchEvents(existingIds, accountId) : []; const seenUids = new Set(existing.map((e) => e.uid).filter(Boolean) as string[]); toCreate = events.filter((e) => !e.uid || !seenUids.has(e.uid)); } catch { // Couldn't dedupe — proceed and let the server reject genuine dupes. } if (toCreate.length === 0) return 0; - const count = await apiBatchCreateEvents(toCreate, calendarId); + const count = await apiBatchCreateEvents(toCreate, serverCalendarId, accountId); await get().refresh(); return count; }, @@ -326,7 +361,7 @@ export const useCalendarStore = create()( setDefaultCalendar: async (id) => { const cal = get().calendars.find((c) => c.id === id); - await apiSetDefaultCalendar(id, cal?.accountId); + await apiSetDefaultCalendar(cal?.originalId || id, cal?.accountId); set({ calendars: get().calendars.map((c) => { if (c.id === id) return { ...c, isDefault: true }; @@ -341,8 +376,13 @@ export const useCalendarStore = create()( }, createTask: async (task, calendarId) => { - const accountId = get().calendars.find((c) => c.id === calendarId)?.accountId; - await apiCreateEvent({ ...task, '@type': 'Task' }, calendarId, undefined, accountId); + const cal = get().calendars.find((c) => c.id === calendarId); + await apiCreateEvent( + { ...task, '@type': 'Task' }, + cal?.originalId || calendarId, + undefined, + cal?.accountId, + ); await get().refresh(); },