mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: omit progressUpdated from task completion payload (#958)
Stalwart Mail Server strictly validates JSCalendar task properties on CalendarEvent/set and rejects updates containing top-level progressUpdated with invalidProperties (RFC 8984 §5.2.2 only defined progressUpdated on Participant objects, obsoleted in JSCalendar 2.0). - Omit progressUpdated from toggleTaskComplete in stores/task-store.ts - Remove progressUpdated from CALENDAR_TASK_PROPERTIES in lib/jmap/client.ts - Defensively strip progressUpdated in updateCalendarEvent and createCalendarTask - Make progressUpdated optional in CalendarTask interface - Add unit tests for task-store and JMAP client task methods
This commit is contained in:
@@ -0,0 +1,124 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { JMAPClient } from '../jmap/client';
|
||||
import type { CalendarTask } from '../jmap/types';
|
||||
|
||||
function makeSession() {
|
||||
return {
|
||||
capabilities: { 'urn:ietf:params:jmap:core': {}, 'urn:ietf:params:jmap:calendars': {} },
|
||||
accounts: {
|
||||
'acct-1': { name: 'test', isPersonal: true, accountCapabilities: { 'urn:ietf:params:jmap:calendars': {} } },
|
||||
},
|
||||
primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1', 'urn:ietf:params:jmap:calendars': 'acct-1' },
|
||||
apiUrl: 'https://mail.example.com/jmap/api',
|
||||
downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}',
|
||||
uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/',
|
||||
eventSourceUrl: '',
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(body: unknown): Response {
|
||||
return new Response(JSON.stringify(body), { status: 200, headers: { 'Content-Type': 'application/json' } });
|
||||
}
|
||||
|
||||
type Call = { method: string; args: Record<string, unknown> };
|
||||
|
||||
describe('JMAPClient task operations (#958)', () => {
|
||||
let client: JMAPClient;
|
||||
let sentCalls: Call[];
|
||||
|
||||
beforeEach(() => {
|
||||
sentCalls = [];
|
||||
client = new JMAPClient('https://mail.example.com', 'test', 'pass');
|
||||
// Simulate active session
|
||||
(client as unknown as { session: unknown; accountId: string; apiUrl: string }).session = makeSession();
|
||||
(client as unknown as { accountId: string }).accountId = 'acct-1';
|
||||
(client as unknown as { apiUrl: string }).apiUrl = 'https://mail.example.com/jmap/api';
|
||||
(client as unknown as { accounts: Record<string, unknown> }).accounts = makeSession().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, unknown>, string][];
|
||||
const methodResponses: [string, Record<string, unknown>, string][] = [];
|
||||
|
||||
for (const [method, args, tag] of calls) {
|
||||
sentCalls.push({ method, args });
|
||||
|
||||
if (method === 'CalendarEvent/set') {
|
||||
const update = args.update as Record<string, unknown> | undefined;
|
||||
const create = args.create as Record<string, unknown> | undefined;
|
||||
const updated: Record<string, unknown> = {};
|
||||
const created: Record<string, unknown> = {};
|
||||
|
||||
if (update) {
|
||||
for (const id of Object.keys(update)) {
|
||||
updated[id] = { id };
|
||||
}
|
||||
}
|
||||
if (create) {
|
||||
for (const id of Object.keys(create)) {
|
||||
created[id] = { id: 'created-id-123' };
|
||||
}
|
||||
}
|
||||
|
||||
methodResponses.push(['CalendarEvent/set', { updated, created, notUpdated: {}, notCreated: {} }, tag]);
|
||||
} else if (method === 'CalendarEvent/get') {
|
||||
methodResponses.push([
|
||||
'CalendarEvent/get',
|
||||
{
|
||||
list: [
|
||||
{
|
||||
id: 'created-id-123',
|
||||
'@type': 'Task',
|
||||
uid: 'uid-123',
|
||||
title: 'Task Title',
|
||||
progress: 'needs-action',
|
||||
calendarIds: { 'cal-1': true },
|
||||
},
|
||||
],
|
||||
notFound: [],
|
||||
},
|
||||
tag,
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
return jsonResponse({ methodResponses });
|
||||
}));
|
||||
});
|
||||
|
||||
it('updateCalendarTask strips progressUpdated before sending CalendarEvent/set (#958)', async () => {
|
||||
const updates: Partial<CalendarTask> & { progressUpdated?: string | null } = {
|
||||
progress: 'completed',
|
||||
progressUpdated: '2026-09-06T12:00:00Z',
|
||||
};
|
||||
|
||||
await client.updateCalendarTask('task-123', updates as Partial<CalendarTask>);
|
||||
|
||||
const setCall = sentCalls.find((c) => c.method === 'CalendarEvent/set');
|
||||
expect(setCall).toBeDefined();
|
||||
|
||||
const updatePayload = (setCall?.args.update as Record<string, Record<string, unknown>>)?.['task-123'];
|
||||
expect(updatePayload).toBeDefined();
|
||||
expect(updatePayload.progress).toBe('completed');
|
||||
expect(updatePayload).not.toHaveProperty('progressUpdated');
|
||||
});
|
||||
|
||||
it('createCalendarTask strips progressUpdated before sending CalendarEvent/set', async () => {
|
||||
const task: Partial<CalendarTask> & { progressUpdated?: string | null } = {
|
||||
title: 'New Task',
|
||||
progress: 'needs-action',
|
||||
progressUpdated: '2026-09-06T12:00:00Z',
|
||||
};
|
||||
|
||||
await client.createCalendarTask(task as Partial<CalendarTask>);
|
||||
|
||||
const setCall = sentCalls.find((c) => c.method === 'CalendarEvent/set');
|
||||
expect(setCall).toBeDefined();
|
||||
|
||||
const createPayload = (setCall?.args.create as Record<string, Record<string, unknown>>)?.['new-task'];
|
||||
expect(createPayload).toBeDefined();
|
||||
expect(createPayload['@type']).toBe('Task');
|
||||
expect(createPayload.title).toBe('New Task');
|
||||
expect(createPayload).not.toHaveProperty('progressUpdated');
|
||||
});
|
||||
});
|
||||
+2
-3
@@ -344,7 +344,6 @@ const CALENDAR_TASK_PROPERTIES = [
|
||||
'utcStart',
|
||||
'utcEnd',
|
||||
'progress',
|
||||
'progressUpdated',
|
||||
'priority',
|
||||
'privacy',
|
||||
'color',
|
||||
@@ -6516,7 +6515,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
const accountId = targetAccountId || this.getCalendarsAccountId();
|
||||
|
||||
// Strip client-only and server-immutable fields before sending to JMAP
|
||||
const { id: _id, uid: _uid, '@type': _typ, created: _cr, updated: _up, sequence: _sq, isOrigin: _io, isDraft: _idr, originalId: _oi, baseEventId: _be, originalCalendarIds: _oc, accountId: _ai, accountName: _an, isShared: _is, ...cleanUpdates } = updates as CalendarEvent;
|
||||
const { id: _id, uid: _uid, '@type': _typ, created: _cr, updated: _up, sequence: _sq, isOrigin: _io, isDraft: _idr, originalId: _oi, baseEventId: _be, originalCalendarIds: _oc, accountId: _ai, accountName: _an, isShared: _is, progressUpdated: _pu, ...cleanUpdates } = updates as CalendarEvent & { progressUpdated?: string | null };
|
||||
cleanRecurrenceRules(cleanUpdates as unknown as Record<string, unknown>);
|
||||
|
||||
const setArgs: Record<string, unknown> = {
|
||||
@@ -6770,7 +6769,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
async createCalendarTask(task: Partial<CalendarTask>, targetAccountId?: string): Promise<CalendarTask> {
|
||||
const accountId = targetAccountId || this.getCalendarsAccountId();
|
||||
const { '@type': _type, ...taskData } = task;
|
||||
const { '@type': _type, progressUpdated: _pu, ...taskData } = task as Partial<CalendarTask> & { progressUpdated?: string | null };
|
||||
const cleanTask = { ...taskData, '@type': 'Task' };
|
||||
|
||||
debug.group('CalendarTask/create', 'tasks');
|
||||
|
||||
+1
-1
@@ -792,7 +792,7 @@ export interface CalendarTask {
|
||||
timeZone: string | null;
|
||||
showWithoutTime: boolean;
|
||||
progress: 'needs-action' | 'in-process' | 'completed' | 'cancelled';
|
||||
progressUpdated: string | null;
|
||||
progressUpdated?: string | null;
|
||||
priority: number;
|
||||
privacy: 'public' | 'private' | 'secret';
|
||||
keywords: Record<string, boolean> | null;
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||
import { useTaskStore } from '../task-store';
|
||||
import type { CalendarTask } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
|
||||
function makeTask(overrides: Partial<CalendarTask> = {}): CalendarTask {
|
||||
return {
|
||||
id: 'task-1',
|
||||
calendarIds: { 'cal-1': true },
|
||||
'@type': 'Task',
|
||||
uid: 'task-uid-1',
|
||||
title: 'Test task',
|
||||
description: '',
|
||||
due: '2026-09-08T10:00:00',
|
||||
start: null,
|
||||
duration: null,
|
||||
timeZone: null,
|
||||
showWithoutTime: false,
|
||||
progress: 'needs-action',
|
||||
priority: 0,
|
||||
privacy: 'public',
|
||||
keywords: null,
|
||||
categories: null,
|
||||
color: null,
|
||||
created: '2026-09-01T10:00:00Z',
|
||||
updated: '2026-09-01T10:00:00Z',
|
||||
recurrenceRules: null,
|
||||
alerts: null,
|
||||
relatedTo: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('task-store', () => {
|
||||
let mockClient: Partial<IJMAPClient>;
|
||||
|
||||
beforeEach(() => {
|
||||
useTaskStore.setState({
|
||||
tasks: [],
|
||||
selectedTaskId: null,
|
||||
filter: 'all',
|
||||
showCompleted: false,
|
||||
isLoading: false,
|
||||
error: null,
|
||||
});
|
||||
|
||||
mockClient = {
|
||||
getCalendarTasks: vi.fn().mockResolvedValue([makeTask()]),
|
||||
createCalendarTask: vi.fn().mockImplementation(async (t) => makeTask({ id: 'task-created', ...t })),
|
||||
updateCalendarTask: vi.fn().mockResolvedValue(undefined),
|
||||
deleteCalendarTask: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
});
|
||||
|
||||
describe('toggleTaskComplete', () => {
|
||||
it('toggles needs-action to completed without sending progressUpdated in payload (#958)', async () => {
|
||||
const task = makeTask({ id: 't1', progress: 'needs-action' });
|
||||
useTaskStore.setState({ tasks: [task] });
|
||||
|
||||
await useTaskStore.getState().toggleTaskComplete(mockClient as IJMAPClient, task);
|
||||
|
||||
expect(mockClient.updateCalendarTask).toHaveBeenCalledTimes(1);
|
||||
const [calledId, calledUpdates] = (mockClient.updateCalendarTask as ReturnType<typeof vi.fn>).mock.calls[0];
|
||||
expect(calledId).toBe('t1');
|
||||
expect(calledUpdates).toEqual({
|
||||
progress: 'completed',
|
||||
});
|
||||
// Stalwart rejects progressUpdated with invalidProperties error
|
||||
expect(calledUpdates).not.toHaveProperty('progressUpdated');
|
||||
|
||||
const updatedTask = useTaskStore.getState().tasks.find((t) => t.id === 't1');
|
||||
expect(updatedTask?.progress).toBe('completed');
|
||||
});
|
||||
|
||||
it('toggles completed back to needs-action without sending progressUpdated in payload', async () => {
|
||||
const task = makeTask({ id: 't2', progress: 'completed' });
|
||||
useTaskStore.setState({ tasks: [task] });
|
||||
|
||||
await useTaskStore.getState().toggleTaskComplete(mockClient as IJMAPClient, task);
|
||||
|
||||
expect(mockClient.updateCalendarTask).toHaveBeenCalledTimes(1);
|
||||
const [calledId, calledUpdates] = (mockClient.updateCalendarTask as ReturnType<typeof vi.fn>).mock.calls[0];
|
||||
expect(calledId).toBe('t2');
|
||||
expect(calledUpdates).toEqual({
|
||||
progress: 'needs-action',
|
||||
});
|
||||
expect(calledUpdates).not.toHaveProperty('progressUpdated');
|
||||
|
||||
const updatedTask = useTaskStore.getState().tasks.find((t) => t.id === 't2');
|
||||
expect(updatedTask?.progress).toBe('needs-action');
|
||||
});
|
||||
});
|
||||
|
||||
describe('CRUD operations', () => {
|
||||
it('fetches tasks and updates store state', async () => {
|
||||
await useTaskStore.getState().fetchTasks(mockClient as IJMAPClient, ['cal-1']);
|
||||
|
||||
expect(mockClient.getCalendarTasks).toHaveBeenCalledWith(['cal-1']);
|
||||
expect(useTaskStore.getState().tasks).toHaveLength(1);
|
||||
expect(useTaskStore.getState().isLoading).toBe(false);
|
||||
expect(useTaskStore.getState().error).toBeNull();
|
||||
});
|
||||
|
||||
it('creates a task and appends to store', async () => {
|
||||
const created = await useTaskStore.getState().createTask(mockClient as IJMAPClient, { title: 'New Task' });
|
||||
|
||||
expect(mockClient.createCalendarTask).toHaveBeenCalledWith({ title: 'New Task' });
|
||||
expect(created.id).toBe('task-created');
|
||||
expect(useTaskStore.getState().tasks).toContainEqual(created);
|
||||
});
|
||||
|
||||
it('updates a task and preserves unchanged fields', async () => {
|
||||
const task = makeTask({ id: 't3', title: 'Original Title' });
|
||||
useTaskStore.setState({ tasks: [task] });
|
||||
|
||||
await useTaskStore.getState().updateTask(mockClient as IJMAPClient, 't3', { title: 'Updated Title' });
|
||||
|
||||
expect(mockClient.updateCalendarTask).toHaveBeenCalledWith('t3', { title: 'Updated Title' });
|
||||
const updated = useTaskStore.getState().tasks.find((t) => t.id === 't3');
|
||||
expect(updated?.title).toBe('Updated Title');
|
||||
expect(updated?.progress).toBe('needs-action');
|
||||
});
|
||||
|
||||
it('deletes a task and resets selectedTaskId if selected', async () => {
|
||||
const task = makeTask({ id: 't4' });
|
||||
useTaskStore.setState({ tasks: [task], selectedTaskId: 't4' });
|
||||
|
||||
await useTaskStore.getState().deleteTask(mockClient as IJMAPClient, 't4');
|
||||
|
||||
expect(mockClient.deleteCalendarTask).toHaveBeenCalledWith('t4');
|
||||
expect(useTaskStore.getState().tasks).toHaveLength(0);
|
||||
expect(useTaskStore.getState().selectedTaskId).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -83,7 +83,6 @@ export const useTaskStore = create<TaskStore>((set, get) => ({
|
||||
const newProgress = task.progress === 'completed' ? 'needs-action' : 'completed';
|
||||
const updates: Partial<CalendarTask> = {
|
||||
progress: newProgress,
|
||||
progressUpdated: new Date().toISOString(),
|
||||
};
|
||||
await client.updateCalendarTask(task.id, updates);
|
||||
set({
|
||||
|
||||
Reference in New Issue
Block a user