From a69475194481e12cc49255f93c4ffb19853769ee Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:45:48 +0200 Subject: [PATCH] fix: send widget RSVPs and task ticks to the item the tap was made on #52 An RSVP or task tick named only the item's id and looked it up in the stored snapshot when the tap arrived, then used the snapshot's active account. Between a refresh and the redraw that follows, a tap made on the old drawing could reach a snapshot of another account, and JMAP ids are short per-account sequences, so the lookup could match that account's task or event with the same id. A tick also flipped whatever the stored state was, so a quick second tap on a drawing that had not caught up yet undid the first. The tap now carries what the call needs: the item's account, its server ids and, for a tick, the state the user asked for. The change laid over the widget data only touches items of that account. Drawings made before this keep working through the old lookup. --- src/widgets/__tests__/actions.test.ts | 33 ++++++++++++++++++++ src/widgets/__tests__/pending.test.ts | 11 +++++++ src/widgets/actions.ts | 44 +++++++++++++++++++-------- src/widgets/build.ts | 20 ++++++++---- src/widgets/clicks.ts | 32 ++++++++++++++++++- src/widgets/layouts/hubs.tsx | 4 +-- src/widgets/layouts/plans.tsx | 4 +-- src/widgets/parts.tsx | 4 +-- src/widgets/pending.ts | 9 ++++-- src/widgets/snapshot.ts | 4 +++ 10 files changed, 137 insertions(+), 28 deletions(-) diff --git a/src/widgets/__tests__/actions.test.ts b/src/widgets/__tests__/actions.test.ts index 57e964fe..a6a7ff7d 100644 --- a/src/widgets/__tests__/actions.test.ts +++ b/src/widgets/__tests__/actions.test.ts @@ -63,6 +63,7 @@ function snapshotOf(ids: string[]): WidgetSnapshot { }; } +const jmap = await import('../jmap'); const { handleWidgetAction } = await import('../actions'); const { currentView } = await import('../state'); const { saveSnapshot } = await import('../snapshot'); @@ -80,6 +81,38 @@ beforeEach(async () => { await saveSnapshot(snapshotOf(server.inbox)); }); +describe('widget task ticks and RSVPs', () => { + it('go to the account the tapped drawing showed, not the one the widgets show now', async () => { + // The widgets moved on to acc2, which has a task with the same JMAP id. + const s = snapshotOf([]); + s.activeAccountId = 'acc2'; + s.tasks.items = [{ + id: 'e', serverId: 'e', accountId: 'acc2', title: 'Theirs', dueHasTime: false, done: false, calendarName: '', color: '#123456', + }]; + await saveSnapshot(s); + vi.mocked(jmap.openClient).mockClear(); + vi.mocked(jmap.setTaskDone).mockClear(); + await handleWidgetAction('toggleTask', { id: 'e', done: true, serverId: 'e', accountId: 'acc', title: 'Mine' }, 1); + expect(jmap.openClient).toHaveBeenCalledWith('acc'); + expect(jmap.setTaskDone).toHaveBeenCalledWith(expect.anything(), 'e', true, undefined); + }); + + it('mark a task done on both of two quick taps rather than undoing the first', async () => { + vi.mocked(jmap.setTaskDone).mockClear(); + const tap = { id: 't', done: true, serverId: 't', accountId: 'acc', title: 'Task' }; + await Promise.all([handleWidgetAction('toggleTask', tap, 1), handleWidgetAction('toggleTask', tap, 1)]); + expect(vi.mocked(jmap.setTaskDone).mock.calls.map((c) => c[2])).toEqual([true, true]); + }); + + it('send an RSVP with the ids the tap carries', async () => { + vi.mocked(jmap.rsvp).mockClear(); + await handleWidgetAction('rsvp', { + id: 'h1', status: 'accepted', serverId: 'j', participantId: 'a', accountId: 'acc', jmapAccountId: 'd', title: 'Budget', + }, 1); + expect(jmap.rsvp).toHaveBeenCalledWith(expect.anything(), 'j', 'a', 'accepted', 'd'); + }); +}); + describe('widget archive', () => { it('keeps a message archived when a refresh read the server before its archive landed', async () => { const first = handleWidgetAction('archive', { id: 'm1', accountId: 'acc' }, 7); diff --git a/src/widgets/__tests__/pending.test.ts b/src/widgets/__tests__/pending.test.ts index 5dfa46ae..bf53baa7 100644 --- a/src/widgets/__tests__/pending.test.ts +++ b/src/widgets/__tests__/pending.test.ts @@ -56,6 +56,17 @@ describe('applyPending', () => { }); }); +describe('applyPending across accounts', () => { + it('leaves an item of another account with the same id alone', () => { + const s = sampleSnapshot(PREVIEW_NOW); + const task = s.tasks.items.find((t) => !t.done)!; + const shown = applyPending(s, [op({ kind: 'task', id: task.id, done: true, accountId: 'not-this-one' })]); + expect(shown.tasks.items.find((t) => t.id === task.id)!.done).toBe(false); + const own = applyPending(s, [op({ kind: 'task', id: task.id, done: true, accountId: s.activeAccountId! })]); + expect(own.tasks.items.find((t) => t.id === task.id)!.done).toBe(true); + }); +}); + describe('confirmedBy', () => { const mail = op({ kind: 'removeMail', id: 'm' }, { doneAt: 2000 }); const task = op({ kind: 'task', id: 't', done: true }, { doneAt: 2000 }); diff --git a/src/widgets/actions.ts b/src/widgets/actions.ts index 260d92fe..b9ffea6a 100644 --- a/src/widgets/actions.ts +++ b/src/widgets/actions.ts @@ -118,12 +118,22 @@ export async function handleWidgetAction(name: string, data: Data, widgetId: num const id = str(data.id); const status = data.status; if (!id || (status !== 'accepted' && status !== 'tentative' && status !== 'declined')) return; - const before = await currentView(); - const invitation = before.calendar.invitations.find((i) => i.id === id); - if (!invitation) return; - await apply('rsvp', data, invitation.title, { kind: 'rsvp', id, serverId: invitation.serverId, status }, async () => { - const client = await clientFor(before.activeAccountId); - return !!client && rsvp(client, invitation.serverId, invitation.participantId, status, invitation.jmapAccountId); + // The tap carries the invitation (clicks.ts); a drawing made before it + // did is looked up in what the widgets show now. + const view = await currentView(); + const shown = view.calendar.invitations.find((i) => i.id === id); + const accountId = str(data.accountId) ?? shown?.accountId ?? view.activeAccountId; + const serverId = str(data.serverId) ?? shown?.serverId; + const participantId = str(data.participantId) ?? shown?.participantId; + const jmapAccountId = str(data.jmapAccountId) ?? (str(data.serverId) ? undefined : shown?.jmapAccountId); + if (!serverId || !participantId) { + await redrawAll(); + return; + } + const title = str(data.title) ?? shown?.title ?? ''; + await apply('rsvp', data, title, { kind: 'rsvp', id, serverId, status, accountId: accountId ?? undefined }, async () => { + const client = await clientFor(accountId); + return !!client && rsvp(client, serverId, participantId, status, jmapAccountId); }); return; } @@ -131,13 +141,21 @@ export async function handleWidgetAction(name: string, data: Data, widgetId: num case 'toggleTask': { const id = str(data.id); if (!id) return; - const before = await currentView(); - const task = before.tasks.items.find((t) => t.id === id); - if (!task) return; - const done = !task.done; - await apply('toggleTask', data, task.title, { kind: 'task', id, done }, async () => { - const client = await clientFor(before.activeAccountId); - if (!client || !(await setTaskDone(client, task.serverId, done, task.jmapAccountId))) return false; + const view = await currentView(); + const shown = view.tasks.items.find((t) => t.id === id); + const accountId = str(data.accountId) ?? shown?.accountId ?? view.activeAccountId; + const serverId = str(data.serverId) ?? shown?.serverId; + const jmapAccountId = str(data.jmapAccountId) ?? (str(data.serverId) ? undefined : shown?.jmapAccountId); + // The state the user asked for; older drawings only named the task. + const done = typeof data.done === 'boolean' ? data.done : shown ? !shown.done : undefined; + if (!serverId || done === undefined) { + await redrawAll(); + return; + } + const title = str(data.title) ?? shown?.title ?? ''; + await apply('toggleTask', data, title, { kind: 'task', id, done, accountId: accountId ?? undefined }, async () => { + const client = await clientFor(accountId); + if (!client || !(await setTaskDone(client, serverId, done, jmapAccountId))) return false; // In the live app the snapshot's tasks come from the calendar store, // which did not see this change; reload it before the refresh that // follows reads it, or that refresh would undo the tick. diff --git a/src/widgets/build.ts b/src/widgets/build.ts index 6c01e013..50275466 100644 --- a/src/widgets/build.ts +++ b/src/widgets/build.ts @@ -120,6 +120,7 @@ function toEventItem( event: CalendarEvent, calendarsById: Map, selfEmails: string[], + accountId: string, ): EventItem { const calId = Object.keys(event.calendarIds ?? {}).find((id) => event.calendarIds[id]); const cal = calId ? calendarsById.get(calId) : undefined; @@ -155,6 +156,7 @@ function toEventItem( return { id: event.id, serverId: event.originalId || event.id, + accountId, ...(event.accountId ? { jmapAccountId: event.accountId } : {}), title: event.title || '', start, @@ -172,7 +174,12 @@ function toEventItem( }; } -async function buildCalendar(now: number, selfEmails: string[], previous: WidgetSnapshot): Promise { +async function buildCalendar( + now: number, + selfEmails: string[], + previous: WidgetSnapshot, + accountId: string, +): Promise { const calendarsSupported = jmapClient.hasAccountCapability(CAPABILITIES.CALENDARS); if (!calendarsSupported) return { supported: false, events: [], invitations: [], birthdays: await buildBirthdays(now) }; const { useCalendarStore, loadEventsInRange } = require('../stores/calendar-store') as typeof import('../stores/calendar-store'); @@ -203,7 +210,7 @@ async function buildCalendar(now: number, selfEmails: string[], previous: Widget } const live = raw.filter((e) => e.status !== 'cancelled'); const events = live - .map((e) => toEventItem(e, byId, selfEmails)) + .map((e) => toEventItem(e, byId, selfEmails, accountId)) .filter((e) => e.end >= rangeStart && e.start < rangeEnd) .sort((a, b) => a.start - b.start); @@ -215,7 +222,7 @@ async function buildCalendar(now: number, selfEmails: string[], previous: Widget const participantId = getUserParticipantId(e, selfEmails); const key = e.originalId || e.id; if (!participantId || seen.has(key)) continue; - const item = toEventItem(e, byId, selfEmails); + const item = toEventItem(e, byId, selfEmails, accountId); if (item.end < now) continue; seen.add(key); invitations.push({ ...item, participantId }); @@ -262,7 +269,7 @@ async function buildBirthdays(now: number): Promise { return out.sort((a, b) => a.date - b.date).slice(0, 20); } -async function buildTasks(previous: WidgetSnapshot): Promise { +async function buildTasks(previous: WidgetSnapshot, accountId: string): Promise { if (!useSettingsStore.getState().enableCalendarTasks) return { supported: false, items: [] }; const { useCalendarStore } = require('../stores/calendar-store') as typeof import('../stores/calendar-store'); try { @@ -280,6 +287,7 @@ async function buildTasks(previous: WidgetSnapshot): Promise if (live) { const [calendar, tasks, files, scheduled] = await Promise.all([ - buildCalendar(now, section.selfEmails, next), - buildTasks(next), + buildCalendar(now, section.selfEmails, next, activeAccountId), + buildTasks(next, activeAccountId), buildFiles(next), buildScheduled(next), ]); diff --git a/src/widgets/clicks.ts b/src/widgets/clicks.ts index 838f21c9..687866ee 100644 --- a/src/widgets/clicks.ts +++ b/src/widgets/clicks.ts @@ -3,7 +3,7 @@ // names an action the widget task performs in the background (./actions.ts). import { APP_SCHEME } from '../navigation/linking'; -import type { EventItem, MailItem } from './snapshot'; +import type { EventItem, Invitation, MailItem, TaskItem } from './snapshot'; export interface WidgetClick { clickAction?: string; @@ -23,6 +23,36 @@ export function action(name: WidgetActionName, data: Record = { return { clickAction: name, clickActionData: data }; } +/** + * A task tick. It names the state the user asked for, not "the opposite of + * now": a second tap on a drawing that has not caught up yet would otherwise + * undo the first. It carries the task's account and server id so it acts on + * that task even when the widgets have moved on to another account. + */ +export function toggleTask(task: TaskItem): WidgetClick { + return action('toggleTask', { + id: task.id, + done: !task.done, + serverId: task.serverId, + title: task.title, + ...(task.accountId ? { accountId: task.accountId } : {}), + ...(task.jmapAccountId ? { jmapAccountId: task.jmapAccountId } : {}), + }); +} + +/** An RSVP; like {@link toggleTask} it carries everything the call needs. */ +export function rsvp(inv: Invitation, status: 'accepted' | 'tentative' | 'declined'): WidgetClick { + return action('rsvp', { + id: inv.id, + status, + serverId: inv.serverId, + participantId: inv.participantId, + title: inv.title, + ...(inv.accountId ? { accountId: inv.accountId } : {}), + ...(inv.jmapAccountId ? { jmapAccountId: inv.jmapAccountId } : {}), + }); +} + export function open(uri: string): WidgetClick { return { clickAction: 'OPEN_URI', clickActionData: { uri } }; } diff --git a/src/widgets/layouts/hubs.tsx b/src/widgets/layouts/hubs.tsx index 5abeca89..dc784325 100644 --- a/src/widgets/layouts/hubs.tsx +++ b/src/widgets/layouts/hubs.tsx @@ -4,7 +4,7 @@ import React from 'react'; import { FlexWidget, SvgWidget, TextWidget } from 'react-native-android-widget'; -import { action, links, open, type WidgetClick } from '../clicks'; +import { links, open, toggleTask, type WidgetClick } from '../clicks'; import { dueKind, nextEvent, sortTasks, tasksDueNow, upcomingTimed } from '../derive'; import { addDays, sameDay, startOfDay, type Fmt } from '../format'; import { ring, type IconName } from '../icons'; @@ -133,7 +133,7 @@ function HubTaskLine({ p, f, task, now }: { p: WidgetPalette; f: Fmt; task: Task > {/* 48dp touch target around the 20dp box, which sits at the 14dp inset. */} diff --git a/src/widgets/layouts/plans.tsx b/src/widgets/layouts/plans.tsx index f08581e6..b56a0214 100644 --- a/src/widgets/layouts/plans.tsx +++ b/src/widgets/layouts/plans.tsx @@ -5,7 +5,7 @@ import React from 'react'; import { FlexWidget, OverlapWidget, SvgWidget } from 'react-native-android-widget'; -import { action, links, open } from '../clicks'; +import { links, open, rsvp } from '../clicks'; import { busyBlocks, countdownTarget, @@ -107,7 +107,7 @@ export const InvitationsLayout: Layout = ({ s, p, f, height }) => { const names = inv.participants.slice(0, 3).map((x) => x.name); const extra = inv.participants.length - names.length; const people = `${names.join(', ')}${extra > 0 ? ` +${extra}` : ''}`; - const reply = (status: 'accepted' | 'tentative' | 'declined') => action('rsvp', { id: inv.id, status }); + const reply = (status: 'accepted' | 'tentative' | 'declined') => rsvp(inv, status); const notice = noticeFor(s.notice, 'rsvp'); return ( diff --git a/src/widgets/parts.tsx b/src/widgets/parts.tsx index c8e4b988..dfe61696 100644 --- a/src/widgets/parts.tsx +++ b/src/widgets/parts.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { FlexWidget, TextWidget } from 'react-native-android-widget'; -import { action, links, open, type WidgetClick } from './clicks'; +import { action, links, open, toggleTask, type WidgetClick } from './clicks'; import { dueKind } from './derive'; import type { Fmt } from './format'; import { Avatar, ColorBar, Dot, FilledIcon, Icon, Spacer, Txt } from './primitives'; @@ -223,7 +223,7 @@ export function TaskRow({ : kind === 'tomorrow' ? f.t('widgets.tasks.tomorrow', 'Tomorrow') : f.weekdayDayMonth(task.due); - const toggle: WidgetClick = action('toggleTask', { id: task.id }); + const toggle: WidgetClick = toggleTask(task); return ( i.id !== change.id && i.serverId !== change.serverId, ); @@ -137,6 +141,7 @@ export function applyPending(s: WidgetSnapshot, ops: PendingOp[]): WidgetSnapsho (e.serverId === change.serverId && e.myStatus ? { ...e, myStatus: change.status } : e)); break; case 'task': + if (change.accountId && change.accountId !== s.activeAccountId) break; next.tasks.items = next.tasks.items.map((t) => (t.id === change.id ? { ...t, done: change.done } : t)); break; } diff --git a/src/widgets/snapshot.ts b/src/widgets/snapshot.ts index d20d86ef..b6311e10 100644 --- a/src/widgets/snapshot.ts +++ b/src/widgets/snapshot.ts @@ -95,6 +95,8 @@ export interface EventItem { id: string; /** Server id of the stored event, for deep links. */ serverId: string; + /** Signed-in account (app registry id) the event was loaded for. */ + accountId?: string; jmapAccountId?: string; title: string; start: number; @@ -129,6 +131,8 @@ export interface TaskItem { id: string; /** Raw JMAP id, for CalendarEvent/set. */ serverId: string; + /** Signed-in account (app registry id) the task was loaded for. */ + accountId?: string; jmapAccountId?: string; title: string; due?: number;