mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
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.
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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 });
|
||||
|
||||
+31
-13
@@ -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.
|
||||
|
||||
+14
-6
@@ -120,6 +120,7 @@ function toEventItem(
|
||||
event: CalendarEvent,
|
||||
calendarsById: Map<string, { name: string; color?: string }>,
|
||||
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<WidgetSnapshot['calendar']> {
|
||||
async function buildCalendar(
|
||||
now: number,
|
||||
selfEmails: string[],
|
||||
previous: WidgetSnapshot,
|
||||
accountId: string,
|
||||
): Promise<WidgetSnapshot['calendar']> {
|
||||
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<Birthday[]> {
|
||||
return out.sort((a, b) => a.date - b.date).slice(0, 20);
|
||||
}
|
||||
|
||||
async function buildTasks(previous: WidgetSnapshot): Promise<WidgetSnapshot['tasks']> {
|
||||
async function buildTasks(previous: WidgetSnapshot, accountId: string): Promise<WidgetSnapshot['tasks']> {
|
||||
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<WidgetSnapshot['tas
|
||||
return {
|
||||
id: t.id,
|
||||
serverId: t.originalId || t.id,
|
||||
accountId,
|
||||
...(t.accountId ? { jmapAccountId: t.accountId } : {}),
|
||||
title: t.title || '',
|
||||
...(Number.isFinite(due) ? { due } : {}),
|
||||
@@ -424,8 +432,8 @@ async function buildSnapshot(previous: WidgetSnapshot): Promise<WidgetSnapshot>
|
||||
|
||||
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),
|
||||
]);
|
||||
|
||||
+31
-1
@@ -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<string, unknown> = {
|
||||
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 } };
|
||||
}
|
||||
|
||||
@@ -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. */}
|
||||
<FlexWidget
|
||||
{...action('toggleTask', { id: task.id })}
|
||||
{...toggleTask(task)}
|
||||
accessibilityLabel={task.done ? f.t('widgets.tasks.mark_incomplete', 'Mark as not done') : f.t('widgets.tasks.mark_complete', 'Mark as done')}
|
||||
style={{ width: 48, height: 48, justifyContent: 'center', alignItems: 'center' }}
|
||||
>
|
||||
|
||||
@@ -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 (
|
||||
|
||||
@@ -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 (
|
||||
<FlexWidget
|
||||
style={{
|
||||
|
||||
@@ -17,8 +17,11 @@ export const PENDING_KEY = 'widgets:pending:v1';
|
||||
export type PendingChange =
|
||||
| { kind: 'removeMail'; id: string }
|
||||
| { kind: 'markRead'; id: string }
|
||||
| { kind: 'rsvp'; id: string; serverId: string; status: 'accepted' | 'tentative' | 'declined' }
|
||||
| { kind: 'task'; id: string; done: boolean };
|
||||
// `accountId`: the signed-in account the item belongs to. JMAP ids are
|
||||
// only unique within an account, so the change must not touch another
|
||||
// account's item with the same id once the widgets show that account.
|
||||
| { kind: 'rsvp'; id: string; serverId: string; status: 'accepted' | 'tentative' | 'declined'; accountId?: string }
|
||||
| { kind: 'task'; id: string; done: boolean; accountId?: string };
|
||||
|
||||
export interface PendingOp {
|
||||
/** Unique per tap. */
|
||||
@@ -130,6 +133,7 @@ export function applyPending(s: WidgetSnapshot, ops: PendingOp[]): WidgetSnapsho
|
||||
break;
|
||||
}
|
||||
case 'rsvp':
|
||||
if (change.accountId && change.accountId !== s.activeAccountId) break;
|
||||
next.calendar.invitations = next.calendar.invitations.filter(
|
||||
(i) => 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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user