mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
fix: say when saving a task fails and keep what was typed
Saving in the tasks sheet ran `onCreate`/`onUpdate` in a `try/finally` without a `catch`, so a refused create or update became an unhandled rejection: the spinner stopped, nothing was shown, and the user could not tell whether the task was saved. The editor state and the save logic move to `src/lib/task-editor.ts`. `submitTaskEditor` returns the server's reason instead of throwing; the sheet shows it in an alert and leaves the editor untouched so the input can be fixed or saved again. It is cleared only after a successful save.
This commit is contained in:
@@ -81,6 +81,7 @@
|
||||
"add_placeholder": "Add a task…",
|
||||
"add_time": "Add time",
|
||||
"delete_error": "Failed to delete task",
|
||||
"save_error": "Failed to save task",
|
||||
"update_error": "Failed to update task"
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import React from 'react';
|
||||
import {
|
||||
Alert,
|
||||
Animated,
|
||||
Dimensions,
|
||||
Easing,
|
||||
@@ -29,13 +30,19 @@ import type { Calendar, CalendarEvent } from '../../api/types';
|
||||
import { radius, spacing, typography, type ThemePalette } from '../../theme/tokens';
|
||||
import { useColors } from '../../theme/colors';
|
||||
import { getCalendarColor, getTaskDueDate, timePattern, type TimeFormat } from '../../lib/calendar-utils';
|
||||
import { getEffectiveTimeZone } from '../../lib/calendar-timezone';
|
||||
import {
|
||||
emptyTaskEditor,
|
||||
priorityToLevel,
|
||||
submitTaskEditor,
|
||||
taskEditorFromTask,
|
||||
type PriorityLevel,
|
||||
type TaskEditorState,
|
||||
} from '../../lib/task-editor';
|
||||
import { isWritableCalendar } from '../../lib/calendar-editability';
|
||||
import { useCalendarLocale } from '../../lib/calendar-locale';
|
||||
import { useSheetDrag } from '../../lib/use-sheet-drag';
|
||||
|
||||
type TaskFilter = 'all' | 'pending' | 'completed' | 'overdue';
|
||||
type PriorityLevel = 'none' | 'high' | 'medium' | 'low';
|
||||
|
||||
interface TasksSheetProps {
|
||||
visible: boolean;
|
||||
@@ -51,25 +58,6 @@ interface TasksSheetProps {
|
||||
onDelete: (id: string) => Promise<void> | void;
|
||||
}
|
||||
|
||||
// RFC 8984 priority 0-9 <-> the three levels the editor offers (webmail's
|
||||
// task-modal mapping).
|
||||
export function priorityToLevel(p: number | undefined): PriorityLevel {
|
||||
if (p === undefined) return 'none';
|
||||
if (p >= 1 && p <= 4) return 'high';
|
||||
if (p === 5) return 'medium';
|
||||
if (p >= 6 && p <= 9) return 'low';
|
||||
return 'none';
|
||||
}
|
||||
|
||||
export function levelToPriority(l: PriorityLevel): number {
|
||||
switch (l) {
|
||||
case 'high': return 1;
|
||||
case 'medium': return 5;
|
||||
case 'low': return 9;
|
||||
default: return 0;
|
||||
}
|
||||
}
|
||||
|
||||
function isCompleted(task: CalendarEvent): boolean {
|
||||
return task.progress === 'completed' || task.progress === 'cancelled';
|
||||
}
|
||||
@@ -99,32 +87,6 @@ function compareTasks(a: CalendarEvent, b: CalendarEvent): number {
|
||||
return (a.title || '').localeCompare(b.title || '');
|
||||
}
|
||||
|
||||
interface EditorState {
|
||||
id: string | null;
|
||||
title: string;
|
||||
description: string;
|
||||
due: Date | null;
|
||||
withTime: boolean;
|
||||
priority: PriorityLevel;
|
||||
calendarId: string;
|
||||
}
|
||||
|
||||
function emptyEditor(calendarId: string): EditorState {
|
||||
return { id: null, title: '', description: '', due: null, withTime: false, priority: 'none', calendarId };
|
||||
}
|
||||
|
||||
function editorFromTask(task: CalendarEvent, fallbackCalendarId: string): EditorState {
|
||||
return {
|
||||
id: task.id,
|
||||
title: task.title || '',
|
||||
description: task.description || '',
|
||||
due: getTaskDueDate(task),
|
||||
withTime: !!task.due && !task.showWithoutTime && !/^\d{4}-\d{2}-\d{2}$/.test(task.due),
|
||||
priority: priorityToLevel(task.priority),
|
||||
calendarId: Object.keys(task.calendarIds || {})[0] || fallbackCalendarId,
|
||||
};
|
||||
}
|
||||
|
||||
export function TasksSheet({
|
||||
visible,
|
||||
tasks,
|
||||
@@ -155,7 +117,7 @@ export function TasksSheet({
|
||||
const defaultCalendarId = writableCalendars[0]?.id || '';
|
||||
|
||||
const [filter, setFilter] = React.useState<TaskFilter>('all');
|
||||
const [editor, setEditor] = React.useState<EditorState>(() => emptyEditor(defaultCalendarId));
|
||||
const [editor, setEditor] = React.useState<TaskEditorState>(() => emptyTaskEditor(defaultCalendarId));
|
||||
const [expanded, setExpanded] = React.useState(false);
|
||||
const [showDatePicker, setShowDatePicker] = React.useState(false);
|
||||
const [showTimePicker, setShowTimePicker] = React.useState(false);
|
||||
@@ -165,10 +127,10 @@ export function TasksSheet({
|
||||
if (!visible) return;
|
||||
const initial = initialTaskId ? tasks.find((task) => task.id === initialTaskId) : undefined;
|
||||
if (initial) {
|
||||
setEditor(editorFromTask(initial, defaultCalendarId));
|
||||
setEditor(taskEditorFromTask(initial, defaultCalendarId));
|
||||
setExpanded(true);
|
||||
} else {
|
||||
setEditor(emptyEditor(defaultCalendarId));
|
||||
setEditor(emptyTaskEditor(defaultCalendarId));
|
||||
setExpanded(false);
|
||||
}
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
@@ -218,47 +180,21 @@ export function TasksSheet({
|
||||
}), [tasks]);
|
||||
|
||||
const resetEditor = () => {
|
||||
setEditor(emptyEditor(defaultCalendarId));
|
||||
setEditor(emptyTaskEditor(defaultCalendarId));
|
||||
setExpanded(false);
|
||||
};
|
||||
|
||||
const handleSave = async () => {
|
||||
const title = editor.title.trim();
|
||||
if (!title || !editor.calendarId || saving) return;
|
||||
if (!editor.title.trim() || !editor.calendarId || saving) return;
|
||||
setSaving(true);
|
||||
try {
|
||||
const data: Partial<CalendarEvent> = {
|
||||
title,
|
||||
description: editor.description.trim(),
|
||||
priority: levelToPriority(editor.priority),
|
||||
};
|
||||
if (editor.due) {
|
||||
if (editor.withTime) {
|
||||
data.due = format(editor.due, "yyyy-MM-dd'T'HH:mm:ss");
|
||||
data.showWithoutTime = false;
|
||||
// The editor shows the due in local time; label it with the zone
|
||||
// it was entered in, like the event editor, or the task's old zone
|
||||
// would shift it.
|
||||
data.timeZone = getEffectiveTimeZone();
|
||||
} else {
|
||||
data.due = format(editor.due, "yyyy-MM-dd'T'00:00:00");
|
||||
data.showWithoutTime = true;
|
||||
data.timeZone = null;
|
||||
}
|
||||
} else if (editor.id) {
|
||||
data.due = null;
|
||||
}
|
||||
if (editor.id) {
|
||||
const existing = tasks.find((task) => task.id === editor.id);
|
||||
const currentCalendar = existing ? Object.keys(existing.calendarIds || {})[0] : undefined;
|
||||
if (editor.calendarId && currentCalendar && editor.calendarId !== currentCalendar) {
|
||||
data.calendarIds = { [editor.calendarId]: true };
|
||||
}
|
||||
await onUpdate?.(editor.id, data);
|
||||
const result = await submitTaskEditor(editor, tasks, { onCreate, onUpdate });
|
||||
if (result.ok) {
|
||||
resetEditor();
|
||||
} else {
|
||||
await onCreate({ ...data, progress: 'needs-action' }, editor.calendarId);
|
||||
// Keep the editor as it is so the user can fix it or try again.
|
||||
Alert.alert(t('calendar.tasks.save_error', 'Failed to save task'), result.message);
|
||||
}
|
||||
resetEditor();
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
@@ -453,7 +389,7 @@ export function TasksSheet({
|
||||
style={styles.taskText}
|
||||
onPress={() => {
|
||||
if (!onUpdate) return;
|
||||
setEditor(editorFromTask(task, defaultCalendarId));
|
||||
setEditor(taskEditorFromTask(task, defaultCalendarId));
|
||||
setExpanded(true);
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
|
||||
vi.mock('../../stores/settings-store', () => ({
|
||||
useSettingsStore: { getState: () => ({ calendarTimeZone: 'Europe/Berlin' }) },
|
||||
}));
|
||||
|
||||
import {
|
||||
buildTaskEditorChanges,
|
||||
emptyTaskEditor,
|
||||
levelToPriority,
|
||||
priorityToLevel,
|
||||
submitTaskEditor,
|
||||
type TaskEditorState,
|
||||
} from '../task-editor';
|
||||
import type { CalendarEvent } from '../../api/types';
|
||||
|
||||
function editor(partial: Partial<TaskEditorState>): TaskEditorState {
|
||||
return { ...emptyTaskEditor('cal-1'), title: 'Buy milk', ...partial };
|
||||
}
|
||||
|
||||
const existing = {
|
||||
id: 't1',
|
||||
title: 'Old',
|
||||
calendarIds: { 'cal-1': true },
|
||||
} as unknown as CalendarEvent;
|
||||
|
||||
describe('priority mapping', () => {
|
||||
it('maps RFC 8984 priorities onto the editor levels and back', () => {
|
||||
expect(priorityToLevel(undefined)).toBe('none');
|
||||
expect(priorityToLevel(0)).toBe('none');
|
||||
expect(priorityToLevel(3)).toBe('high');
|
||||
expect(priorityToLevel(5)).toBe('medium');
|
||||
expect(priorityToLevel(7)).toBe('low');
|
||||
expect(levelToPriority('high')).toBe(1);
|
||||
expect(levelToPriority('none')).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildTaskEditorChanges', () => {
|
||||
it('labels a timed due with the calendar zone and a date-only due with none', () => {
|
||||
const timed = buildTaskEditorChanges(editor({ due: new Date(2026, 8, 24, 17, 30), withTime: true }));
|
||||
expect(timed).toMatchObject({
|
||||
title: 'Buy milk',
|
||||
due: '2026-09-24T17:30:00',
|
||||
showWithoutTime: false,
|
||||
timeZone: 'Europe/Berlin',
|
||||
});
|
||||
const dateOnly = buildTaskEditorChanges(editor({ due: new Date(2026, 8, 24, 17, 30) }));
|
||||
expect(dateOnly).toMatchObject({ due: '2026-09-24T00:00:00', showWithoutTime: true, timeZone: null });
|
||||
});
|
||||
|
||||
it('clears the due of an edited task and sends a changed calendar only', () => {
|
||||
expect(buildTaskEditorChanges(editor({ id: 't1' }), existing)).toMatchObject({ due: null });
|
||||
expect(buildTaskEditorChanges(editor({ id: 't1' }), existing).calendarIds).toBeUndefined();
|
||||
expect(buildTaskEditorChanges(editor({ id: 't1', calendarId: 'cal-2' }), existing).calendarIds)
|
||||
.toEqual({ 'cal-2': true });
|
||||
});
|
||||
});
|
||||
|
||||
describe('submitTaskEditor', () => {
|
||||
it('creates a new task as needs-action in the chosen calendar', async () => {
|
||||
const onCreate = vi.fn(async () => undefined);
|
||||
const result = await submitTaskEditor(editor({ calendarId: 'cal-2' }), [], { onCreate });
|
||||
expect(result).toEqual({ ok: true });
|
||||
expect(onCreate).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ title: 'Buy milk', progress: 'needs-action' }),
|
||||
'cal-2',
|
||||
);
|
||||
});
|
||||
|
||||
it('reports a refused create with the server reason instead of throwing', async () => {
|
||||
const onCreate = vi.fn(async () => {
|
||||
throw new Error('forbidden: calendar is read-only');
|
||||
});
|
||||
const result = await submitTaskEditor(editor({}), [], { onCreate });
|
||||
expect(result).toEqual({ ok: false, message: 'forbidden: calendar is read-only' });
|
||||
});
|
||||
|
||||
it('reports a refused update too', async () => {
|
||||
const onUpdate = vi.fn(async () => {
|
||||
throw new Error('invalidProperties');
|
||||
});
|
||||
const result = await submitTaskEditor(editor({ id: 't1' }), [existing], {
|
||||
onCreate: vi.fn(),
|
||||
onUpdate,
|
||||
});
|
||||
expect(onUpdate).toHaveBeenCalledWith('t1', expect.objectContaining({ title: 'Buy milk' }));
|
||||
expect(result).toEqual({ ok: false, message: 'invalidProperties' });
|
||||
});
|
||||
|
||||
it('reports a failure without a message', async () => {
|
||||
const result = await submitTaskEditor(editor({}), [], {
|
||||
onCreate: () => Promise.reject('nope'),
|
||||
});
|
||||
expect(result).toEqual({ ok: false, message: undefined });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,122 @@
|
||||
import { format } from 'date-fns';
|
||||
import type { CalendarEvent } from '../api/types';
|
||||
import { getTaskDueDate } from './calendar-utils';
|
||||
import { getEffectiveTimeZone } from './calendar-timezone';
|
||||
|
||||
export type PriorityLevel = 'none' | 'high' | 'medium' | 'low';
|
||||
|
||||
// RFC 8984 priority 0-9 <-> the three levels the editor offers (webmail's
|
||||
// task-modal mapping).
|
||||
export function priorityToLevel(p: number | undefined): PriorityLevel {
|
||||
if (p === undefined) return 'none';
|
||||
if (p >= 1 && p <= 4) return 'high';
|
||||
if (p === 5) return 'medium';
|
||||
if (p >= 6 && p <= 9) return 'low';
|
||||
return 'none';
|
||||
}
|
||||
|
||||
export function levelToPriority(l: PriorityLevel): number {
|
||||
switch (l) {
|
||||
case 'high': return 1;
|
||||
case 'medium': return 5;
|
||||
case 'low': return 9;
|
||||
default: return 0;
|
||||
}
|
||||
}
|
||||
|
||||
/** What the tasks sheet's inline editor holds while the user types. */
|
||||
export interface TaskEditorState {
|
||||
id: string | null;
|
||||
title: string;
|
||||
description: string;
|
||||
due: Date | null;
|
||||
withTime: boolean;
|
||||
priority: PriorityLevel;
|
||||
calendarId: string;
|
||||
}
|
||||
|
||||
export function emptyTaskEditor(calendarId: string): TaskEditorState {
|
||||
return { id: null, title: '', description: '', due: null, withTime: false, priority: 'none', calendarId };
|
||||
}
|
||||
|
||||
export function taskEditorFromTask(task: CalendarEvent, fallbackCalendarId: string): TaskEditorState {
|
||||
return {
|
||||
id: task.id,
|
||||
title: task.title || '',
|
||||
description: task.description || '',
|
||||
due: getTaskDueDate(task),
|
||||
withTime: !!task.due && !task.showWithoutTime && !/^\d{4}-\d{2}-\d{2}$/.test(task.due),
|
||||
priority: priorityToLevel(task.priority),
|
||||
calendarId: Object.keys(task.calendarIds || {})[0] || fallbackCalendarId,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The task properties the editor writes. `existing` is the task being
|
||||
* edited (undefined for a new one); a changed calendar is only sent when it
|
||||
* differs from the one the task is in.
|
||||
*/
|
||||
export function buildTaskEditorChanges(
|
||||
editor: TaskEditorState,
|
||||
existing?: Pick<CalendarEvent, 'calendarIds'>,
|
||||
): Partial<CalendarEvent> {
|
||||
const data: Partial<CalendarEvent> = {
|
||||
title: editor.title.trim(),
|
||||
description: editor.description.trim(),
|
||||
priority: levelToPriority(editor.priority),
|
||||
};
|
||||
if (editor.due) {
|
||||
if (editor.withTime) {
|
||||
data.due = format(editor.due, "yyyy-MM-dd'T'HH:mm:ss");
|
||||
data.showWithoutTime = false;
|
||||
// The editor shows the due in local time; label it with the zone
|
||||
// it was entered in, like the event editor, or the task's old zone
|
||||
// would shift it.
|
||||
data.timeZone = getEffectiveTimeZone();
|
||||
} else {
|
||||
data.due = format(editor.due, "yyyy-MM-dd'T'00:00:00");
|
||||
data.showWithoutTime = true;
|
||||
data.timeZone = null;
|
||||
}
|
||||
} else if (editor.id) {
|
||||
data.due = null;
|
||||
}
|
||||
if (editor.id) {
|
||||
const currentCalendar = existing ? Object.keys(existing.calendarIds || {})[0] : undefined;
|
||||
if (editor.calendarId && currentCalendar && editor.calendarId !== currentCalendar) {
|
||||
data.calendarIds = { [editor.calendarId]: true };
|
||||
}
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
export type TaskSaveResult = { ok: true } | { ok: false; message?: string };
|
||||
|
||||
/**
|
||||
* Create or update the task in the editor. Never throws: a refusal comes
|
||||
* back as `{ ok: false }` with the server's reason, so the sheet can say so
|
||||
* and keep what the user typed instead of clearing the editor.
|
||||
*/
|
||||
export async function submitTaskEditor(
|
||||
editor: TaskEditorState,
|
||||
tasks: CalendarEvent[],
|
||||
handlers: {
|
||||
onCreate: (task: Partial<CalendarEvent>, calendarId: string) => Promise<void> | void;
|
||||
onUpdate?: (id: string, changes: Partial<CalendarEvent>) => Promise<void> | void;
|
||||
},
|
||||
): Promise<TaskSaveResult> {
|
||||
try {
|
||||
if (editor.id) {
|
||||
const existing = tasks.find((task) => task.id === editor.id);
|
||||
await handlers.onUpdate?.(editor.id, buildTaskEditorChanges(editor, existing));
|
||||
} else {
|
||||
await handlers.onCreate(
|
||||
{ ...buildTaskEditorChanges(editor), progress: 'needs-action' },
|
||||
editor.calendarId,
|
||||
);
|
||||
}
|
||||
return { ok: true };
|
||||
} catch (err) {
|
||||
return { ok: false, message: err instanceof Error && err.message ? err.message : undefined };
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user