fix: report calendar deletes and imports the server refused

`deleteEvents` never looked at the `CalendarEvent/set` answer, so a
refused delete (read-only calendar, method error) removed the event
from the screen and said nothing until the next refresh brought it
back. Clearing a calendar ignored `notDestroyed`/`notUpdated`, and an
import the server refused entirely returned 0, which the invitation
banner showed as "Already in your calendar".

- check the destroy result in `deleteEvents` and `clearCalendarEvents`
  with `assertSetResult`, and the unlink result as well
- throw from `batchCreateEvents` when nothing was created and the
  server refused events; a partial import still returns its count
- show an alert when deleting an event or a task fails, and refresh
  after a partly failed import or clear
This commit is contained in:
Linus Rath
2026-09-24 19:12:02 +02:00
parent c0582ab950
commit a6ef0abc2a
5 changed files with 120 additions and 28 deletions
+1
View File
@@ -80,6 +80,7 @@
"tasks": {
"add_placeholder": "Add a task…",
"add_time": "Add time",
"delete_error": "Failed to delete task",
"update_error": "Failed to update task"
}
},
+59
View File
@@ -16,6 +16,8 @@ import {
createEvent,
updateEvent,
deleteEvents,
batchCreateEvents,
clearCalendarEvents,
findEventsByUid,
toLocalDateTime,
} from '../calendar';
@@ -378,5 +380,62 @@ describe('calendar operations', () => {
const call = mockRequest.mock.calls[0][0][0];
expect(call[1].destroy).toEqual(['ev1']);
});
it('throws when the server refuses to destroy (B24)', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['CalendarEvent/set', {
destroyed: [],
notDestroyed: { ev1: { type: 'forbidden', description: 'read-only calendar' } },
}, '0']],
});
await expect(deleteEvents(['ev1'])).rejects.toThrow(/destroy event ev1.*read-only calendar/);
});
it('throws on a method-level error', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['error', { type: 'accountNotFound' }, '0']],
});
await expect(deleteEvents(['ev1'])).rejects.toThrow('accountNotFound');
});
});
describe('batchCreateEvents', () => {
it('returns how many were created when only some were refused', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['CalendarEvent/set', {
created: { 'evt-0': { id: 'a' } },
notCreated: { 'evt-1': { type: 'invalidProperties' } },
}, '0']],
});
expect(await batchCreateEvents([{ title: 'A' }, { title: 'B' }], 'cal-1')).toBe(1);
});
it('throws when the server refused every event (B24)', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['CalendarEvent/set', {
notCreated: { 'evt-0': { type: 'invalidProperties', properties: ['participants'] } },
}, '0']],
});
await expect(batchCreateEvents([{ title: 'A' }], 'cal-1')).rejects.toThrow(/invalidProperties/);
});
});
describe('clearCalendarEvents', () => {
it('throws when the server refuses to destroy the events (B24)', async () => {
mockRequest
.mockResolvedValueOnce({ methodResponses: [['CalendarEvent/query', { ids: ['ev1'] }, '0']] })
.mockResolvedValueOnce({
methodResponses: [['CalendarEvent/get', { list: [{ id: 'ev1', calendarIds: { 'cal-1': true } }] }, '0']],
})
.mockResolvedValueOnce({
methodResponses: [['CalendarEvent/set', { notDestroyed: { ev1: { type: 'forbidden' } } }, '0']],
});
await expect(clearCalendarEvents('cal-1')).rejects.toThrow(/forbidden/);
});
});
});
+19 -6
View File
@@ -1,6 +1,7 @@
import { jmapClient } from './jmap-client';
import { CAPABILITIES } from './types';
import type { Calendar, CalendarEvent, CalendarRights } from './types';
import { assertSetResult } from './jmap-result';
import { getEffectiveTimeZone } from '../lib/calendar-timezone';
import { SCAN_PROPERTIES, type ScannedCalendarObject } from '../lib/calendar-component-detection';
@@ -447,7 +448,9 @@ export async function createEvent(
return normalizeRecurrenceProperties({ ...payload, ...created } as CalendarEvent);
}
// Batch-create many events in one CalendarEvent/set. Returns the number created.
// Batch-create many events in one CalendarEvent/set. Returns the number
// created; throws when the server refused every one of them, so a refusal
// doesn't read as "nothing new to import".
export async function batchCreateEvents(
events: Partial<CalendarEvent>[],
calendarId: string,
@@ -465,8 +468,13 @@ export async function batchCreateEvents(
[['CalendarEvent/set', { accountId, create }, '0']],
USING,
);
const result = methodResult<{ created?: Record<string, unknown> }>(res);
return result.created ? Object.keys(result.created).length : 0;
const result = methodResult<{
created?: Record<string, unknown>;
notCreated?: Record<string, { description?: string; type?: string }>;
}>(res);
const count = result.created ? Object.keys(result.created).length : 0;
if (count === 0) assertSetResult(result, undefined, 'event');
return count;
}
export async function updateEvent(
@@ -493,10 +501,11 @@ export async function deleteEvents(
targetAccountId?: string,
): Promise<void> {
const accountId = targetAccountId || jmapClient.accountId;
await jmapClient.request(
const res = await jmapClient.request(
[['CalendarEvent/set', setArgs(accountId, { destroy: ids }, sendSchedulingMessages), '0']],
USING,
);
assertSetResult(methodResult(res), ids, 'event');
}
// RSVP to an invitation: patch the participant's participationStatus via a JSON
@@ -722,7 +731,9 @@ export async function clearCalendarEvents(
[['CalendarEvent/set', { accountId, destroy: toDestroy }, '0']],
USING,
);
removed += (methodResult<{ destroyed?: string[] }>(res).destroyed ?? []).length;
const result = methodResult<{ destroyed?: string[] }>(res);
assertSetResult(result, toDestroy, 'event');
removed += (result.destroyed ?? []).length;
}
if (toUnlink.length > 0) {
const update: Record<string, unknown> = {};
@@ -731,7 +742,9 @@ export async function clearCalendarEvents(
[['CalendarEvent/set', { accountId, update }, '0']],
USING,
);
removed += Object.keys(methodResult<{ updated?: Record<string, unknown> }>(res).updated ?? {}).length;
const result = methodResult<{ updated?: Record<string, unknown> }>(res);
assertSetResult(result, Object.keys(update), 'event');
removed += Object.keys(result.updated ?? {}).length;
}
if (toDestroy.length === 0 && toUnlink.length === 0) break;
if (ids.length < 1000) break;
+27 -15
View File
@@ -436,15 +436,25 @@ export default function CalendarScreen() {
openEditDirect(event);
}, [openEditDirect, isReadOnlyEvent]);
const reportError = React.useCallback(
(err: unknown) => {
Alert.alert(
t('calendar.notifications.event_error', 'Something went wrong'),
err instanceof Error ? err.message : undefined,
);
},
[t],
);
const handleDeleteFromDetail = React.useCallback((event: CalendarEvent) => {
if (isReadOnlyEvent(event)) { setDetailEvent(null); return; }
setDetailEvent(null);
if (isRecurringSeriesMember(event)) {
setPendingAction({ kind: 'delete', event });
} else {
void deleteEvent(event.id);
deleteEvent(event.id).catch(reportError);
}
}, [deleteEvent, isReadOnlyEvent]);
}, [deleteEvent, isReadOnlyEvent, reportError]);
// "This and following": end the master at the occurrence and hand back the
// master plus its untouched rules so the caller can start a new series (or
@@ -588,9 +598,9 @@ export default function CalendarScreen() {
setPendingAction({ kind: 'delete', event });
return;
}
await deleteEvent(event.id);
await deleteEvent(event.id).catch(reportError);
},
[deleteEvent],
[deleteEvent, reportError],
);
const onRefresh = React.useCallback(async () => {
@@ -602,16 +612,6 @@ export default function CalendarScreen() {
}
}, [refresh]);
const reportError = React.useCallback(
(err: unknown) => {
Alert.alert(
t('calendar.notifications.event_error', 'Something went wrong'),
err instanceof Error ? err.message : undefined,
);
},
[t],
);
// The store flips the checkbox optimistically and reverts it when the
// server refuses; say so instead of leaving the user guessing.
const handleToggleTask = React.useCallback(
@@ -626,6 +626,18 @@ export default function CalendarScreen() {
[toggleTaskComplete, t],
);
const handleDeleteTask = React.useCallback(
(id: string) => {
deleteTask(id).catch((err: unknown) => {
Alert.alert(
t('calendar.tasks.delete_error', 'Failed to delete task'),
err instanceof Error ? err.message : undefined,
);
});
},
[deleteTask, t],
);
// Clone the event one day later and open it in the editor (webmail's
// handleDuplicateFromDetail).
const handleDuplicateFromDetail = React.useCallback(
@@ -960,7 +972,7 @@ export default function CalendarScreen() {
onCreate={createTask}
onUpdate={updateTask}
onToggle={handleToggleTask}
onDelete={deleteTask}
onDelete={handleDeleteTask}
/>
<ICalImportSheet
+14 -7
View File
@@ -639,11 +639,15 @@ export const useCalendarStore = create<CalendarState>()(
}
const prepared = toCreate.map(prepareImportedEvent);
let count = 0;
// Batch in chunks of 50 to avoid oversized requests.
for (let i = 0; i < prepared.length; i += 50) {
count += await apiBatchCreateEvents(prepared.slice(i, i + 50), serverCalendarId, accountId);
try {
// Batch in chunks of 50 to avoid oversized requests.
for (let i = 0; i < prepared.length; i += 50) {
count += await apiBatchCreateEvents(prepared.slice(i, i + 50), serverCalendarId, accountId);
}
} finally {
// Also when a later chunk was refused: show what did get in.
if (count > 0 || linked > 0) await get().refresh();
}
if (count > 0 || linked > 0) await get().refresh();
return count + linked;
},
@@ -674,9 +678,12 @@ export const useCalendarStore = create<CalendarState>()(
clearCalendarEvents: async (id) => {
const cal = get().calendars.find((c) => c.id === id);
const removed = await apiClearCalendarEvents(cal?.originalId || id, cal?.accountId);
await get().refresh();
return removed;
try {
return await apiClearCalendarEvents(cal?.originalId || id, cal?.accountId);
} finally {
// Also after a refused batch: earlier batches may have gone through.
await get().refresh();
}
},
shareCalendar: async (id, principalId, rights) => {