From 41e8f00509f7c11fc14687acd8e18c35c2176ff7 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:57:07 +0100 Subject: [PATCH] fix: keep a calendar write that outlasts an account switch out of the next account --- locales/rn/en.json | 1 + .../calendar-store-account-switch.test.ts | 261 ++++++++++++++++++ .../calendar-store-occurrence-answers.test.ts | 6 +- .../calendar-store-sort-order.test.ts | 4 +- src/stores/__tests__/calendar-store.test.ts | 42 +-- src/stores/calendar-store.ts | 198 ++++++++++--- 6 files changed, 442 insertions(+), 70 deletions(-) create mode 100644 src/stores/__tests__/calendar-store-account-switch.test.ts diff --git a/locales/rn/en.json b/locales/rn/en.json index edb4c1dc..b52747bb 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -6,6 +6,7 @@ "not_starred": "Not starred" }, "calendar": { + "account_changed": "The account changed while the calendar was loading or saving.", "alerts": { "minutes_after": "{count, plural, one {# minute after} other {# minutes after}}" }, diff --git a/src/stores/__tests__/calendar-store-account-switch.test.ts b/src/stores/__tests__/calendar-store-account-switch.test.ts new file mode 100644 index 00000000..d4e48279 --- /dev/null +++ b/src/stores/__tests__/calendar-store-account-switch.test.ts @@ -0,0 +1,261 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Writes still under way when the account changes. Event and calendar ids +// repeat across accounts (two accounts can both have calendar "b" with event +// "c"), so a write that finishes after the switch must not send its next +// step, which would reach the next account, nor change the store, which +// shows that account by then. + +vi.mock('../../api/calendar', () => ({ + getCalendars: vi.fn(), + queryEvents: vi.fn(async () => []), + getEvents: vi.fn(async () => []), + getEventIdentities: vi.fn(async () => []), + scanCalendarObjects: vi.fn(async () => []), + isCalendarAccessDenied: vi.fn(() => false), + noteCalendarAccessError: vi.fn(() => false), + resetCalendarAccessDenied: vi.fn(), + createEvent: vi.fn(), + updateEvent: vi.fn(async () => undefined), + deleteEvents: vi.fn(async () => undefined), + batchCreateEvents: vi.fn(async (events: unknown[]) => ({ created: events.length, refused: [] })), + rsvpEvent: vi.fn(async () => undefined), + createCalendar: vi.fn(), + updateCalendar: vi.fn(async () => undefined), + deleteCalendar: vi.fn(async () => undefined), + setDefaultCalendar: vi.fn(async () => undefined), + supportsSyntheticCalendarIds: vi.fn(async () => false), + queryExpandedEvents: vi.fn(async () => []), + hydrateExpandedOccurrences: vi.fn(async (events: unknown[]) => events), + resetSyntheticIdSupport: vi.fn(), + resetTimeZoneChecks: vi.fn(), +})); + +vi.mock('@react-native-async-storage/async-storage', () => ({ + default: { + getItem: vi.fn().mockResolvedValue(null), + setItem: vi.fn().mockResolvedValue(undefined), + removeItem: vi.fn().mockResolvedValue(undefined), + }, +})); + +vi.mock('../../api/jmap-client', () => ({ + jmapClient: { accountId: 'acc-1', isConnected: true }, +})); + +vi.mock('../../device-sync/app/request', () => ({ requestDeviceSync: vi.fn() })); + +import * as calendarApi from '../../api/calendar'; +import type { CalendarEvent } from '../../api/types'; +import { CalendarAccountChangedError, useCalendarStore } from '../calendar-store'; + +const mockUpdate = vi.mocked(calendarApi.updateEvent); +const mockDelete = vi.mocked(calendarApi.deleteEvents); +const mockCreate = vi.mocked(calendarApi.createEvent); +const mockGetEvents = vi.mocked(calendarApi.getEvents); +const mockQueryEvents = vi.mocked(calendarApi.queryEvents); +const mockQueryExpanded = vi.mocked(calendarApi.queryExpandedEvents); +const mockBatchCreate = vi.mocked(calendarApi.batchCreateEvents); +const mockUpdateCalendar = vi.mocked(calendarApi.updateCalendar); +const mockCreateCalendar = vi.mocked(calendarApi.createCalendar); + +const RID = '2026-03-02T09:00:00'; +const participants = { + p0: { calendarAddress: 'mailto:me@example.com', roles: { owner: true }, participationStatus: 'accepted' }, + p1: { calendarAddress: 'mailto:bob@example.com', roles: { attendee: true }, participationStatus: 'accepted' }, +}; + +const event = (id: string, title: string): CalendarEvent => ({ + id, uid: `${id}@example.com`, title, start: '2026-03-03T10:00:00', duration: 'PT1H', calendarIds: { b: true }, +} as unknown as CalendarEvent); + +// The next account: its calendar "b" holds an event "c" too. +const THEIRS = event('c', 'Their event'); + +/** What an account switch does to the store while a request is under way. */ +function switchAccount(): void { + useCalendarStore.getState().reset(); + useCalendarStore.setState({ + calendars: [{ id: 'b', name: 'Their calendar', sortOrder: 0 }] as never, + events: [THEIRS], + tasks: [{ ...THEIRS, '@type': 'Task', progress: 'needs-action' } as CalendarEvent], + loadedRange: { after: '2026-03-01T00:00:00Z', before: '2026-04-01T00:00:00Z' }, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + useCalendarStore.getState().reset(); + useCalendarStore.setState({ + calendars: [{ id: 'b', name: 'Mine', sortOrder: 0 }, { id: 'e', name: 'Other', sortOrder: 1 }] as never, + events: [event('c', 'My event')], + tasks: [{ ...event('c', 'My task'), '@type': 'Task', progress: 'needs-action' } as CalendarEvent], + loadedRange: { after: '2026-03-01T00:00:00Z', before: '2026-04-01T00:00:00Z' }, + }); +}); + +describe('a write the account switch overtook', () => { + it('names the account it started in and leaves the next account\'s event alone (updateEvent)', async () => { + mockUpdate.mockImplementationOnce(async () => switchAccount()); + + await useCalendarStore.getState().updateEvent('c', { title: 'Renamed' }); + + expect(mockUpdate).toHaveBeenCalledWith('c', { title: 'Renamed' }, undefined, 'acc-1'); + expect(useCalendarStore.getState().events).toEqual([THEIRS]); + }); + + it('leaves the next account\'s event of the same id in the store (deleteEvent)', async () => { + mockDelete.mockImplementationOnce(async () => switchAccount()); + + await useCalendarStore.getState().deleteEvent('c'); + + expect(mockDelete).toHaveBeenCalledWith(['c'], undefined, 'acc-1'); + expect(useCalendarStore.getState().events).toEqual([THEIRS]); + }); + + it('neither re-reads nor shows the event it made (createEvent)', async () => { + mockCreate.mockImplementationOnce(async () => { + switchAccount(); + return event('n', 'New'); + }); + + await useCalendarStore.getState().createEvent({ title: 'New' }, 'b'); + + expect(mockCreate).toHaveBeenCalledWith({ title: 'New' }, 'b', undefined, 'acc-1'); + expect(mockGetEvents).not.toHaveBeenCalled(); + expect(useCalendarStore.getState().events).toEqual([THEIRS]); + }); + + it('leaves the answer off the next account\'s event (rsvpEvent)', async () => { + useCalendarStore.setState({ events: [{ ...event('c', 'Invite'), participants } as unknown as CalendarEvent] }); + const theirs = { ...THEIRS, participants }; + vi.mocked(calendarApi.rsvpEvent).mockImplementationOnce(async () => { + switchAccount(); + useCalendarStore.setState({ events: [theirs as unknown as CalendarEvent] }); + }); + + await useCalendarStore.getState().rsvpEvent('c', 'p1', 'declined'); + + expect(calendarApi.rsvpEvent).toHaveBeenCalledWith('c', 'p1', 'declined', undefined, 'acc-1'); + expect(useCalendarStore.getState().events[0].participants?.p1.participationStatus).toBe('accepted'); + }); + + it('sends no second step of a first change to one server occurrence', async () => { + useCalendarStore.setState({ + events: [{ + id: 's1', baseEventId: 'base', uid: 'series@example.com', recurrenceId: RID, start: RID, duration: 'PT1H', + title: 'Standup', recurrenceRules: [{ frequency: 'weekly' }], recurrenceOverrides: null, + calendarIds: { b: true }, organizerCalendarAddress: 'mailto:me@example.com', participants, sequence: 1, + } as unknown as CalendarEvent], + }); + mockUpdate.mockImplementationOnce(async () => switchAccount()); + + await expect(useCalendarStore.getState().updateEvent('s1', { title: 'Standup, room 2', participants } as Partial, { + sendSchedulingMessages: true, + })).rejects.toBeInstanceOf(CalendarAccountChangedError); + + // The quiet override only: the change itself would go to the next account. + expect(mockUpdate).toHaveBeenCalledTimes(1); + expect(mockUpdate.mock.calls[0][2]).toBe(false); + expect(mockQueryExpanded).not.toHaveBeenCalled(); + }); + + it('sends no second step of a first change to one occurrence expanded on the device', async () => { + useCalendarStore.setState({ + events: [{ + id: `base:${RID}`, originalId: 'base', uid: 'series@example.com', recurrenceId: RID, start: RID, duration: 'PT1H', + title: 'Standup', recurrenceRules: [{ frequency: 'weekly' }], recurrenceOverrides: null, + calendarIds: { b: true }, organizerCalendarAddress: 'mailto:me@example.com', participants, sequence: 1, + } as unknown as CalendarEvent], + }); + mockUpdate.mockImplementationOnce(async () => switchAccount()); + + await expect(useCalendarStore.getState().updateEvent(`base:${RID}`, { title: 'Standup, room 2', participants } as Partial, { + sendSchedulingMessages: true, + })).rejects.toBeInstanceOf(CalendarAccountChangedError); + + expect(mockUpdate).toHaveBeenCalledTimes(1); + expect(mockUpdate.mock.calls[0][2]).toBe(false); + }); + + it('imports no further batch into the next account\'s calendar of the same id', async () => { + mockBatchCreate.mockImplementationOnce(async (events: unknown[]) => { + switchAccount(); + return { created: events.length, refused: [] }; + }); + const events = Array.from({ length: 60 }, (_, i) => ({ uid: `u${i}`, title: `E${i}`, start: '2026-03-03T10:00:00' })); + + await expect(useCalendarStore.getState().importEvents(events, 'b')).rejects.toBeInstanceOf(CalendarAccountChangedError); + + expect(mockQueryEvents).toHaveBeenCalledWith([], '', '', 'acc-1'); + expect(mockBatchCreate).toHaveBeenCalledTimes(1); + expect(mockBatchCreate.mock.calls[0][2]).toBe('acc-1'); + }); + + it('links no further UID once the account changed (importEvents)', async () => { + vi.mocked(calendarApi.getEventIdentities).mockResolvedValueOnce([ + { id: 'x1', uid: 'u1', calendarIds: { e: true } }, + { id: 'x2', uid: 'u2', calendarIds: { e: true } }, + ] as never); + mockUpdate.mockImplementationOnce(async () => switchAccount()); + + await expect(useCalendarStore.getState().importEvents([{ uid: 'u1' }, { uid: 'u2' }], 'b')) + .rejects.toBeInstanceOf(CalendarAccountChangedError); + + expect(mockUpdate).toHaveBeenCalledTimes(1); + expect(mockBatchCreate).not.toHaveBeenCalled(); + }); + + it('renumbers none of the next account\'s calendars (reorderCalendars)', async () => { + useCalendarStore.setState({ calendars: [{ id: 'b', name: 'Mine', sortOrder: 5 }, { id: 'e', name: 'Other', sortOrder: 6 }] as never }); + mockUpdateCalendar.mockImplementationOnce(async () => switchAccount()); + + await expect(useCalendarStore.getState().reorderCalendars(['e', 'b'])).rejects.toBeInstanceOf(CalendarAccountChangedError); + + expect(mockUpdateCalendar).toHaveBeenCalledTimes(1); + expect(mockUpdateCalendar.mock.calls[0][2]).toBe('acc-1'); + expect(useCalendarStore.getState().calendars).toEqual([{ id: 'b', name: 'Their calendar', sortOrder: 0 }]); + }); + + it('adds the calendar it made to no other account\'s list (createCalendar)', async () => { + mockCreateCalendar.mockImplementationOnce(async () => { + switchAccount(); + return { id: 'n', name: 'New' } as never; + }); + + await useCalendarStore.getState().createCalendar('New'); + + expect(mockCreateCalendar).toHaveBeenCalledWith('New', undefined, undefined, 'acc-1'); + expect(useCalendarStore.getState().calendars.map((c) => c.id)).toEqual(['b']); + }); + + it('removes none of the next account\'s calendar or events (removeCalendar)', async () => { + vi.mocked(calendarApi.deleteCalendar).mockImplementationOnce(async () => switchAccount()); + + await useCalendarStore.getState().removeCalendar('b'); + + expect(calendarApi.deleteCalendar).toHaveBeenCalledWith('b', 'acc-1'); + expect(useCalendarStore.getState().calendars.map((c) => c.id)).toEqual(['b']); + expect(useCalendarStore.getState().events).toEqual([THEIRS]); + }); + + it('changes none of the next account\'s tasks', async () => { + mockUpdate.mockImplementationOnce(async () => switchAccount()); + await useCalendarStore.getState().updateTask('c', { title: 'Renamed' }); + expect(useCalendarStore.getState().tasks[0].title).toBe('Their event'); + + mockDelete.mockImplementationOnce(async () => switchAccount()); + await useCalendarStore.getState().deleteTask('c'); + expect(useCalendarStore.getState().tasks).toHaveLength(1); + + // A refused tick is put back only on the task it was made on. + useCalendarStore.setState({ tasks: [{ ...event('c', 'Mine'), '@type': 'Task', progress: 'needs-action' } as CalendarEvent] }); + mockUpdate.mockImplementationOnce(async () => { + switchAccount(); + useCalendarStore.setState({ tasks: [{ ...THEIRS, '@type': 'Task', progress: 'completed' } as CalendarEvent] }); + throw new Error('Network request failed'); + }); + await expect(useCalendarStore.getState().toggleTaskComplete('c')).rejects.toThrow('Network request failed'); + expect(useCalendarStore.getState().tasks[0].progress).toBe('completed'); + }); +}); diff --git a/src/stores/__tests__/calendar-store-occurrence-answers.test.ts b/src/stores/__tests__/calendar-store-occurrence-answers.test.ts index 529fc654..c661b356 100644 --- a/src/stores/__tests__/calendar-store-occurrence-answers.test.ts +++ b/src/stores/__tests__/calendar-store-occurrence-answers.test.ts @@ -111,7 +111,7 @@ describe.each(['0.16.23', '1.0'])('a first change to one server occurrence on St ['s9', { title: 'Standup, room 2', participants }, true], ]); // Looked up around the occurrence, expanded. - expect(mockGetEvents).toHaveBeenCalledWith(['s9', 'other'], undefined, { expanded: true }); + expect(mockGetEvents).toHaveBeenCalledWith(['s9', 'other'], 'acc-1', { expanded: true }); }); it('sends the change to the same id when the override keeps it (recurrence-keyed ids, live)', async () => { @@ -134,7 +134,7 @@ describe.each(['0.16.23', '1.0'])('a first change to one server occurrence on St } as Partial, { sendSchedulingMessages: true }); expect(mockUpdate).toHaveBeenCalledTimes(1); - expect(mockUpdate.mock.calls[0]).toEqual(['s1', expect.objectContaining({ participants: askedAgain }), true, undefined]); + expect(mockUpdate.mock.calls[0]).toEqual(['s1', expect.objectContaining({ participants: askedAgain }), true, 'acc-1']); }); it('sends the change once to an occurrence that has its override already', async () => { @@ -248,7 +248,7 @@ describe('an occurrence\'s alerts on a series that uses the calendar\'s default await useCalendarStore.getState().updateEvent(`base:${RID}`, edit); - expect(mockUpdate.mock.calls[0]).toEqual(['base', { useDefaultAlerts: false, alerts: null }, false, undefined]); + expect(mockUpdate.mock.calls[0]).toEqual(['base', { useDefaultAlerts: false, alerts: null }, false, 'acc-1']); expect(mockUpdate.mock.calls[1][0]).toBe('base'); expect(mockUpdate.mock.calls[1][1]).toHaveProperty('recurrenceOverrides'); }); diff --git a/src/stores/__tests__/calendar-store-sort-order.test.ts b/src/stores/__tests__/calendar-store-sort-order.test.ts index 0d2d6cde..719b982e 100644 --- a/src/stores/__tests__/calendar-store-sort-order.test.ts +++ b/src/stores/__tests__/calendar-store-sort-order.test.ts @@ -60,8 +60,8 @@ describe('calendar order', () => { await done; expect(mockUpdateCalendar.mock.calls).toEqual([ - ['b', { sortOrder: 1 }, undefined], - ['a', { sortOrder: 2 }, undefined], + ['b', { sortOrder: 1 }, 'acc-1'], + ['a', { sortOrder: 2 }, 'acc-1'], ]); }); diff --git a/src/stores/__tests__/calendar-store.test.ts b/src/stores/__tests__/calendar-store.test.ts index 4def2022..da7dd819 100644 --- a/src/stores/__tests__/calendar-store.test.ts +++ b/src/stores/__tests__/calendar-store.test.ts @@ -560,7 +560,7 @@ describe('calendar-store', () => { 'cal-1', ); - expect(mockGetEvents).toHaveBeenCalledWith(['ev-new'], undefined); + expect(mockGetEvents).toHaveBeenCalledWith(['ev-new'], 'acc-1'); expect(result.utcStart).toBe('2026-03-02T08:00:00Z'); const events = useCalendarStore.getState().events; expect(events).toHaveLength(3); @@ -594,7 +594,7 @@ describe('calendar-store', () => { // The third arg is the iMIP scheduling flag — undefined here since the // event has no participants to notify. The fourth is the owning // account — undefined for primary-account events. - expect(mockUpdateEvent).toHaveBeenCalledWith('master', { title: 'New' }, undefined, undefined); + expect(mockUpdateEvent).toHaveBeenCalledWith('master', { title: 'New' }, undefined, 'acc-1'); }); }); @@ -611,7 +611,7 @@ describe('calendar-store', () => { await useCalendarStore.getState().setDefaultCalendar('cal-2'); - expect(mockSetDefaultCalendar).toHaveBeenCalledWith('cal-2', undefined); + expect(mockSetDefaultCalendar).toHaveBeenCalledWith('cal-2', 'acc-1'); const calendars = useCalendarStore.getState().calendars; expect(calendars.find((c) => c.id === 'cal-1')?.isDefault).toBe(false); expect(calendars.find((c) => c.id === 'cal-2')?.isDefault).toBe(true); @@ -655,7 +655,7 @@ describe('calendar-store', () => { ); await useCalendarStore.getState().deleteEvent(master!.id); - expect(mockDeleteEvents).toHaveBeenCalledWith(['ev1'], undefined, undefined); + expect(mockDeleteEvents).toHaveBeenCalledWith(['ev1'], undefined, 'acc-1'); // The visible range was reloaded (empty here). expect(mockQueryEvents).toHaveBeenCalled(); }); @@ -685,10 +685,10 @@ describe('calendar-store', () => { // No overrides yet: a pointer into the missing map would fail, so the // whole map is sent. expect(mockUpdateEvent).toHaveBeenNthCalledWith( - 1, 'ev1', { recurrenceOverrides: { '2026-03-02T09:00:00': { excluded: true } } }, undefined, undefined, + 1, 'ev1', { recurrenceOverrides: { '2026-03-02T09:00:00': { excluded: true } } }, undefined, 'acc-1', ); expect(mockUpdateEvent).toHaveBeenNthCalledWith( - 2, 'ev1', { 'recurrenceOverrides/2026-03-03T09:00:00': { excluded: true } }, undefined, undefined, + 2, 'ev1', { 'recurrenceOverrides/2026-03-03T09:00:00': { excluded: true } }, undefined, 'acc-1', ); }); }); @@ -722,7 +722,7 @@ describe('calendar-store', () => { ); // The UID in cal-1 is linked into cal-2; the one already in cal-2 is skipped. - expect(mockUpdateEvent).toHaveBeenCalledWith('e1', { calendarIds: { 'cal-1': true, 'cal-2': true } }, undefined, undefined); + expect(mockUpdateEvent).toHaveBeenCalledWith('e1', { calendarIds: { 'cal-1': true, 'cal-2': true } }, undefined, 'acc-1'); expect(count).toBe(2); expect(refused).toEqual([]); const created = batch.mock.calls[0][0][0]; @@ -907,7 +907,7 @@ describe('calendar-store', () => { start: '2026-03-02T10:00:00', locations: { l: { name: 'Room 1' } }, sequence: 2, - }, undefined, undefined); + }, undefined, 'acc-1'); expect(mockQueryExpanded).toHaveBeenCalled(); }); @@ -932,14 +932,14 @@ describe('calendar-store', () => { sequence: 2, }, }, - }, undefined, undefined); + }, undefined, 'acc-1'); // Remembered: a delete goes straight to the exclusion. await useCalendarStore.getState().deleteEvent('s2'); expect(mockDeleteEvents).not.toHaveBeenCalled(); expect(mockUpdateEvent).toHaveBeenLastCalledWith('base', { recurrenceOverrides: { '2026-03-09T09:00:00': { excluded: true } }, - }, undefined, undefined); + }, undefined, 'acc-1'); } finally { useCalendarStore.getState().reset(); } @@ -954,7 +954,7 @@ describe('calendar-store', () => { await useCalendarStore.getState().deleteEvent('s1'); - expect(mockDeleteEvents).toHaveBeenCalledWith(['s1'], undefined, undefined); + expect(mockDeleteEvents).toHaveBeenCalledWith(['s1'], undefined, 'acc-1'); expect(useCalendarStore.getState().events.map((e) => e.id)).toEqual(['s2']); }); @@ -967,7 +967,7 @@ describe('calendar-store', () => { await useCalendarStore.getState().deleteEvent('s5'); - expect(mockDeleteEvents).toHaveBeenCalledWith(['plain'], undefined, undefined); + expect(mockDeleteEvents).toHaveBeenCalledWith(['plain'], undefined, 'acc-1'); expect(useCalendarStore.getState().events).toEqual([]); }); @@ -1010,7 +1010,7 @@ describe('calendar-store', () => { await useCalendarStore.getState().rsvpEvent('s1', 'me', 'accepted'); - expect(mockRsvpEvent).toHaveBeenCalledWith('base', 'me', 'accepted', undefined, undefined); + expect(mockRsvpEvent).toHaveBeenCalledWith('base', 'me', 'accepted', undefined, 'acc-1'); }); it('answers just one server occurrence through its synthetic id', async () => { @@ -1036,7 +1036,7 @@ describe('calendar-store', () => { expect(mockUpdateEvent).toHaveBeenCalledWith('s1', { sequence: 4, participants: { ...participants, me: { ...participants.me, participationStatus: 'declined' } }, - }, true, undefined); + }, true, 'acc-1'); expect(useCalendarStore.getState().events[0].participants?.me.participationStatus).toBe('declined'); }); }); @@ -1067,7 +1067,7 @@ describe('calendar-store', () => { participants: { me: { calendarAddress: 'mailto:me@x', participationStatus: 'tentative' } }, }, }, - }, true, undefined); + }, true, 'acc-1'); }); }); @@ -1099,7 +1099,7 @@ describe('calendar-store', () => { // Never the series itself: the change becomes this occurrence's override. expect(mockUpdateEvent).toHaveBeenCalledWith('ev1', { 'recurrenceOverrides/a': { start: '2026-03-02T09:00:00', duration: 'PT30M', title: 'New' }, - }, undefined, undefined); + }, undefined, 'acc-1'); // Refetched: siblings now carry the server's state. const events = useCalendarStore.getState().events; expect(events.length).toBeGreaterThan(0); @@ -1118,7 +1118,7 @@ describe('calendar-store', () => { await useCalendarStore.getState().updateEvent('ev1', { calendarIds: { 'acc-2:cal-9': true } }); - expect(mockUpdateEvent).toHaveBeenCalledWith('ev1', { calendarIds: { 'cal-9': true } }, undefined, undefined); + expect(mockUpdateEvent).toHaveBeenCalledWith('ev1', { calendarIds: { 'cal-9': true } }, undefined, 'acc-1'); }); it('getMasterEvent fetches the master an occurrence points at', async () => { @@ -1161,7 +1161,7 @@ describe('calendar-store', () => { // No organizer on the event: repaired from the invitation's replyTo. expect(mockRsvpEvent).toHaveBeenCalledWith( - 'ev9', 'p1', 'accepted', 'mailto:boss@example.com', undefined, + 'ev9', 'p1', 'accepted', 'mailto:boss@example.com', 'acc-1', ); }); @@ -1195,7 +1195,7 @@ describe('calendar-store', () => { 't1', { progress: 'completed', percentComplete: 100 }, undefined, - undefined, + 'acc-1', ); expect(useCalendarStore.getState().tasks[0].progress).toBe('completed'); }); @@ -1229,7 +1229,7 @@ describe('calendar-store', () => { 't1', { progress: 'needs-action', percentComplete: 0 }, undefined, - undefined, + 'acc-1', ); }); @@ -1381,7 +1381,7 @@ describe('editing one occurrence of a series', () => { participants: { ...participants, me: { ...participants.me, participationStatus: 'tentative' } }, }, }, - }, true, undefined); + }, true, 'acc-1'); }); it('keeps the 0.16.19 fallback payload for a server that rejects synthetic ids', async () => { diff --git a/src/stores/calendar-store.ts b/src/stores/calendar-store.ts index f78c0755..0838ebcd 100644 --- a/src/stores/calendar-store.ts +++ b/src/stores/calendar-store.ts @@ -315,14 +315,53 @@ export function calendarStoreGeneration(): number { return storeGeneration; } -/** A range load the store's reset overtook: what it read belongs to the account before. */ +/** + * A load or write the store's reset overtook: what it read belongs to the + * account before, and what it had still to write would reach the next one. + */ export class CalendarAccountChangedError extends Error { constructor() { - super('The calendar was reset while its events were loading'); + super(t('calendar.account_changed', 'The account changed while the calendar was loading or saving.')); this.name = 'CalendarAccountChangedError'; } } +/** + * The account a write is made for, taken when it starts: the store + * generation and the primary JMAP account the client serves. Calendar and + * event ids repeat across accounts, so a write still under way after an + * account switch must not send its next step, which would reach the next + * account, nor change the store, which shows that account by then. + */ +interface WriteScope { + generation: number; + primaryAccountId: string | null; +} + +function connectedAccountId(): string | null { + return jmapClient.isConnected ? jmapClient.accountId : null; +} + +function writeScope(): WriteScope { + return { generation: storeGeneration, primaryAccountId: connectedAccountId() }; +} + +function inScope(scope: WriteScope): boolean { + return scope.generation === storeGeneration && connectedAccountId() === scope.primaryAccountId; +} + +function assertInScope(scope: WriteScope): void { + if (!inScope(scope)) throw new CalendarAccountChangedError(); +} + +/** + * The JMAP account a write goes to, named on every request: a shared + * calendar's owner, else the primary account the write started in. + */ +function writeAccount(scope: WriteScope, accountId: string | undefined): string | undefined { + return accountId ?? scope.primaryAccountId ?? undefined; +} + /** * fetchCalendars: load the calendar list into the store. True when it did; * false without a session, when the request failed, or when a reset overtook @@ -540,6 +579,7 @@ async function updateBaseEvent( updates: Record, sendSchedulingMessages: boolean | undefined, accountId: string | undefined, + scope: WriteScope, ): Promise { const writesOverrides = Object.keys(updates).some((key) => key.split('/', 1)[0] === 'recurrenceOverrides'); if (writesOverrides && overrideSeriesKeysRejected) { @@ -551,6 +591,7 @@ async function updateBaseEvent( } catch (err) { if (!writesOverrides || !isOverrideSeriesKeyRejection(err)) throw err; overrideSeriesKeysRejected = true; + assertInScope(scope); await apiUpdateEvent(id, withoutOverrideSeriesKeys(updates), sendSchedulingMessages, accountId); } } @@ -569,6 +610,7 @@ async function updateOccurrence( patchFor: (options: OverridePatchOptions) => Partial, sendSchedulingMessages: boolean | undefined, accountId: string | undefined, + scope: WriteScope, ): Promise { if (!syntheticIdRejected) { try { @@ -578,10 +620,11 @@ async function updateOccurrence( if (!isSyntheticIdMutationUnsupported(err)) throw err; syntheticIdRejected = true; } + assertInScope(scope); } const fallback = buildFallbackOverridePatch(instance, patchFor({ seriesKeys: true })); if (!fallback || !instance.baseEventId) throw new Error('Cannot resolve the occurrence to override'); - await updateBaseEvent(instance.baseEventId, fallback, sendSchedulingMessages, accountId); + await updateBaseEvent(instance.baseEventId, fallback, sendSchedulingMessages, accountId, scope); } /** @@ -639,11 +682,14 @@ async function updateOccurrenceInTwoSteps( patch: Partial, sendSchedulingMessages: boolean | undefined, accountId: string | undefined, + scope: WriteScope, ): Promise { - await updateOccurrence(instance, syntheticId, (options) => withNewOverrideDetails(instance, {}, options), false, accountId); + await updateOccurrence(instance, syntheticId, (options) => withNewOverrideDetails(instance, {}, options), false, accountId, scope); + assertInScope(scope); const current = await findServerOccurrence(instance, accountId).catch(() => null); + assertInScope(scope); if (!current) throw new OccurrenceNotFoundAgainError(); - await updateOccurrence(current, current.id, () => patch, sendSchedulingMessages, accountId); + await updateOccurrence(current, current.id, () => patch, sendSchedulingMessages, accountId, scope); } /** @@ -656,8 +702,9 @@ async function updateOccurrenceInTwoSteps( * never shown. 0.16 stores and shows it either way; its series keeps the * same alerts. */ -async function stopSeriesDefaultAlerts(baseId: string, accountId: string | undefined): Promise { +async function stopSeriesDefaultAlerts(baseId: string, accountId: string | undefined, scope: WriteScope): Promise { const [base] = await fetchEvents([baseId], accountId); + assertInScope(scope); if (!base?.useDefaultAlerts) return; const shown = base.alerts ?? {}; await apiUpdateEvent(baseId, { @@ -673,6 +720,7 @@ async function destroyOccurrence( syntheticId: string, sendSchedulingMessages: boolean | undefined, accountId: string | undefined, + scope: WriteScope, ): Promise { if (!syntheticIdRejected) { try { @@ -682,6 +730,7 @@ async function destroyOccurrence( if (!isSyntheticIdMutationUnsupported(err)) throw err; syntheticIdRejected = true; } + assertInScope(scope); } const fallback = buildFallbackExcludePatch(instance); if (!fallback || !instance.baseEventId) throw new Error('Cannot resolve the occurrence to exclude'); @@ -697,11 +746,12 @@ async function updateBrowserOccurrence( target: MutationTarget, updates: Partial, sendSchedulingMessages: boolean | undefined, + scope: WriteScope, ): Promise { const occurrence = target.storeEvent!; const patch = buildFallbackOverridePatch(occurrence, withNewOverrideDetails(occurrence, updates, { seriesKeys: true })); if (!patch) throw new Error('Cannot resolve the occurrence to override'); - await updateBaseEvent(target.realId, patch, sendSchedulingMessages, target.accountId); + await updateBaseEvent(target.realId, patch, sendSchedulingMessages, writeAccount(scope, target.accountId), scope); } /** @@ -714,21 +764,25 @@ async function updateBrowserOccurrenceInTwoSteps( target: MutationTarget, updates: Partial, sendSchedulingMessages: boolean | undefined, + scope: WriteScope, ): Promise { const steps = buildOverrideInTwoSteps(target.storeEvent!, updates); if (!steps) throw new Error('Cannot resolve the occurrence to override'); - await updateBaseEvent(target.realId, steps.quiet, false, target.accountId); - await updateBaseEvent(target.realId, steps.change, sendSchedulingMessages, target.accountId); + const accountId = writeAccount(scope, target.accountId); + await updateBaseEvent(target.realId, steps.quiet, false, accountId, scope); + assertInScope(scope); + await updateBaseEvent(target.realId, steps.change, sendSchedulingMessages, accountId, scope); } /** Delete one occurrence the device expanded by excluding it on its base event. */ async function destroyBrowserOccurrence( target: MutationTarget, sendSchedulingMessages: boolean | undefined, + scope: WriteScope, ): Promise { const patch = buildFallbackExcludePatch(target.storeEvent!); if (!patch) throw new Error('Cannot resolve the occurrence to exclude'); - await apiUpdateEvent(target.realId, patch, sendSchedulingMessages, target.accountId); + await apiUpdateEvent(target.realId, patch, sendSchedulingMessages, writeAccount(scope, target.accountId)); } /** @@ -964,6 +1018,7 @@ export const useCalendarStore = create()( // 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 scope = writeScope(); const calendars = get().calendars; const cal = calendars.find((c) => c.id === calendarId); const accountId = cal?.accountId; @@ -973,19 +1028,22 @@ export const useCalendarStore = create()( event, cal?.originalId || calendarId, schedule, - accountId, + writeAccount(scope, accountId), ); + // Made; the store shows another account by now. + if (!inScope(scope)) return created; // The /set echo lacks server-computed properties (utcStart/utcEnd, the // normalised recurrence rule); re-read the event so it renders at the // right instant, then expand a recurring series across the loaded range // instead of showing a single instance until the next refresh. let full = created; try { - const fetched = (await fetchEvents([created.id], accountId)) ?? []; + const fetched = (await fetchEvents([created.id], writeAccount(scope, accountId))) ?? []; if (fetched[0]) full = fetched[0]; } catch { // Keep the echo; the next refresh reconciles. } + if (!inScope(scope)) return full; // 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. @@ -1004,8 +1062,10 @@ export const useCalendarStore = create()( // A change to an occurrence stays on that occurrence: through its // synthetic id, or as an override on the base event it was expanded // from. Everything else goes to the stored event. + const scope = writeScope(); const target = resolveMutationTarget(get().events, id, 'occurrence'); const { storeEvent, realId, accountId } = target; + const account = writeAccount(scope, accountId); // Remap namespaced (shared-calendar) store ids in calendarIds back to the // raw server ids the owning account knows. const patch: Record = { ...changes }; @@ -1042,15 +1102,16 @@ export const useCalendarStore = create()( : storeEvent.participants); if (target.isOccurrence && storeEvent) { if (stopsDefaultAlerts && storeEvent.baseEventId) { - await stopSeriesDefaultAlerts(storeEvent.baseEventId, accountId); + await stopSeriesDefaultAlerts(storeEvent.baseEventId, account, scope); + assertInScope(scope); } if (twoSteps) { try { - await updateOccurrenceInTwoSteps(storeEvent, realId, patch as Partial, schedule, accountId); + await updateOccurrenceInTwoSteps(storeEvent, realId, patch as Partial, schedule, account, scope); } catch (err) { // Its quiet override is on the server, and the series' occurrences // may have other ids now: a retry must not go to the ones in view. - if (err instanceof OccurrenceNotFoundAgainError) await get().refresh().catch(() => undefined); + if (err instanceof OccurrenceNotFoundAgainError && inScope(scope)) await get().refresh().catch(() => undefined); throw err; } } else { @@ -1059,19 +1120,25 @@ export const useCalendarStore = create()( realId, (options) => withNewOverrideDetails(storeEvent, patch as Partial, options), schedule, - accountId, + account, + scope, ); } } else if (target.isBrowserOccurrence) { - if (stopsDefaultAlerts) await stopSeriesDefaultAlerts(realId, accountId); + if (stopsDefaultAlerts) { + await stopSeriesDefaultAlerts(realId, account, scope); + assertInScope(scope); + } if (twoSteps) { - await updateBrowserOccurrenceInTwoSteps(target, patch as Partial, schedule); + await updateBrowserOccurrenceInTwoSteps(target, patch as Partial, schedule, scope); } else { - await updateBrowserOccurrence(target, patch as Partial, schedule); + await updateBrowserOccurrence(target, patch as Partial, schedule, scope); } } else { - await updateBaseEvent(realId, patch, schedule, accountId); + await updateBaseEvent(realId, patch, schedule, account, scope); } + // Saved; the store shows another account by now, whose events can carry the same ids. + if (!inScope(scope)) return; set({ events: get().events.map((e) => (e.id === id ? { ...e, ...(changes as Partial) } : e)), }); @@ -1089,19 +1156,24 @@ export const useCalendarStore = create()( deleteEvent: async (id) => { // Deleting an occurrence removes just that one: through its synthetic // id, or by excluding it on the base event it was expanded from. + const scope = writeScope(); const target = resolveMutationTarget(get().events, id, 'occurrence'); const { storeEvent, realId, accountId } = target; const touchesSeries = (!!storeEvent && isRecurringSeriesMember(storeEvent)) || hasServerOccurrencesOf(get().events, realId, accountId); const schedule = hasSchedulingParticipants(storeEvent) ? true : undefined; if (target.isOccurrence && storeEvent) { - await destroyOccurrence(storeEvent, realId, schedule, accountId); + await destroyOccurrence(storeEvent, realId, schedule, writeAccount(scope, accountId), scope); + if (!inScope(scope)) return; set({ events: get().events.filter((e) => e.id !== id) }); } else if (target.isBrowserOccurrence) { - await destroyBrowserOccurrence(target, schedule); + await destroyBrowserOccurrence(target, schedule, scope); + if (!inScope(scope)) return; set({ events: get().events.filter((e) => e.id !== id) }); } else { - await apiDeleteEvents([realId], schedule, accountId); + await apiDeleteEvents([realId], schedule, writeAccount(scope, accountId)); + // Deleted; the store shows another account by now, whose events can carry the same ids. + if (!inScope(scope)) return; knownMasters.delete(id); // Destroying a master removes every expanded occurrence of it, not // just the tapped one. @@ -1127,7 +1199,10 @@ export const useCalendarStore = create()( // and its SEQUENCE - a change that raises it from a stale one goes out // lower than the attendees' copies, which 0.16 refuses. const realId = seriesIdOf(event); + const generation = storeGeneration; const fetched = (await fetchEvents([realId], event.accountId)) ?? []; + // Read from the account before a reset: not the next account's master. + if (generation !== storeGeneration) throw new CalendarAccountChangedError(); const master = fetched[0]; if (!master) return null; // Known from here on, so a change to it is sent to its own account under @@ -1147,10 +1222,12 @@ export const useCalendarStore = create()( // isn't in the store; the caller hands it over instead. With scope // 'series' an occurrence answers for its whole series: the stored event // is updated. + const write = writeScope(); const target = resolveMutationTarget(get().events, eventId, scope); const storeEvent = target.storeEvent ?? event; const realId = target.storeEvent || !event ? target.realId : seriesIdOf(event); const accountId = target.storeEvent ? target.accountId : event?.accountId ?? target.accountId; + const account = writeAccount(write, accountId); const occurrence = scope === 'occurrence' && (target.isOccurrence || target.isBrowserOccurrence) ? target.storeEvent ?? null : null; @@ -1164,9 +1241,9 @@ export const useCalendarStore = create()( const patch = patchFor({ seriesKeys: true }); if (!patch) throw new Error('Participant not found on this occurrence'); if (target.isOccurrence) { - await updateOccurrence(occurrence, target.realId, (options) => patchFor(options) ?? {}, true, accountId); + await updateOccurrence(occurrence, target.realId, (options) => patchFor(options) ?? {}, true, account, write); } else { - await updateBrowserOccurrence(target, patch, true); + await updateBrowserOccurrence(target, patch, true, write); } } else { // Repair events that are missing the organizer (e.g. imported ones) so @@ -1175,8 +1252,10 @@ export const useCalendarStore = create()( replyTo?.imip && storeEvent && !storeEvent.organizerCalendarAddress ? replyTo.imip : undefined; - await apiRsvpEvent(realId, participantId, status, repair, accountId); + await apiRsvpEvent(realId, participantId, status, repair, account); } + // Answered; the store shows another account by now, whose events can carry the same ids. + if (!inScope(write)) return; requestDeviceSync(CALENDAR_AUTHORITY); set({ events: get().events.map((e) => { @@ -1196,11 +1275,12 @@ export const useCalendarStore = create()( importEvents: async (events, calendarId) => { if (events.length === 0) return { imported: 0, refused: [] }; + const scope = writeScope(); // 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 accountId = writeAccount(scope, cal?.accountId); const serverCalendarId = cal?.originalId || calendarId; const refused: RefusedImport[] = []; // Stalwart enforces UID uniqueness across calendars (#113): @@ -1211,7 +1291,9 @@ export const useCalendarStore = create()( let linked = 0; try { const existingIds = await queryEvents([], '', '', accountId); + assertInScope(scope); const existing = await getEventIdentities(existingIds, accountId); + assertInScope(scope); const byUid = new Map(); for (const e of existing) if (e.uid) byUid.set(e.uid, e); const fresh: Partial[] = []; @@ -1222,6 +1304,7 @@ export const useCalendarStore = create()( continue; } if (found.calendarIds?.[serverCalendarId]) continue; + assertInScope(scope); try { await apiUpdateEvent( found.id, @@ -1236,7 +1319,8 @@ export const useCalendarStore = create()( } } toCreate = fresh; - } catch { + } catch (err) { + if (err instanceof CalendarAccountChangedError) throw err; // Couldn't dedupe — proceed and let the server reject genuine dupes. } const prepared = toCreate.map(prepareImportedEvent); @@ -1244,6 +1328,8 @@ export const useCalendarStore = create()( try { // Batch in chunks of 50 to avoid oversized requests. for (let i = 0; i < prepared.length; i += 50) { + // The rest would go to the next account's calendar of the same id. + assertInScope(scope); try { const result = await apiBatchCreateEvents(prepared.slice(i, i + 50), serverCalendarId, accountId); count += result.created; @@ -1259,7 +1345,7 @@ export const useCalendarStore = create()( } } finally { // Also when a chunk was refused: show what did get in. - if (count > 0 || linked > 0) { + if ((count > 0 || linked > 0) && inScope(scope)) { requestDeviceSync(CALENDAR_AUTHORITY); await get().refresh(); } @@ -1269,15 +1355,20 @@ export const useCalendarStore = create()( }, createCalendar: async (name, color, description) => { - const created = await apiCreateCalendar(name, color, description); + const scope = writeScope(); + const created = await apiCreateCalendar(name, color, description, writeAccount(scope, undefined)); + // Made in the account before a switch: not one of the next account's. + if (!inScope(scope)) return created; set({ calendars: sortCalendars([...get().calendars, created]) }); requestDeviceSync(CALENDAR_AUTHORITY); return created; }, updateCalendar: async (id, updates) => { + const scope = writeScope(); const cal = get().calendars.find((c) => c.id === id); - await apiUpdateCalendar(cal?.originalId || id, updates, cal?.accountId); + await apiUpdateCalendar(cal?.originalId || id, updates, writeAccount(scope, cal?.accountId)); + if (!inScope(scope)) return; set({ calendars: sortCalendars(get().calendars.map((c) => (c.id === id ? { ...c, ...updates } as Calendar : c))), }); @@ -1287,6 +1378,7 @@ export const useCalendarStore = create()( reorderCalendars: async (orderedIds) => { const updates = sortOrderUpdates(get().calendars, orderedIds); if (updates.length === 0) return; + const scope = writeScope(); const before = get().calendars; const withSortOrders = (sortOrders: Map) => sortCalendars(before.map((c) => sortOrders.has(c.id) ? { ...c, sortOrder: sortOrders.get(c.id)! } : c)); @@ -1295,23 +1387,28 @@ export const useCalendarStore = create()( const saved = new Map(); try { for (const { id, sortOrder } of updates) { + // The rest would renumber the next account's calendars of the same ids. + assertInScope(scope); const cal = before.find((c) => c.id === id); - await apiUpdateCalendar(cal?.originalId || id, { sortOrder }, cal?.accountId); + await apiUpdateCalendar(cal?.originalId || id, { sortOrder }, writeAccount(scope, cal?.accountId)); saved.set(id, sortOrder); } } catch (err) { // Show the order the server now holds: the moves saved before the - // failure stay, the rest are undone. - set({ calendars: withSortOrders(saved) }); + // failure stay, the rest are undone. Not over the next account's list. + if (inScope(scope)) set({ calendars: withSortOrders(saved) }); throw err; } finally { - if (saved.size > 0) requestDeviceSync(CALENDAR_AUTHORITY); + if (saved.size > 0 && inScope(scope)) requestDeviceSync(CALENDAR_AUTHORITY); } }, removeCalendar: async (id) => { + const scope = writeScope(); const cal = get().calendars.find((c) => c.id === id); - await apiDeleteCalendar(cal?.originalId || id, cal?.accountId); + await apiDeleteCalendar(cal?.originalId || id, writeAccount(scope, cal?.accountId)); + // Removed; the next account may have a calendar of the same id. + if (!inScope(scope)) return; set({ calendars: get().calendars.filter((c) => c.id !== id), events: get().events.filter((e) => !e.calendarIds?.[id]), @@ -1322,9 +1419,10 @@ export const useCalendarStore = create()( }, clearCalendarEvents: async (id) => { + const scope = writeScope(); const cal = get().calendars.find((c) => c.id === id); try { - const cleared = await apiClearCalendarEvents(cal?.originalId || id, cal?.accountId); + const cleared = await apiClearCalendarEvents(cal?.originalId || id, writeAccount(scope, cal?.accountId)); requestDeviceSync(CALENDAR_AUTHORITY); return cleared; } finally { @@ -1334,8 +1432,10 @@ export const useCalendarStore = create()( }, shareCalendar: async (id, principalId, rights) => { + const scope = writeScope(); const cal = get().calendars.find((c) => c.id === id); - await apiSetCalendarShare(cal?.originalId || id, principalId, rights, cal?.accountId); + await apiSetCalendarShare(cal?.originalId || id, principalId, rights, writeAccount(scope, cal?.accountId)); + if (!inScope(scope)) return; set({ calendars: get().calendars.map((c) => { if (c.id !== id) return c; @@ -1348,8 +1448,10 @@ export const useCalendarStore = create()( }, setDefaultCalendar: async (id) => { + const scope = writeScope(); const cal = get().calendars.find((c) => c.id === id); - await apiSetDefaultCalendar(cal?.originalId || id, cal?.accountId); + await apiSetDefaultCalendar(cal?.originalId || id, writeAccount(scope, cal?.accountId)); + if (!inScope(scope)) return; set({ calendars: get().calendars.map((c) => { if (c.id === id) return { ...c, isDefault: true }; @@ -1364,20 +1466,24 @@ export const useCalendarStore = create()( }, createTask: async (task, calendarId) => { + const scope = writeScope(); const cal = get().calendars.find((c) => c.id === calendarId); await apiCreateEvent( { ...task, '@type': 'Task' }, cal?.originalId || calendarId, undefined, - cal?.accountId, + writeAccount(scope, cal?.accountId), ); await get().fetchTasks(); }, updateTask: async (id, changes) => { + const scope = writeScope(); const task = get().tasks.find((t) => t.id === id); const realId = task?.originalId || id; - await apiUpdateEvent(realId, changes, undefined, task?.accountId); + await apiUpdateEvent(realId, changes, undefined, writeAccount(scope, task?.accountId)); + // Saved; the next account's tasks can carry the same ids. + if (!inScope(scope)) return; set({ tasks: get().tasks.map((t) => (t.id === id ? { ...t, ...changes } : t)) }); }, @@ -1395,19 +1501,23 @@ export const useCalendarStore = create()( // Flip the checkbox right away and put it back if the server refuses. const patchTask = (changes: Partial) => set({ tasks: get().tasks.map((t) => (t.id === id ? { ...t, ...changes } : t)) }); + const scope = writeScope(); patchTask(next); try { - await apiUpdateEvent(task.originalId || id, next, undefined, task.accountId); + await apiUpdateEvent(task.originalId || id, next, undefined, writeAccount(scope, task.accountId)); } catch (err) { - patchTask({ progress: task.progress, percentComplete: task.percentComplete }); + // Not on the next account's task of the same id. + if (inScope(scope)) patchTask({ progress: task.progress, percentComplete: task.percentComplete }); throw err; } }, deleteTask: async (id) => { + const scope = writeScope(); const task = get().tasks.find((t) => t.id === id); const realId = task?.originalId || id; - await apiDeleteEvents([realId], undefined, task?.accountId); + await apiDeleteEvents([realId], undefined, writeAccount(scope, task?.accountId)); + if (!inScope(scope)) return; set({ tasks: get().tasks.filter((t) => t.id !== id) }); },