From a7e4e84f5d37ad711d6f99e2c50f260adc321be1 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:20:14 +0200 Subject: [PATCH] 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. --- locales/rn/en.json | 1 + src/components/calendar/TasksSheet.tsx | 104 ++++----------------- src/lib/__tests__/task-editor.test.ts | 97 ++++++++++++++++++++ src/lib/task-editor.ts | 122 +++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 84 deletions(-) create mode 100644 src/lib/__tests__/task-editor.test.ts create mode 100644 src/lib/task-editor.ts diff --git a/locales/rn/en.json b/locales/rn/en.json index 763c8467..3b8cd081 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -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" } }, diff --git a/src/components/calendar/TasksSheet.tsx b/src/components/calendar/TasksSheet.tsx index 1f91ce64..41ac9e07 100644 --- a/src/components/calendar/TasksSheet.tsx +++ b/src/components/calendar/TasksSheet.tsx @@ -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; } -// 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('all'); - const [editor, setEditor] = React.useState(() => emptyEditor(defaultCalendarId)); + const [editor, setEditor] = React.useState(() => 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 = { - 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); }} > diff --git a/src/lib/__tests__/task-editor.test.ts b/src/lib/__tests__/task-editor.test.ts new file mode 100644 index 00000000..964e4b50 --- /dev/null +++ b/src/lib/__tests__/task-editor.test.ts @@ -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 { + 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 }); + }); +}); diff --git a/src/lib/task-editor.ts b/src/lib/task-editor.ts new file mode 100644 index 00000000..f5ac0391 --- /dev/null +++ b/src/lib/task-editor.ts @@ -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, +): Partial { + const data: Partial = { + 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, calendarId: string) => Promise | void; + onUpdate?: (id: string, changes: Partial) => Promise | void; + }, +): Promise { + 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 }; + } +}