From 37440e02b01ece068036884493e445e5dcdbb2f6 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 12 Oct 2026 07:36:46 +0200 Subject: [PATCH] fix: let tasks move into and live in calendars shared with you #1204 --- lib/__tests__/jmap-calendar-tasks.test.ts | 91 ++++++++++++++++ lib/demo/demo-client.ts | 3 +- lib/jmap/client-interface.ts | 3 +- lib/jmap/client.ts | 126 ++++++++++++++++++++-- lib/jmap/types.ts | 5 + stores/__tests__/task-store.test.ts | 11 ++ stores/task-store.ts | 9 +- 7 files changed, 238 insertions(+), 10 deletions(-) diff --git a/lib/__tests__/jmap-calendar-tasks.test.ts b/lib/__tests__/jmap-calendar-tasks.test.ts index 9657aef0d..3d83c6e00 100644 --- a/lib/__tests__/jmap-calendar-tasks.test.ts +++ b/lib/__tests__/jmap-calendar-tasks.test.ts @@ -122,3 +122,94 @@ describe('JMAPClient task operations (#958)', () => { expect(createPayload).not.toHaveProperty('progressUpdated'); }); }); + +describe('JMAPClient tasks in shared calendars (#1204)', () => { + let client: JMAPClient; + let sentCalls: Call[]; + + // Tasks per account as the server stores them (bare ids). + const serverTasks: Record[]> = { + 'acct-1': [{ id: 'own-1', '@type': 'Task', uid: 'uid-own', title: 'Mine', progress: 'needs-action', calendarIds: { 'cal-1': true }, created: '2026-10-01T00:00:00Z', isDraft: false, isOrigin: true, locations: { l1: { '@type': 'Location', name: 'Desk' } } }], + 'acct-2': [{ id: 'shared-1', '@type': 'Task', uid: 'uid-shared', title: 'Theirs', progress: 'needs-action', calendarIds: { 'cal-s': true } }], + }; + + beforeEach(() => { + sentCalls = []; + const session = makeSession(); + session.accounts = { + ...session.accounts, + 'acct-2': { name: 'owner', isPersonal: false, accountCapabilities: { 'urn:ietf:params:jmap:calendars': {} } }, + } as typeof session.accounts; + client = new JMAPClient('https://mail.example.com', 'test', 'pass'); + (client as unknown as { session: unknown }).session = session; + (client as unknown as { accountId: string }).accountId = 'acct-1'; + (client as unknown as { apiUrl: string }).apiUrl = session.apiUrl; + (client as unknown as { accounts: Record }).accounts = session.accounts; + + vi.stubGlobal('fetch', vi.fn(async (_url: string, init?: RequestInit) => { + const body = JSON.parse((init?.body as string) || '{}'); + const calls = (body.methodCalls || []) as [string, Record, string][]; + const methodResponses: [string, Record, string][] = []; + for (const [method, args, tag] of calls) { + sentCalls.push({ method, args }); + const list = serverTasks[args.accountId as string] ?? []; + if (method === 'CalendarEvent/query') { + methodResponses.push([method, { ids: list.map((t) => t.id) }, tag]); + } else if (method === 'CalendarEvent/get') { + const ids = args.ids as string[]; + const found = list.filter((t) => ids.includes(t.id as string)); + // A freshly created task is fetched back by its new id. + const fresh = ids.includes('new-id') ? [{ id: 'new-id', '@type': 'Task', uid: 'uid-own', title: 'Mine', progress: 'needs-action', calendarIds: { 'cal-s': true } }] : []; + methodResponses.push([method, { list: [...found, ...fresh], notFound: [] }, tag]); + } else if (method === 'CalendarEvent/set') { + const created = args.create ? { 'new-task': { id: 'new-id' } } : {}; + const updated = Object.fromEntries(Object.keys((args.update as object) ?? {}).map((id) => [id, null])); + methodResponses.push([method, { created, updated, destroyed: args.destroy ?? [] }, tag]); + } + } + return jsonResponse({ methodResponses }); + })); + }); + + it('lists tasks from shared accounts with namespaced ids and calendarIds', async () => { + const tasks = await client.getCalendarTasks(); + const shared = tasks.find((t) => t.title === 'Theirs'); + expect(shared?.id).toBe('acct-2:shared-1'); + expect(shared?.calendarIds).toEqual({ 'acct-2:cal-s': true }); + expect(tasks.find((t) => t.title === 'Mine')?.id).toBe('own-1'); + }); + + it('moves a task into a shared calendar by creating it there and destroying the original', async () => { + const newId = await client.updateCalendarTask('own-1', { title: 'Mine', calendarIds: { 'acct-2:cal-s': true } }); + + const sets = sentCalls.filter((c) => c.method === 'CalendarEvent/set'); + expect(sets).toHaveLength(2); + const [create, destroy] = sets; + expect(create.args.accountId).toBe('acct-2'); + const payload = (create.args.create as Record>)['new-task']; + expect(payload.calendarIds).toEqual({ 'cal-s': true }); + expect(payload.uid).toBe('uid-own'); + expect(payload.locations).toEqual({ l1: { '@type': 'Location', name: 'Desk' } }); + for (const key of ['id', 'created', 'isDraft', 'isOrigin']) expect(payload).not.toHaveProperty(key); + expect(destroy.args).toMatchObject({ accountId: 'acct-1', destroy: ['own-1'] }); + expect(newId).toBe('acct-2:new-id'); + }); + + it('updates a shared task in place in its own account with real ids', async () => { + const id = await client.updateCalendarTask('acct-2:shared-1', { progress: 'completed', calendarIds: { 'acct-2:cal-s': true } }); + + const sets = sentCalls.filter((c) => c.method === 'CalendarEvent/set'); + expect(sets).toHaveLength(1); + expect(sets[0].args.accountId).toBe('acct-2'); + expect(sets[0].args.update).toEqual({ 'shared-1': { progress: 'completed', calendarIds: { 'cal-s': true } } }); + expect(id).toBe('acct-2:shared-1'); + }); + + it('creates a task in a shared calendar in the owning account', async () => { + const created = await client.createCalendarTask({ title: 'New', calendarIds: { 'acct-2:cal-s': true } }); + const set = sentCalls.find((c) => c.method === 'CalendarEvent/set'); + expect(set?.args.accountId).toBe('acct-2'); + expect((set?.args.create as Record>)['new-task'].calendarIds).toEqual({ 'cal-s': true }); + expect(created.id).toBe('acct-2:new-id'); + }); +}); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index e107a513b..1096cc907 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -1093,9 +1093,10 @@ export class DemoJMAPClient implements IJMAPClient { return full; } - async updateCalendarTask(taskId: string, updates: Partial): Promise { + async updateCalendarTask(taskId: string, updates: Partial): Promise { const task = this.data.calendarTasks.find(t => t.id === taskId); if (task) Object.assign(task, updates, { updated: new Date().toISOString() }); + return taskId; } async deleteCalendarTask(taskId: string): Promise { diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 6fcba87b2..670dd5838 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -476,7 +476,8 @@ export interface IJMAPClient { // ── Calendar Tasks ──────────────────────────────────────────── getCalendarTasks(calendarIds?: string[], targetAccountId?: string): Promise; createCalendarTask(task: Partial, targetAccountId?: string): Promise; - updateCalendarTask(taskId: string, updates: Partial, targetAccountId?: string): Promise; + /** Resolves to the task's id afterwards; it changes when the task moves to another account. */ + updateCalendarTask(taskId: string, updates: Partial, targetAccountId?: string): Promise; deleteCalendarTask(taskId: string, targetAccountId?: string): Promise; // ── Sharing (RFC 9670 Principals) ───────────────────────────── diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 2ae2d81e0..5feca38d8 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -7577,8 +7577,72 @@ export class JMAPClient implements IJMAPClient { * CalendarEvent/query (which returns events *and* tasks), fetch in * /get-sized batches, and detect tasks client-side. */ + // Store ids for shared calendars and their events and tasks are namespaced + // `accountId:id`; bare ids belong to the primary calendars account. JMAP ids + // never contain ':'. + private splitCalendarObjectId(id: string): { accountId: string; id: string } { + const sep = id.indexOf(':'); + if (sep < 0) return { accountId: this.getCalendarsAccountId(), id }; + return { accountId: id.slice(0, sep), id: id.slice(sep + 1) }; + } + + private namespaceCalendarTask(task: CalendarTask, accountId: string): CalendarTask { + if (accountId === this.getCalendarsAccountId()) return task; + return { + ...task, + id: `${accountId}:${task.id}`, + calendarIds: Object.fromEntries( + Object.entries(task.calendarIds || {}).map(([calId, v]) => [`${accountId}:${calId}`, v]) + ), + originalId: task.id, + accountId, + isShared: true, + }; + } + + // Maps the (possibly namespaced) calendarIds of a task to the account that + // owns those calendars and their real ids there. + private resolveTaskCalendarIds(calendarIds: Record): { accountId: string; calendarIds: Record } { + let accountId: string | undefined; + const real: Record = {}; + for (const [key, value] of Object.entries(calendarIds)) { + const parts = this.splitCalendarObjectId(key); + if (accountId && parts.accountId !== accountId) { + throw new Error("A task can only belong to calendars of one account"); + } + accountId = parts.accountId; + real[parts.id] = value; + } + return { accountId: accountId ?? this.getCalendarsAccountId(), calendarIds: real }; + } + + /** + * Without `targetAccountId`, collects tasks from every calendar account, not + * just the primary one, so tasks in calendars shared with the user show up + * too. Shared tasks come back namespaced like shared events. (#1204) + */ async getCalendarTasks(calendarIds?: string[], targetAccountId?: string): Promise { - const accountId = targetAccountId || this.getCalendarsAccountId(); + if (targetAccountId) return this.getAccountCalendarTasks(calendarIds, targetAccountId); + + const primaryId = this.getCalendarsAccountId(); + const tasks: CalendarTask[] = []; + for (const accountId of this.getCalendarCapableAccountIds()) { + if (accountId !== primaryId && this.calendarAccessDenied.has(accountId)) continue; + let accountCalendarIds: string[] | undefined; + if (calendarIds && calendarIds.length > 0) { + accountCalendarIds = calendarIds + .map((id) => this.splitCalendarObjectId(id)) + .filter((parts) => parts.accountId === accountId) + .map((parts) => parts.id); + if (accountCalendarIds.length === 0) continue; + } + const accountTasks = await this.getAccountCalendarTasks(accountCalendarIds, accountId); + tasks.push(...accountTasks.map((task) => this.namespaceCalendarTask(task, accountId))); + } + return tasks; + } + + private async getAccountCalendarTasks(calendarIds: string[] | undefined, accountId: string): Promise { debug.group('CalendarTask/fetch', 'tasks'); debug.log('tasks', 'CalendarTask/fetch start', { accountId, calendarIds: calendarIds || 'all' }); @@ -7670,8 +7734,15 @@ export class JMAPClient implements IJMAPClient { } async createCalendarTask(task: Partial, targetAccountId?: string): Promise { - const accountId = targetAccountId || this.getCalendarsAccountId(); - const { '@type': _type, progressUpdated: _pu, ...taskData } = task as Partial & { progressUpdated?: string | null }; + let accountId = targetAccountId || this.getCalendarsAccountId(); + const { '@type': _type, progressUpdated: _pu, id: _id, originalId: _oi, accountId: _ai, isShared: _is, ...taskData } = task as Partial & { progressUpdated?: string | null }; + if (!targetAccountId && taskData.calendarIds && Object.keys(taskData.calendarIds).length > 0) { + // A task created in a shared calendar has to be created in the account + // that owns the calendar. (#1204) + const resolved = this.resolveTaskCalendarIds(taskData.calendarIds); + accountId = resolved.accountId; + taskData.calendarIds = resolved.calendarIds; + } const cleanTask = { ...taskData, '@type': 'Task' }; debug.group('CalendarTask/create', 'tasks'); @@ -7723,7 +7794,10 @@ export class JMAPClient implements IJMAPClient { const notFound = getResponse.methodResponses[0][1].notFound || []; debug.log('calendar', 'CalendarTask/create get response', { found: list.length, notFound }); if (list[0]) { - const created = normalizeCalendarTask({ ...list[0], '@type': 'Task' as const } as CalendarTask); + const created = this.namespaceCalendarTask( + normalizeCalendarTask({ ...list[0], '@type': 'Task' as const } as CalendarTask), + accountId, + ); debug.log('tasks', 'CalendarTask/create final task object', { id: created.id, uid: created.uid, @@ -7745,12 +7819,50 @@ export class JMAPClient implements IJMAPClient { throw new Error("Failed to fetch created task"); } - async updateCalendarTask(taskId: string, updates: Partial, targetAccountId?: string): Promise { - await this.updateCalendarEvent(taskId, updates as unknown as Partial, false, targetAccountId); + /** + * Updates a task and resolves to its id afterwards. Moving a task into a + * calendar of another account (a shared calendar) recreates it there, so + * the id changes. (#1204) + */ + async updateCalendarTask(taskId: string, updates: Partial, targetAccountId?: string): Promise { + const source = targetAccountId ? { accountId: targetAccountId, id: taskId } : this.splitCalendarObjectId(taskId); + let patch = updates; + if (!targetAccountId && updates.calendarIds) { + const dest = this.resolveTaskCalendarIds(updates.calendarIds); + patch = { ...updates, calendarIds: dest.calendarIds }; + if (dest.accountId !== source.accountId) { + return this.moveCalendarTaskToAccount(source, dest.accountId, patch); + } + } + await this.updateCalendarEvent(source.id, patch as unknown as Partial, false, source.accountId); + return taskId; + } + + // JMAP has no cross-account CalendarEvent move: create the task in the + // destination account, then destroy the original. Creating first means a + // failed create leaves the task where it was. + private async moveCalendarTaskToAccount( + source: { accountId: string; id: string }, + destAccountId: string, + patch: Partial, + ): Promise { + const response = await this.request([ + ["CalendarEvent/get", { accountId: source.accountId, ids: [source.id] }, "0"] + ], this.calendarUsing()); + const [method, result] = response.methodResponses?.[0] ?? []; + const original = method === "CalendarEvent/get" ? result.list?.[0] : undefined; + if (!original) throw new Error("Task not found"); + + // Drop what the server sets itself; keep the uid so it stays the same task. + const { id: _id, created: _cr, updated: _up, sequence: _sq, isDraft: _idr, isOrigin: _io, baseEventId: _be, blobId: _bl, utcStart: _us, utcEnd: _ue, ...rest } = original as Record; + const created = await this.createCalendarTask({ ...rest, ...patch } as Partial, destAccountId); + await this.deleteCalendarEvent(source.id, false, source.accountId); + return created.id; } async deleteCalendarTask(taskId: string, targetAccountId?: string): Promise { - await this.deleteCalendarEvent(taskId, false, targetAccountId); + const target = targetAccountId ? { accountId: targetAccountId, id: taskId } : this.splitCalendarObjectId(taskId); + await this.deleteCalendarEvent(target.id, false, target.accountId); } // ─── JMAP FileNode methods (draft-ietf-jmap-filenode) ─── diff --git a/lib/jmap/types.ts b/lib/jmap/types.ts index 99a7aa4b1..1b0620363 100644 --- a/lib/jmap/types.ts +++ b/lib/jmap/types.ts @@ -830,6 +830,11 @@ export interface CalendarTask { recurrenceRules: CalendarRecurrenceRule[] | null; alerts: Record | null; relatedTo: Record | null; + // Client-only, set on tasks from a shared calendar account. Their `id` and + // `calendarIds` are namespaced `accountId:id` like shared events. + originalId?: string; + accountId?: string; + isShared?: boolean; } /** diff --git a/stores/__tests__/task-store.test.ts b/stores/__tests__/task-store.test.ts index e2074c57b..73adad370 100644 --- a/stores/__tests__/task-store.test.ts +++ b/stores/__tests__/task-store.test.ts @@ -121,6 +121,17 @@ describe('task-store', () => { expect(updated?.progress).toBe('needs-action'); }); + it('reloads the list when a move to a shared calendar gives the task a new id (#1204)', async () => { + useTaskStore.setState({ tasks: [makeTask({ id: 't5' })], selectedTaskId: 't5' }); + (mockClient.updateCalendarTask as ReturnType).mockResolvedValue('acct-2:new'); + (mockClient.getCalendarTasks as ReturnType).mockResolvedValue([makeTask({ id: 'acct-2:new' })]); + + await useTaskStore.getState().updateTask(mockClient as IJMAPClient, 't5', { calendarIds: { 'acct-2:cal-s': true } }); + + expect(useTaskStore.getState().tasks.map((t) => t.id)).toEqual(['acct-2:new']); + expect(useTaskStore.getState().selectedTaskId).toBe('acct-2:new'); + }); + it('deletes a task and resets selectedTaskId if selected', async () => { const task = makeTask({ id: 't4' }); useTaskStore.setState({ tasks: [task], selectedTaskId: 't4' }); diff --git a/stores/task-store.ts b/stores/task-store.ts index b14f9e191..e595d413f 100644 --- a/stores/task-store.ts +++ b/stores/task-store.ts @@ -65,7 +65,14 @@ export const useTaskStore = create((set, get) => ({ }, updateTask: async (client, id, updates) => { - await client.updateCalendarTask(id, updates); + const newId = await client.updateCalendarTask(id, updates); + if (newId && newId !== id) { + // Moved into a calendar of another account: the server recreated it + // under a new id, so reload rather than patch the old entry. (#1204) + set({ selectedTaskId: get().selectedTaskId === id ? newId : get().selectedTaskId }); + await get().fetchTasks(client); + return; + } set({ tasks: get().tasks.map(t => t.id === id ? { ...t, ...updates, updated: new Date().toISOString() } : t), });