diff --git a/CLAUDE.md b/CLAUDE.md index 6ece437c..70ab0b7c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ Commune is a shared communal expense management and group management platform. The application was migrated. The stack below is what runs today. - **Web (active app):** `apps/web`. React 19 + Vite 7 + TypeScript + wouter, plain CSS with shared tokens, tested with Vitest. pnpm. Vercel serves the production build (`apps/web/vercel.json`). -- **Hosted data and auth:** Neon (Managed Auth, Postgres, Data API). Schema and functions live in `infra/neon/migrations`: 40 migrations, 0001 to 0040, applied in numeric order. `infra/neon/schema.sql` is the schema they produce, with every function in full. Production builds use `apps/web/src/live/neon.ts`. +- **Hosted data and auth:** Neon (Managed Auth, Postgres, Data API). Schema and functions live in `infra/neon/migrations`: 41 migrations, 0001 to 0041, applied in numeric order. `infra/neon/schema.sql` is the schema they produce, with every function in full. Production builds use `apps/web/src/live/neon.ts`. - **Local development backend:** `apps/local-api`, a loopback-only PostgreSQL API on port 3002 that calls the same database functions. Vite proxies `/api` to it. Its database is set up as `infra/local/README.md` describes. It is never deployed. - **Email:** `apps/reminder-worker`, a Neon Function that sends reminder and invitation emails from the outbox through Resend, with one-click unsubscribe and a bounce webhook. - **Shared logic:** `packages/core` (split arithmetic, balances, reading typed amounts, and writing money and dates), imported as `@commune/core` through a path in `apps/web/tsconfig.json` and an alias in `apps/web/vite.config.ts`. It has no `package.json`. Amounts and day-and-month dates are written by its `formatMoney` and `formatDay`; `apps/web/tests/formatting.test.ts` fails if app code uses `Intl.NumberFormat`, `toLocaleString` or `toLocaleDateString`. diff --git a/apps/local-api/README.md b/apps/local-api/README.md index 340ae805..1b014826 100644 --- a/apps/local-api/README.md +++ b/apps/local-api/README.md @@ -58,7 +58,7 @@ browser code sends same-origin `/api` requests. Non-2xx errors are | POST | `/api/communes/:id/operations` | `{kind,category,title,date,time,allDay,assigneeId,rotationIds,repeat,notes,requestId}`; any member; at most 1,000 tasks and events per commune; a new task is open; a repeated `requestId` returns the same one, or 409 once it has been removed | `{id,version,duplicate}` (201, or 200 for a repeat) | | PUT | `/api/operations/:id` | the same fields with `expectedVersion`; changes what, when and who only, never status or completion; any occurrence or handover since makes the edit stale (409) | `{id,version,duplicate}` | | DELETE | `/api/operations/:id` | the person who added it or an admin; it and its occurrences leave every view and are kept, as is the activity; repeating is harmless | `{id,version,duplicate}` | -| POST | `/api/operations/:id/occurrences` | `{action:"completed"\|"skipped"\|"reopened",expectedDate,today,requestId,reason?}`; the database advances the date and rotation; a stale `expectedDate` is 409; only a skip takes a `reason` (up to 200 characters) | `{id,duplicate,date,assignee,assigneeId,turnOwnerId,status}` | +| POST | `/api/operations/:id/occurrences` | `{action:"completed"\|"skipped"\|"reopened",expectedDate,today,requestId,reason?,doneBy?}`; the database advances the date and rotation; a stale `expectedDate` is 409; only a skip takes a `reason` (up to 200 characters); completing a task someone else holds needs `doneBy` (`"me"` or `"assignee"`, 0041), and without it is 400 | `{id,duplicate,date,assignee,assigneeId,turnOwnerId,status}` | | POST | `/api/operations/:id/handovers` | `{toMemberId,expectedDate,expectedAssigneeId,today,reason?,requestId}`; any member hands the current occurrence to another active member; on a task that takes turns the rotation then carries on from the person whose turn it was; a stale date or holder is 409 | `{id,duplicate,date,assignee,assigneeId,turnOwnerId,status}` | | POST | `/api/communes/:id/announcements` | `{title,body?,needsAcknowledgement,requestId}` admin only; title up to 80, body up to 1000 characters; at most 20 up at once; a repeated `requestId` returns the same notice | `{id,duplicate,archived}` (201, or 200 for a repeat) | | POST | `/api/announcements/:id/acknowledge` | `{}` member confirms an announcement that asks for it; repeating is harmless; refused once taken down | `{id,duplicate,archived}` | diff --git a/apps/local-api/src/server.ts b/apps/local-api/src/server.ts index b895a3d8..5545479e 100644 --- a/apps/local-api/src/server.ts +++ b/apps/local-api/src/server.ts @@ -793,10 +793,12 @@ route("POST", /^\/api\/operations\/([0-9a-f-]{36})\/occurrences$/, async (req, r throw new ApiError(400, "Invalid task action"); } if (input.reason != null && typeof input.reason !== "string") throw new ApiError(400, "Invalid reason"); + // Marking someone else's task done says who did it: "me" or "assignee". + if (input.doneBy != null && !["me", "assignee"].includes(String(input.doneBy))) throw new ApiError(400, "Invalid doneBy"); const result = await userAction(req, async (client) => client.query( - "SELECT public.record_operation_occurrence($1, $2, $3::date, $4::date, $5, $6) AS result", + "SELECT public.record_operation_occurrence($1, $2, $3::date, $4::date, $5, $6, $7) AS result", [uuid(match[1]), input.action, isoDate(input.expectedDate, false, "expectedDate"), - isoDate(input.today, false, "today"), uuid(input.requestId, "requestId"), input.reason ?? null])); + isoDate(input.today, false, "today"), uuid(input.requestId, "requestId"), input.reason ?? null, input.doneBy ?? null])); const occurrence = result.rows[0].result as { duplicate: boolean }; send(res, occurrence, occurrence.duplicate ? 200 : 201); }); diff --git a/apps/local-api/tests/note-and-task-commands.mjs b/apps/local-api/tests/note-and-task-commands.mjs index 5bcb43b0..bb24fc6c 100644 --- a/apps/local-api/tests/note-and-task-commands.mjs +++ b/apps/local-api/tests/note-and-task-commands.mjs @@ -100,21 +100,21 @@ assert.equal(ok(await b("POST", `/communes/${id}/operations`, { ...chore, reques let row = (await snapshot()).operations.find(item => item.id === task.id); assert.deepEqual([row.version, row.status, row.lastDone, row.createdBy], [1, "open", null, blake.id]); -// Blake opens the task to edit it; meanwhile Casey marks it done. Blake's +// Blake opens the task to edit it; meanwhile Casey marks it done for him. Blake's // save would put the task back, so it is refused and the completion stays. -ok(await c("POST", `/operations/${task.id}/occurrences`, { action: "completed", expectedDate: today, today, requestId: randomUUID() }), 201); +ok(await c("POST", `/operations/${task.id}/occurrences`, { action: "completed", expectedDate: today, today, requestId: randomUUID(), doneBy: "assignee" }), 201); const staleTask = await b("PUT", `/operations/${task.id}`, { ...chore, title: "Hoover the stairs and hall", expectedVersion: 1 }); assert.equal(staleTask.status, 409); assert.match(staleTask.error, /This task was changed by someone else/); row = (await snapshot()).operations.find(item => item.id === task.id); -assert.deepEqual([row.version, row.title, row.assigneeId, row.lastDone], [2, "Hoover the stairs", casey.id, { by: blake.name, date: today }]); +assert.deepEqual([row.version, row.title, row.assigneeId, row.lastDone], [2, "Hoover the stairs", casey.id, { by: blake.name, date: today, markedBy: casey.name }]); // Saved on top of the latest version, an edit changes what, when and who, // and leaves the completion as it is. const edited = ok(await b("PUT", `/operations/${task.id}`, { ...chore, title: "Hoover the stairs and hall", date: row.date, assigneeId: row.assigneeId, status: "done", lastDone: null, expectedVersion: row.version })); assert.deepEqual([edited.version, edited.duplicate], [3, false]); row = (await snapshot()).operations.find(item => item.id === task.id); -assert.deepEqual([row.title, row.status, row.lastDone], ["Hoover the stairs and hall", "open", { by: blake.name, date: today }]); +assert.deepEqual([row.title, row.status, row.lastDone], ["Hoover the stairs and hall", "open", { by: blake.name, date: today, markedBy: casey.name }]); // Casey cannot remove a task Blake added; Avery, an admin, can. It leaves // the calendar, keeps its history and takes no further actions; a late retry @@ -129,7 +129,7 @@ assert.equal(late.status, 409); assert.match(late.error, /This task has been removed/); const lateAdd = await b("POST", `/communes/${id}/operations`, { ...chore, requestId: taskRequest }); assert.deepEqual([lateAdd.status, lateAdd.error], [409, "This task has been removed"]); -assert.deepEqual(await events(task.id), ["Completed. Next: Casey Commands", "Created", "Removed", "Updated"]); +assert.deepEqual(await events(task.id), ["Created", "Marked done for Blake Commands. Next: Casey Commands", "Removed", "Updated"]); // The person who added a task can remove it too. const visit = ok(await c("POST", `/communes/${id}/operations`, { kind: "event", category: "visit", title: "Gas safety check", date: today, allDay: true, repeat: "none", notes: "", requestId: randomUUID() }), 201); diff --git a/apps/local-api/tests/operations-history.mjs b/apps/local-api/tests/operations-history.mjs index e1846de7..a295579a 100644 --- a/apps/local-api/tests/operations-history.mjs +++ b/apps/local-api/tests/operations-history.mjs @@ -87,7 +87,7 @@ assert.deepEqual([done.duplicate, done.date, done.assignee, done.taskStatus], [f const again = ok(await act(bins, "completed", today, a, { requestId }), 200); assert.deepEqual([again.id, again.duplicate, again.date, again.assignee], [done.id, true, day(7), blake.name]); // A second person's stale click cannot advance the rotation twice. -const stale = await act(bins, "completed", today, b); +const stale = await act(bins, "completed", today, b, { doneBy: "assignee" }); assert.equal(stale.status, 409); assert.match(stale.error, /already moved on/); let saved = await read(bins, b); @@ -108,7 +108,8 @@ assert.equal(saved.history[0].undoneBy, blake.id); assert.ok(saved.history[0].undoneAt); // Six people-at-once completions of one occurrence: exactly one counts. -const burst = await Promise.all(Array.from({ length: 6 }, (_, index) => act(bins, "completed", today, index % 2 ? b : a))); +const burst = await Promise.all(Array.from({ length: 6 }, (_, index) => + index % 2 ? act(bins, "completed", today, b, { doneBy: "assignee" }) : act(bins, "completed", today, a))); assert.equal(burst.filter(result => result.status === 201).length, 1, JSON.stringify(burst)); assert.equal(burst.filter(result => result.status === 409).length, 5); saved = await read(bins); @@ -127,7 +128,7 @@ assert.equal((await read(bins)).undoOccurrenceId, null); assert.equal((await a("POST", `/occurrences/${second.id}/undo`, {})).status, 409); assert.deepEqual([(await read(bins)).date, (await read(bins)).assignee], [day(15), casey.name]); // The rotation wraps round to the first person. -const wrapped = ok(await act(bins, "completed", day(15)), 201); +const wrapped = ok(await act(bins, "completed", day(15), a, { doneBy: "assignee" }), 201); assert.equal(wrapped.assignee, avery.name); @@ -135,10 +136,16 @@ assert.equal(wrapped.assignee, avery.name); const late = await create({ title: "Deep clean", date: day(-40), assigneeId: blake.id, repeat: "monthly" }); let expected = day(-40); do { expected = addMonth(expected); } while (expected <= today); -const monthly = ok(await act(late, "completed", day(-40)), 201); +// Marking Blake's task done says who did it (0041): without an answer it is +// refused, and with one both names go on it. +const unanswered = await act(late, "completed", day(-40)); +assert.equal(unanswered.status, 400); +assert.equal(unanswered.error, "This is Blake's turn. Say whether you did it or they did."); +assert.equal((await act(late, "completed", day(-40), a, { doneBy: "someone" })).status, 400); +const monthly = ok(await act(late, "completed", day(-40), a, { doneBy: "assignee" }), 201); assert.deepEqual([monthly.date, monthly.assignee, monthly.taskStatus], [expected, blake.name, "open"]); -// The assignee, not whoever ticked the box, is credited for a repeating task. -assert.deepEqual((await read(late)).lastDone, { by: blake.name, date: today }); +assert.deepEqual((await read(late)).lastDone, { by: blake.name, date: today, markedBy: avery.name }); +assert.deepEqual([(await read(late)).history[0].actorId, (await read(late)).history[0].doneById], [avery.id, blake.id]); ok(await a("POST", `/occurrences/${monthly.id}/undo`, {})); assert.deepEqual([(await read(late)).date, (await read(late)).lastDone], [day(-40), null]); const skipped = ok(await act(late, "skipped", day(-40)), 201); @@ -170,10 +177,13 @@ assert.equal((await read(late)).date, day(-40)); // ---------------------------------------- one-off tasks, reopen, and events const key = await create({ title: "Return the spare key", date: today, assigneeId: blake.id }); assert.equal((await act(key, "skipped", today)).status, 409); -const finished = ok(await act(key, "completed", today), 201); +const finished = ok(await act(key, "completed", today, a, { doneBy: "me" }), 201); assert.deepEqual([finished.taskStatus, finished.date], ["done", today]); -assert.deepEqual((await read(key)).lastDone, { by: avery.name, date: today }); +assert.deepEqual((await read(key)).lastDone, { by: avery.name, date: today, covering: blake.name }); assert.equal((await act(key, "completed", today, b)).status, 409); +// The activity names both people. +assert.ok(ok(await a("GET", `/communes/${id}/snapshot`)).activityEvents.some(item => item.entityId === key + && item.description === `Completed, covering ${blake.name}`)); ok(await b("POST", `/occurrences/${finished.id}/undo`, {})); assert.deepEqual([(await read(key)).status, (await read(key)).lastDone], ["open", null]); // Status and completion cannot be sent with an add or an edit: a new task diff --git a/apps/web/src/data/operations.ts b/apps/web/src/data/operations.ts index 37ac9261..aba22094 100644 --- a/apps/web/src/data/operations.ts +++ b/apps/web/src/data/operations.ts @@ -26,9 +26,25 @@ export type OperationOccurrence = { nextAssigneeId?: string | null; /** Why it was skipped or handed over, when someone said. */ reason?: string | null; + /** + * Who a completion credits: the person who marked it, or the holder they + * marked it for. Missing on older entries and on tasks held by a name only. + */ + doneById?: string | null; undoneAt: string | null; undoneBy: string | null; }; +/** + * Who did a task someone else holds, said by the person marking it done: + * themselves, covering the turn, or the holder. + */ +export type DoneBy = "me" | "assignee"; + +/** Marking this task done has to say who did it: someone else holds it. */ +export function needsDoneBy(item: OperationItem, youId: string): boolean { + return item.kind === "task" && item.status === "open" && Boolean(item.assigneeId) && item.assigneeId !== youId; +} + /** What the server reports after a task action or its undo. */ export type OperationChange = { occurrenceId: string; @@ -61,7 +77,11 @@ export type OperationItem = { */ turnOwnerId?: string | null; turnOwner?: string | null; - lastDone?: { by: string; date: string }; + /** + * The last completion: who did it, and who marked it done for them or + * whose turn they covered, when that was someone else. + */ + lastDone?: { by: string; date: string; markedBy?: string; covering?: string }; repeat: OperationRepeat; status: OperationStatus; notes: string; diff --git a/apps/web/src/live/neon.ts b/apps/web/src/live/neon.ts index 6b3fd313..1f9354dc 100644 --- a/apps/web/src/live/neon.ts +++ b/apps/web/src/live/neon.ts @@ -683,6 +683,8 @@ async function route(path: string, init: RequestInit): Promise { p_expected_date: body.expectedDate, p_today: body.today, p_request_id: body.requestId, // Sent only with a reason, so a skip without one matches the 0018 signature. ...(body.reason ? { p_reason: body.reason } : {}), + // Sent only when marking someone else's task done (0041). + ...(body.doneBy ? { p_done_by: body.doneBy } : {}), }))); } else if (method === "POST" && pathId(path, /^\/operations\/([^/]+)\/handovers$/)) { response = asObject(unwrap(await db().rpc("hand_over_operation", { diff --git a/apps/web/src/screens/Hub.css b/apps/web/src/screens/Hub.css index 68fc7c18..4db83c3b 100644 --- a/apps/web/src/screens/Hub.css +++ b/apps/web/src/screens/Hub.css @@ -110,6 +110,7 @@ .c-hub-task.is-done .c-hub-task-copy strong { color: var(--hub-muted); text-decoration: line-through; } .c-hub-task input { width: 17px; height: 17px; flex: none; margin: 0; accent-color: #426d50; cursor: pointer; } .c-hub-late { color: var(--hub-warning); } +.c-hub-doneby { margin: 0 0 16px; } .c-hub-pinned { margin-top: 18px; padding: 0 15px; border-radius: 9px; background: var(--hub-sage); } .c-hub-pinned .c-hub-note-row { border: 0; } .c-hub-recent { padding: 6px 2px; } diff --git a/apps/web/src/screens/Hub.tsx b/apps/web/src/screens/Hub.tsx index fd07abfd..b0ce4580 100644 --- a/apps/web/src/screens/Hub.tsx +++ b/apps/web/src/screens/Hub.tsx @@ -6,7 +6,8 @@ import { Icon } from "../ui/Icon"; import { CalendarDate } from "../ui/CalendarDate"; import { CoverImage, coverToneClass } from "../ui/CoverImage"; import { isNotePinned, noteCategory, notePreview } from "../data/notePresentation"; -import { addDays, dateFromIso, todayIso } from "../data/operations"; +import { addDays, dateFromIso, firstNameOf, needsDoneBy, todayIso, type DoneBy, type OperationItem } from "../data/operations"; +import { DoneByChoice } from "../ui/DoneByChoice"; import { formatTimestampDay } from "../data/reminders"; import type { Hub, RouteId } from "../data/household"; import "./Hub.css"; @@ -43,8 +44,10 @@ function HubLocation({ hub }: { hub: Pick(null); + // Someone else's task, ticked: it asks who did it before it is marked done. + const [asking, setAsking] = useState(null); const [error, setError] = useState(""); const [feedback, setFeedback] = useState<{ text: string; undoId?: string } | null>(null); // A completed task leaves the list (and its checkbox with it). Move focus to @@ -68,11 +71,16 @@ function Responsibilities() { const end = addDays(today, 6); const items = operations.filter(item => item.status === "open" && item.date <= end && (item.date >= today || item.kind === "task")) .sort((a, b) => a.date.localeCompare(b.date)).slice(0, 5); - async function complete(id: string, title: string) { - setPending(id); setError(""); setFeedback(null); + async function complete(item: OperationItem, doneBy?: DoneBy) { + const { id, title } = item; + if (!doneBy && needsDoneBy(item, you.id)) { setAsking(id); setError(""); setFeedback(null); return; } + setAsking(null); setPending(id); setError(""); setFeedback(null); try { - const result = await toggleOperation(id); - setFeedback({ text: result.status === "done" ? `${title} marked done.` : `${title} done. Next: ${result.assignee ?? "unassigned"}.`, undoId: result.occurrenceId }); + const result = await toggleOperation(id, doneBy); + const holder = firstNameOf(item.assignee); + const said = doneBy === "assignee" ? `${title} marked done for ${holder}.` : doneBy === "me" ? `${title} done, covering ${holder}.` + : result.status === "done" ? `${title} marked done.` : `${title} done.`; + setFeedback({ text: result.status === "done" ? said : `${said} Next: ${result.assignee ?? "unassigned"}.`, undoId: result.occurrenceId }); setFocusTarget({ kind: "undo" }); } catch (cause) { setError(cause instanceof Error ? cause.message : "Could not update the task. Please try again."); } @@ -91,16 +99,19 @@ function Responsibilities() { return
{items.map(item => { + const choice = asking === item.id && void complete(item, doneBy)} + onCancel={() => { setAsking(null); setFocusTarget({ kind: "task", id: item.id }); }} />; const due = item.date === today ? "Today" : new Intl.DateTimeFormat("en-GB", { weekday: "short", day: "numeric", month: "short" }).format(dateFromIso(item.date)); const late = item.status === "open" && item.date < today; - return
+ return [
{item.kind === "task" && } -
; + checked={item.status === "done"} disabled={pending !== null} onChange={() => void complete(item)} />} +
, choice]; })}
{!items.length &&

Nothing planned for this week.

} diff --git a/apps/web/src/screens/Operations.tsx b/apps/web/src/screens/Operations.tsx index 2474cf7f..a27ea680 100644 --- a/apps/web/src/screens/Operations.tsx +++ b/apps/web/src/screens/Operations.tsx @@ -6,9 +6,11 @@ import { formatTimeLabel, isoFromDate, holdsTurn, + needsDoneBy, nextPerson, overdueDays, todayIso, + type DoneBy, type OperationItem, } from "../data/operations"; import { useApp } from "../state"; @@ -16,6 +18,7 @@ import { Icon } from "../ui/Icon"; import { Avatar } from "../ui/widgets"; import { CalendarDate } from "../ui/CalendarDate"; import { cx } from "../ui/cx"; +import { DoneByChoice } from "../ui/DoneByChoice"; type Cut = "week" | "people" | "month"; /** The late card, and which of its buttons, an Undo puts focus back on. */ @@ -63,6 +66,8 @@ export function OperationsScreen() { const [toast, setToast] = useState<{ text: string; undoId?: string; from?: LateAction } | null>(null); const [error, setError] = useState(""); const [undoing, setUndoing] = useState(false); + // The late task whose card is asking who did it. + const [asking, setAsking] = useState(null); const toastTimer = useRef(undefined); const toastRef = useRef(null); const toastHeld = useRef(false); @@ -187,17 +192,26 @@ export function OperationsScreen() { showDate(isoFromDate(date)); } - async function completeTask(item: OperationItem) { + async function completeTask(item: OperationItem, doneBy?: DoneBy) { + // Someone else's task first asks who did it. + if (!doneBy && needsDoneBy(item, you.id)) { + setAsking(item.id); + return; + } + setAsking(null); try { // The server decides the next date and person; report what it saved. - const result = await toggleOperation(item.id); + const result = await toggleOperation(item.id, doneBy); const handed = item.repeat !== "none" && item.rotation?.length ? firstNameOf(result.assignee ?? undefined) : ""; + const holder = firstNameOf(item.assignee); + const lead = doneBy === "assignee" ? `Marked done for ${holder}` : doneBy === "me" ? `Done, covering ${holder}` : "Done"; + const next = shortDateFormatter.format(dateFromIso(result.date)); flash( handed - ? `Handed to ${handed} · ${shortDateFormatter.format(dateFromIso(result.date))}` + ? doneBy ? `${lead} · next ${handed}, ${next}` : `Handed to ${handed} · ${next}` : item.repeat !== "none" - ? `Done · next ${shortDateFormatter.format(dateFromIso(result.date))}` - : `Done · ${item.title}`, + ? `${lead} · next ${next}` + : `${lead} · ${item.title}`, result.occurrenceId, { id: item.id, action: "done" }, ); @@ -282,9 +296,11 @@ export function OperationsScreen() { item={item} today={today} pending={mutationPending} + asking={asking === item.id} onOpen={() => go("operation", { operation: item.id })} onSkip={() => skipTask(item)} - onDone={() => completeTask(item)} + onDone={(doneBy) => completeTask(item, doneBy)} + onCancel={() => setAsking(null)} /> ))} @@ -510,24 +526,30 @@ function LateCard({ item, today, pending, + asking, onOpen, onSkip, onDone, + onCancel, }: { item: OperationItem; today: string; /** A change is saving: the actions wait so one click is not sent twice. */ pending: boolean; + /** Done was pressed on someone else's task: the card asks who did it. */ + asking: boolean; onOpen: () => void; onSkip: () => void; - onDone: () => void; + onDone: (doneBy?: DoneBy) => void; + onCancel: () => void; }) { + const card = useRef(null); const next = item.rotation?.length ? firstNameOf(nextPerson(item)) : ""; const who = firstNameOf(item.assignee); return ( -
+

{lateLabel(item.date, today)} · {who || "Unassigned"}

@@ -548,7 +570,19 @@ function LateCard({

{who ? `${who} still has this.` : "Nobody has this yet."}{next ? ` Next is ${next}.` : ""}

-
+ {asking ? ( + { + onCancel(); + // Back to the Done button the question replaced. + requestAnimationFrame(() => card.current?.querySelector('[data-late-action="done"]')?.focus()); + }} + /> + ) :
@@ -558,10 +592,10 @@ function LateCard({ Skip a week{" "}for {item.title} ) : null} - -
+
}
); } diff --git a/apps/web/src/state.tsx b/apps/web/src/state.tsx index da8b773e..8ef74d24 100644 --- a/apps/web/src/state.tsx +++ b/apps/web/src/state.tsx @@ -29,6 +29,7 @@ import { import { todayIso, type NewOperation, + type DoneBy, type OperationChange, type OperationItem, type OperationStore, @@ -381,7 +382,8 @@ export type AppState = { */ updateOperation: (id: string, item: NewOperation, expectedVersion?: number) => Promise; /** Completes an open task, or reopens a done one. The server advances repeats and rotation. */ - toggleOperation: (id: string) => Promise; + /** Marking someone else's task done says who did it (needsDoneBy). */ + toggleOperation: (id: string, doneBy?: DoneBy) => Promise; skipOperation: (id: string, reason?: string) => Promise; handOverOperation: (id: string, toMemberId: string, reason?: string) => Promise; /** Restores the task to exactly how it was before that action. */ @@ -1819,12 +1821,13 @@ export function AppProvider({ children }: { children: ReactNode }) { turnOwnerId: typeof raw.turnOwnerId === "string" ? raw.turnOwnerId : null, status: raw.status === "done" ? "done" : "open", } satisfies OperationChange; }), [mutate, refreshRecords]); - const taskAction = useCallback((id: string, action: "completed" | "skipped" | "reopened", reason?: string) => { + const taskAction = useCallback((id: string, action: "completed" | "skipped" | "reopened", reason?: string, doneBy?: DoneBy) => { const item = operationsRef.current.find((row) => row.id === id); if (!item) return Promise.reject(new Error("Schedule item not found")); return occurrenceCommand(`/operations/${encodeURIComponent(id)}/occurrences`, { action, expectedDate: item.date, today: todayIso(), requestId: newRequestId(), ...(reason?.trim() ? { reason: reason.trim() } : {}), + ...(doneBy ? { doneBy } : {}), }); }, [occurrenceCommand]); // The expected date and holder reject a handover made from a stale screen. @@ -1838,9 +1841,9 @@ export function AppProvider({ children }: { children: ReactNode }) { }, [occurrenceCommand]); const undoOperation = useCallback((occurrenceId: string) => occurrenceCommand(`/occurrences/${encodeURIComponent(occurrenceId)}/undo`, {}), [occurrenceCommand]); - const toggleOperation = useCallback((id: string) => { + const toggleOperation = useCallback((id: string, doneBy?: DoneBy) => { const item = operationsRef.current.find((row) => row.id === id); - if (item?.status !== "done") return taskAction(id, "completed"); + if (item?.status !== "done") return taskAction(id, "completed", undefined, doneBy); // Undo the completion when it is still the latest state; a task finished // before occurrences existed is reopened as a new recorded action. return item.undoOccurrenceId ? undoOperation(item.undoOccurrenceId) : taskAction(id, "reopened"); diff --git a/apps/web/src/ui/DoneByChoice.tsx b/apps/web/src/ui/DoneByChoice.tsx new file mode 100644 index 00000000..d219b94d --- /dev/null +++ b/apps/web/src/ui/DoneByChoice.tsx @@ -0,0 +1,111 @@ +import { useEffect, useId, useRef, useState, type CSSProperties, type KeyboardEvent, type PointerEvent } from "react"; +import { firstNameOf, type DoneBy } from "../data/operations"; +import { cx } from "./cx"; +import "./done-by.css"; + +/** How long an answer is held before it counts. */ +export const HOLD_MS = 900; + +/** + * Asked before marking done a task someone else holds: who did it? Each + * answer is pressed and held, so nobody marks a task done for someone by a + * stray tap, and both names go on the task (see needsDoneBy). + */ +export function DoneByChoice({ holder, title, disabled, onChoose, onCancel, className }: { + /** The holder's name, as the task shows it. */ + holder: string; + /** The task, for screen readers. */ + title: string; + disabled?: boolean; + onChoose: (doneBy: DoneBy) => void; + onCancel: () => void; + className?: string; +}) { + const id = useId(); + const name = firstNameOf(holder) || "They"; + const [hint, setHint] = useState(""); + const group = useRef(null); + // The question replaces the Done button, so focus moves to its first answer. + useEffect(() => { group.current?.querySelector("button")?.focus(); }, []); + return ( +
+

+ This is {name}’s turn. Who did it? ({title}) +

+

+ Press and hold your answer. Your name goes on it either way. +

+
+ onChoose("me")} onShort={setHint} /> + onChoose("assignee")} onShort={setHint} /> + +
+

{hint}

+
+ ); +} + +/** A button that acts only once it has been held down for HOLD_MS. */ +function HoldButton({ label, disabled, onHeld, onShort }: { + label: string; disabled?: boolean; onHeld: () => void; onShort: (hint: string) => void; +}) { + const [holding, setHolding] = useState(false); + const timer = useRef(undefined); + const done = useRef(false); + // A press that began here; the click that follows it is not a plain click. + const pressed = useRef(false); + useEffect(() => () => window.clearTimeout(timer.current), []); + const start = () => { + if (disabled || timer.current !== undefined) return; + done.current = false; + pressed.current = true; + setHolding(true); + onShort(""); + timer.current = window.setTimeout(() => { + timer.current = undefined; + done.current = true; + setHolding(false); + onHeld(); + }, HOLD_MS); + }; + const stop = () => { + if (timer.current === undefined) return; + window.clearTimeout(timer.current); + timer.current = undefined; + setHolding(false); + if (!done.current) onShort("Keep holding until the bar fills."); + }; + const isHoldKey = (event: KeyboardEvent) => event.key === " " || event.key === "Enter"; + return ( + + ); +} diff --git a/apps/web/src/ui/done-by.css b/apps/web/src/ui/done-by.css new file mode 100644 index 00000000..dcdb4286 --- /dev/null +++ b/apps/web/src/ui/done-by.css @@ -0,0 +1,23 @@ +/* "Who did it?" before marking someone else's task done (DoneByChoice.tsx). + Each answer fills while it is held and counts once full. */ +@layer screens { +.c-doneby { display: grid; gap: 6px; padding: 12px 14px; border-radius: var(--radius-control); background: var(--sage-soft); color: var(--ink); text-align: left; } +.c-doneby-ask { margin: 0; font-size: 14px; font-weight: 600; } +.c-doneby-how { margin: 0; font-size: 12.5px; line-height: 1.5; color: var(--ink-soft); } +.c-doneby-actions { display: flex; flex-wrap: wrap; gap: 8px; margin-top: 4px; } +.c-doneby-hold { + position: relative; overflow: hidden; isolation: isolate; + touch-action: none; user-select: none; -webkit-user-select: none; -webkit-touch-callout: none; + -webkit-tap-highlight-color: transparent; +} +.c-doneby-hold::before { + content: ""; position: absolute; inset: 0; z-index: -1; + background: var(--sage-mist); + transform: scaleX(0); transform-origin: left center; + transition: transform 120ms ease-out; +} +.c-doneby-hold.is-holding::before { transform: scaleX(1); transition: transform var(--hold-ms, 900ms) linear; } +.c-doneby-hold.is-holding { border-color: var(--sage); } +.c-doneby-hint { min-height: 18px; margin: 0; font-size: 12px; color: var(--ink-soft); } +.c-doneby-hint:empty { min-height: 0; } +} diff --git a/apps/web/src/windows/OperationsWindows.tsx b/apps/web/src/windows/OperationsWindows.tsx index b74fd5b7..12fbb68a 100644 --- a/apps/web/src/windows/OperationsWindows.tsx +++ b/apps/web/src/windows/OperationsWindows.tsx @@ -4,6 +4,7 @@ import { firstNameOf, holdsEdit, holdsTurn, + needsDoneBy, nextPerson, nextOccurrence, todayIso, @@ -21,6 +22,7 @@ import { LIMITS } from "../data/limits"; import { commandFailure, UNCERTAIN_RETRY_SAFE } from "../data/commands"; import { newRequestId } from "../data/requestId"; import { useFormError } from "../ui/components"; +import { DoneByChoice } from "../ui/DoneByChoice"; const OPERATION_CATEGORIES: Array<[OperationCategory, string]> = [ ["delivery", "Delivery"], @@ -51,8 +53,9 @@ export function OperationView() { personName, } = useApp(); const [confirmDelete, setConfirmDelete] = useState(false); - // One small panel at a time: skip with a reason, or hand over the turn. - const [panel, setPanel] = useState<"" | "skip" | "handover">(""); + // One small panel at a time: skip with a reason, hand over the turn, or + // say who did someone else's task. + const [panel, setPanel] = useState<"" | "skip" | "handover" | "doneby">(""); const [reason, setReason] = useState(""); const [handTo, setHandTo] = useState(""); const [error, setError] = useState(""); @@ -62,7 +65,7 @@ export function OperationView() { void action().then(() => { setMessage(success); setPanel(""); setReason(""); setHandTo(""); }) .catch((failure) => setError(failure instanceof Error ? failure.message : "Could not update item")); }; - const openPanel = (next: "skip" | "handover") => { + const openPanel = (next: "skip" | "handover" | "doneby") => { setError(""); setMessage(""); setReason(""); setHandTo(""); setPanel(next); }; if (!item) { @@ -155,6 +158,12 @@ export function OperationView() { ) : null} + {item.kind === "task" && item.lastDone ? ( +
+ Last done + {lastDoneLine(item.lastDone)} +
+ ) : null}
Visible to Everyone in this hub @@ -184,6 +193,20 @@ export function OperationView() { disabled={mutationPending} onUndo={(occurrenceId) => run(() => undoOperation(occurrenceId), "Undone. The task is back as it was.")} /> + {panel === "doneby" ? ( + { + const holder = firstNameOf(item.assignee) || "them"; + run(() => toggleOperation(item.id, doneBy), + `${doneBy === "assignee" ? `Marked done for ${holder}` : `Marked done, covering ${holder}`}. You can undo this in History.`); + }} + onCancel={() => setPanel("")} + /> + ) : null} {panel === "skip" ? (

Move this on a week? {firstNameOf(item.assignee) || "Whoever has it"} keeps it.

@@ -289,7 +312,10 @@ export function OperationView() { className="a-btn" type="button" disabled={mutationPending} - onClick={() => run(() => toggleOperation(item.id), item.status === "done" ? "Reopened." : "Marked done. You can undo this in History.")} + aria-expanded={needsDoneBy(item, you.id) ? panel === "doneby" : undefined} + onClick={() => needsDoneBy(item, you.id) + ? openPanel("doneby") + : run(() => toggleOperation(item.id), item.status === "done" ? "Reopened." : "Marked done. You can undo this in History.")} > {item.status === "done" ? "Mark open" : "Done"} @@ -346,6 +372,25 @@ const OCCURRENCE_VERB: Record = { /** The database keeps a reason to 200 characters. */ const OCCURRENCE_REASON_MAX = 200; +/** + * Who did it, and who marked it when that was someone else: "Ama, marked by + * Ben", or "Ben, covering Ama". Entries before 0041 name who pressed it. + */ +function doneByLine(entry: OperationOccurrence, nameOf: (userId: string) => string): string { + const actor = nameOf(entry.actorId); + if (entry.action !== "completed" || !entry.doneById) return actor; + if (entry.doneById !== entry.actorId) return `${nameOf(entry.doneById)}, marked by ${actor}`; + return entry.assigneeId && entry.assigneeId !== entry.actorId && entry.assignee + ? `${actor}, covering ${firstNameOf(entry.assignee)}` : actor; +} + +/** "Ama Owusu · 4 Oct · marked by Ben", "Ben · 4 Oct · covering Ama". */ +function lastDoneLine(lastDone: NonNullable): string { + return [lastDone.by, formatDay(lastDone.date), + lastDone.markedBy ? `marked by ${firstNameOf(lastDone.markedBy)}` : "", + lastDone.covering ? `covering ${firstNameOf(lastDone.covering)}` : ""].filter(Boolean).join(" · "); +} + /** What happened to each occurrence, newest first. Only the latest can be undone. */ function OccurrenceHistory({ history, undoId, nameOf, disabled, onUndo }: { history: OperationOccurrence[]; undoId: string | null; nameOf: (userId: string) => string; @@ -358,7 +403,7 @@ function OccurrenceHistory({ history, undoId, nameOf, disabled, onUndo }: { {history.map((entry) => (
- {OCCURRENCE_VERB[entry.action]} by {nameOf(entry.actorId)} on {formatDay(entry.actedOn)} + {OCCURRENCE_VERB[entry.action]} by {doneByLine(entry, nameOf)} on {formatDay(entry.actedOn)} {entry.action === "completed" || entry.action === "skipped" ? ` · was due ${formatDay(entry.dueOn)}${entry.assignee ? ` with ${firstNameOf(entry.assignee)}` : ""}` : ""} {entry.action === "completed" && entry.nextDate !== entry.dueOn ? ` · next ${formatDay(entry.nextDate)}${entry.nextAssignee ? ` with ${firstNameOf(entry.nextAssignee)}` : ""}` : ""} diff --git a/apps/web/tests/neon-lifecycle-contract.test.tsx b/apps/web/tests/neon-lifecycle-contract.test.tsx index 50e4fc96..992061ec 100644 --- a/apps/web/tests/neon-lifecycle-contract.test.tsx +++ b/apps/web/tests/neon-lifecycle-contract.test.tsx @@ -190,6 +190,10 @@ describe("hosted money lifecycle and task commands", () => { expect(mocks.rpc).toHaveBeenCalledWith("record_operation_occurrence", { p_operation_id: "task-1", p_action: "completed", p_expected_date: "2026-10-02", p_today: "2026-10-02", p_request_id: requestId }); expect(done).toMatchObject({ id: "occurrence-1", date: "2026-10-09", assignee: "Blake" }); + await send("/operations/task-1/occurrences", { action: "completed", expectedDate: "2026-10-02", today: "2026-10-02", requestId, doneBy: "assignee" }); + expect(mocks.rpc).toHaveBeenLastCalledWith("record_operation_occurrence", { + p_operation_id: "task-1", p_action: "completed", p_expected_date: "2026-10-02", p_today: "2026-10-02", p_request_id: requestId, + p_done_by: "assignee" }); await send("/occurrences/occurrence-1/undo", {}); expect(mocks.rpc).toHaveBeenLastCalledWith("undo_operation_occurrence", { p_occurrence_id: "occurrence-1" }); expect(mocks.from).not.toHaveBeenCalled(); diff --git a/apps/web/tests/task-done-by.test.tsx b/apps/web/tests/task-done-by.test.tsx new file mode 100644 index 00000000..45857abe --- /dev/null +++ b/apps/web/tests/task-done-by.test.tsx @@ -0,0 +1,174 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, fireEvent, render, screen, within } from "@testing-library/react"; +import { Router } from "wouter"; +import { memoryLocation } from "wouter/memory-location"; +import { App } from "../src/App"; +import { preloadAll } from "../src/shell/lazyComponent"; +import { AppProvider, useApp, type AppState } from "../src/state"; +import { api, post, loadCommunes, loadSnapshot } from "../src/live/api"; +import type { Commune, Snapshot } from "../src/live/model"; +import { addDays, todayIso, type OperationItem } from "../src/data/operations"; +import { HOLD_MS } from "../src/ui/DoneByChoice"; + +vi.mock("../src/live/api", async (importOriginal) => ({ + ...(await importOriginal()), + api: vi.fn(), post: vi.fn(), loadCommunes: vi.fn(), loadSnapshot: vi.fn(), +})); + +const user = { id: "user-1", name: "Test Person", email: "test@example.test", emailVerified: true }; +const commune: Commune = { id: "home-1", name: "Our home", kind: "household", currency: "GBP", role: "admin" }; +const initial: Snapshot = { + commune, + members: [ + { userId: user.id, name: user.name, role: "admin", active: true }, + { userId: "user-2", name: "Housemate Two", role: "member", active: true }, + ], + expenses: [], funds: [], notes: [], operations: [], notifications: [], reminderTargets: [], sentReminders: [], + recurringBills: [], splitTemplates: [], activityEvents: [], +}; +const today = todayIso(); +const rota: OperationItem = { + id: "task-1", hubId: commune.id, kind: "task", category: "chore", title: "Put the bins out", date: addDays(today, -2), allDay: true, + assignee: "Housemate Two", assigneeId: "user-2", rotation: [user.name, "Housemate Two"], rotationIds: [user.id, "user-2"], + repeat: "weekly", status: "open", notes: "", undoOccurrenceId: null, history: [], version: 1, +}; +let current: Snapshot; +let app: AppState; +function Capture() { app = useApp(); return null; } +function mount(path: string) { + const location = memoryLocation({ path, record: true }); + render(); +} +const modalDescriptor = Object.getOwnPropertyDescriptor(HTMLDialogElement.prototype, "showModal"); +const closeDescriptor = Object.getOwnPropertyDescriptor(HTMLDialogElement.prototype, "close"); +const originalScroll = Element.prototype.scrollIntoView; +// The screens and windows a browser loads on first use are loaded up front, +// so each test sees them drawn at once. +beforeAll(preloadAll); +beforeAll(() => { + Element.prototype.scrollIntoView = vi.fn(); + Object.defineProperty(HTMLDialogElement.prototype, "showModal", { configurable: true, value: function (this: HTMLDialogElement) { this.setAttribute("open", ""); } }); + Object.defineProperty(HTMLDialogElement.prototype, "close", { configurable: true, value: function (this: HTMLDialogElement) { this.removeAttribute("open"); } }); +}); +afterAll(() => { + Element.prototype.scrollIntoView = originalScroll; + if (modalDescriptor) Object.defineProperty(HTMLDialogElement.prototype, "showModal", modalDescriptor); + else delete (HTMLDialogElement.prototype as unknown as Record).showModal; + if (closeDescriptor) Object.defineProperty(HTMLDialogElement.prototype, "close", closeDescriptor); + else delete (HTMLDialogElement.prototype as unknown as Record).close; +}); +beforeEach(() => { + vi.resetAllMocks(); + sessionStorage.clear(); + current = structuredClone(initial); + current.operations = [structuredClone(rota)]; + vi.mocked(api).mockImplementation(async (path) => { + if (path === "/auth/me") return { user }; + if (path === "/reminders/usage") return { totalUsed: 0, totalLimit: 60, recipientLimit: 8, recipientUsage: {}, nextSlotAt: null }; + if (path.endsWith("/invites")) return { invites: [] }; + return { ok: true }; + }); + vi.mocked(post).mockResolvedValue({ ok: true }); + vi.mocked(loadCommunes).mockResolvedValue({ communes: [commune] }); + vi.mocked(loadSnapshot).mockImplementation(async () => current); +}); +afterEach(cleanup); + +const hold = () => act(() => new Promise((resolve) => setTimeout(resolve, HOLD_MS + 60))); +function serverCompletes() { + const next = addDays(today, 5); + current.operations = [{ ...rota, date: next, assignee: user.name, assigneeId: user.id }]; + return { id: "occurrence-1", duplicate: false, date: next, assignee: user.name, assigneeId: user.id, status: "open" }; +} + +describe("marking someone else's task done", () => { + it("asks who did it, ignores a tap, and records the answer once it is held", async () => { + vi.mocked(post).mockImplementation(async () => serverCompletes()); + mount("/operations"); + const done = await screen.findByRole("button", { name: /^Done · Test \(/ }); + await act(async () => { fireEvent.click(done); }); + expect(vi.mocked(post)).not.toHaveBeenCalled(); + const ask = screen.getByRole("group", { name: /This is Housemate’s turn\. Who did it\?/ }); + const theyDid = within(ask).getByRole("button", { name: "Housemate did it" }); + expect(document.activeElement).toBe(within(ask).getByRole("button", { name: "I did it" })); + // A tap is not enough. + fireEvent.pointerDown(theyDid, { button: 0, pointerId: 1 }); + fireEvent.pointerUp(theyDid, { button: 0, pointerId: 1 }); + fireEvent.click(theyDid); + expect(within(ask).getByRole("status").textContent).toBe("Keep holding until the bar fills."); + fireEvent.click(theyDid); + expect(within(ask).getByRole("status").textContent).toBe("Press and hold to confirm."); + expect(vi.mocked(post)).not.toHaveBeenCalled(); + // Held until the bar fills, it counts. + fireEvent.pointerDown(theyDid, { button: 0, pointerId: 1 }); + expect(theyDid.className).toContain("is-holding"); + await hold(); + expect(vi.mocked(post)).toHaveBeenCalledTimes(1); + expect(vi.mocked(post).mock.calls[0][1]).toMatchObject({ action: "completed", expectedDate: rota.date, doneBy: "assignee" }); + expect(document.querySelector(".ops-toast")?.textContent).toContain("Marked done for Housemate"); + }); + + it("takes a keyboard hold for I did it, and Cancel puts the Done button back", async () => { + vi.mocked(post).mockImplementation(async () => serverCompletes()); + mount("/operations"); + const done = await screen.findByRole("button", { name: /^Done · Test \(/ }); + await act(async () => { fireEvent.click(done); }); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(screen.queryByRole("group", { name: /Who did it/ })).toBeNull(); + await act(async () => { fireEvent.click(screen.getByRole("button", { name: /^Done · Test \(/ })); }); + const mine = screen.getByRole("button", { name: "I did it" }); + fireEvent.keyDown(mine, { key: " " }); + await hold(); + fireEvent.keyUp(mine, { key: " " }); + expect(vi.mocked(post).mock.calls[0][1]).toMatchObject({ action: "completed", doneBy: "me" }); + expect(document.querySelector(".ops-toast")?.textContent).toContain("Done, covering Housemate"); + }); + + it("marks your own task done at once, with no question", async () => { + current.operations = [{ ...rota, assignee: user.name, assigneeId: user.id }]; + vi.mocked(post).mockImplementation(async () => serverCompletes()); + mount("/operations"); + const done = await screen.findByRole("button", { name: /^Done · Housemate \(/ }); + await act(async () => { fireEvent.click(done); }); + expect(screen.queryByRole("group", { name: /Who did it/ })).toBeNull(); + expect(vi.mocked(post).mock.calls[0][1]).not.toHaveProperty("doneBy"); + }); + + it("asks in the task window and on This Hub too", async () => { + vi.mocked(post).mockImplementation(async () => serverCompletes()); + mount("/hub"); + const box = await screen.findByRole("checkbox", { name: "Mark Put the bins out done" }); + await act(async () => { fireEvent.click(box); }); + expect(vi.mocked(post)).not.toHaveBeenCalled(); + const hubAsk = screen.getByRole("group", { name: /This is Housemate’s turn/ }); + fireEvent.click(within(hubAsk).getByRole("button", { name: "Cancel" })); + + act(() => app.go("operation", { operation: "task-1" })); + const windowFrame = await screen.findByRole("dialog", { name: "Put the bins out" }); + await act(async () => { fireEvent.click(within(windowFrame).getByRole("button", { name: "Done" })); }); + const ask = within(windowFrame).getByRole("group", { name: /This is Housemate’s turn/ }); + fireEvent.pointerDown(within(ask).getByRole("button", { name: "Housemate did it" }), { button: 0, pointerId: 1 }); + await hold(); + expect(vi.mocked(post).mock.calls[0][1]).toMatchObject({ doneBy: "assignee" }); + expect(within(windowFrame).getByText("Marked done for Housemate. You can undo this in History.")).toBeTruthy(); + }); + + it("shows both names in the history and on the task", async () => { + current.operations = [{ ...rota, date: addDays(today, 5), assignee: user.name, assigneeId: user.id, + lastDone: { by: "Housemate Two", date: today, markedBy: user.name }, + history: [ + { id: "occurrence-2", action: "completed", dueOn: today, assignee: "Housemate Two", assigneeId: "user-2", actorId: user.id, doneById: "user-2", + actedOn: today, actedAt: new Date().toISOString(), nextDate: addDays(today, 5), nextAssignee: user.name, undoneAt: null, undoneBy: null }, + { id: "occurrence-1", action: "completed", dueOn: addDays(today, -7), assignee: "Housemate Two", assigneeId: "user-2", actorId: user.id, doneById: user.id, + actedOn: addDays(today, -7), actedAt: new Date(Date.now() - 7 * 86_400_000).toISOString(), nextDate: today, nextAssignee: "Housemate Two", undoneAt: null, undoneBy: null }, + ] }]; + mount("/operations"); + await screen.findByRole("group", { name: "Operations view" }); + act(() => app.go("operation", { operation: "task-1" })); + const windowFrame = await screen.findByRole("dialog", { name: "Put the bins out" }); + const entries = windowFrame.querySelectorAll(".operation-history > div"); + expect(entries[0].textContent).toContain("Completed by Housemate Two, marked by Test Person on"); + expect(entries[1].textContent).toContain("Completed by Test Person, covering Housemate on"); + expect(within(windowFrame).getByText("Last done").nextElementSibling?.textContent).toMatch(/^Housemate Two · .+ · marked by Test$/); + }); +}); diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index ad8fe711..991cc128 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -144,6 +144,14 @@ for a handoff, not a production release. best distance in local storage. If the connection returns mid-run the screen says so and waits for Open Commune instead of reconnecting. The "something went wrong" card (`shell/Recovery.tsx`) has no game. +- From 11 October, for review: who did a task (migration 0041). Marking done + a task someone else holds asks "This is Ama's turn. Who did it?" with "I + did it" and "Ama did it", each pressed and held until its bar fills + (`ui/DoneByChoice.tsx`, on the late card, the task window and This Hub's + week). The database refuses the completion without an answer. The task's + last done names who did it and who marked it ("Ama Owusu · 11 Oct · marked + by Ben") or whose turn they covered, and the history line says the same. + The rotation moves on from the holder's turn as before. - The website is `web/` (the editorial homepage); the earlier v2 site is in `web-backup/`. Launch names `website` (3004) and `website-backup` (3003). - No push, deployment, hosted migration or provider change has been made. Do diff --git a/infra/neon/README.md b/infra/neon/README.md index 28c08afd..e6247656 100644 --- a/infra/neon/README.md +++ b/infra/neon/README.md @@ -14,8 +14,8 @@ this is not a replacement of Neon Auth or a switch back to Supabase. - [Current design language and preserved layouts](../../docs/DESIGN-LANGUAGE.md) - [Local build-out from 5 October: migrations 0020 onwards](../../docs/LOCAL-BUILDOUT-2026-10-05.md) -Migrations **0015 to 0040 are local only**. They have not been applied to any -Neon branch. Release order when they are: apply 0013 to 0040 in numeric order +Migrations **0015 to 0041 are local only**. They have not been applied to any +Neon branch. Release order when they are: apply 0013 to 0041 in numeric order to the preview branch, refresh that branch's Data API schema cache, then deploy the matching web client. The new client calls `send_reminder_command`, `record_share_receipt`, `reverse_share_receipt`, `correct_expense`, @@ -125,6 +125,14 @@ released client reads it. Apply it and refresh the Data API schema cache before deploying the client that reads `label` and `date`; with an older database that client shows the old text. +0041 records who did a task. `record_operation_occurrence` gains an optional +`p_done_by` (`'me'` or `'assignee'`), and marking done a task that someone +else holds is refused without it, so a client released before 0041 can still +complete its own and unheld tasks but not someone else's. `last_done` gains +`markedBy` and `covering`, each occurrence a `done_by_id`, and the snapshot's +task history a `doneById`. Apply it, refresh the Data API schema cache, then +deploy the client that asks "Who did it?". + ## Writing a migration [`schema.sql`](schema.sql) is the schema the migrations produce, with every @@ -313,7 +321,7 @@ rate limits) are recorded in [`docs/AUTH-SETTINGS.md`](../../docs/AUTH-SETTINGS. Broad grants would let direct writes bypass ledger invariants. Keep preview branches separate from production. 3. Apply `migrations/0001_commune_core.sql` through - `migrations/0040_search_dates.sql` in numeric order to the + `migrations/0041_task_done_by.sql` in numeric order to the development branch via an **unpooled** connection. Run `tests/grants.sql`, review RLS, and run two-account signed-JWT tests before applying the same reviewed migrations to production. After a migration adds or changes a diff --git a/infra/neon/migrations/0041_task_done_by.sql b/infra/neon/migrations/0041_task_done_by.sql new file mode 100644 index 00000000..bc3b6d18 --- /dev/null +++ b/infra/neon/migrations/0041_task_done_by.sql @@ -0,0 +1,286 @@ +-- Who did a task. Anyone in a commune can mark any task done, and the +-- history already recorded who pressed the button, but the task's "last +-- done" credited the person whose turn it was, so a task could be marked +-- done for someone without their name and the tapper's sitting together. +-- +-- Marking done a task that someone else holds now has to say who did it: +-- 'me' the person marking it did it, covering the holder's turn +-- 'assignee' the holder did it, and the person marking it says so +-- record_operation_occurrence refuses the completion without an answer. +-- On a task the caller holds, or one nobody holds, no answer is needed. +-- +-- last_done names the person who did it (by) and, as well, whoever marked it +-- for them (markedBy) or whose turn they covered (covering). A task held by +-- a name with no account keeps crediting that name, with the person who +-- marked it when that is someone else. Each completion stores the member it +-- credits (done_by_id), and the snapshot's task history carries it as +-- doneById. The rotation moves on exactly as before: from the holder's turn. +-- +-- record_operation_occurrence gains an optional p_done_by; a client that +-- calls it without one keeps working on its own tasks. The functions are +-- written out in full, as 0040 left them apart from the done-by parts. +-- Apply after 0040. Release with the matching web client. +BEGIN; + +ALTER TABLE public.operation_occurrences + ADD COLUMN done_by_id text CHECK (done_by_id IS NULL OR length(done_by_id) BETWEEN 1 AND 128); + +-- The last completion: who did it, when, and who marked it for them or whose +-- turn they covered. +CREATE OR REPLACE FUNCTION public.valid_operation_last_done(p_last_done jsonb) RETURNS boolean + LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE + SET search_path TO 'pg_catalog' + AS $_$ +DECLARE + v_date text; + v_year integer; + v_month integer; + v_day integer; + v_days_in_month integer; +BEGIN + IF p_last_done IS NULL THEN + RETURN true; + END IF; + IF jsonb_typeof(p_last_done) <> 'object' + OR jsonb_typeof(p_last_done->'by') IS DISTINCT FROM 'string' + OR jsonb_typeof(p_last_done->'date') IS DISTINCT FROM 'string' + OR length(btrim(p_last_done->>'by')) NOT BETWEEN 1 AND 80 THEN + RETURN false; + END IF; + IF EXISTS ( + SELECT 1 FROM jsonb_object_keys(p_last_done) AS field(key) + WHERE field.key NOT IN ('by', 'date', 'markedBy', 'covering') + ) THEN + RETURN false; + END IF; + IF EXISTS ( + SELECT 1 FROM jsonb_each(p_last_done) AS field(key, value) + WHERE field.key IN ('markedBy', 'covering') + AND (jsonb_typeof(field.value) <> 'string' OR length(btrim(field.value #>> '{}')) NOT BETWEEN 1 AND 80) + ) THEN + RETURN false; + END IF; + v_date := p_last_done->>'date'; + IF v_date !~ '^[0-9]{4}-[0-9]{2}-[0-9]{2}$' THEN + RETURN false; + END IF; + v_year := substring(v_date, 1, 4)::integer; + v_month := substring(v_date, 6, 2)::integer; + v_day := substring(v_date, 9, 2)::integer; + IF v_year = 0 OR v_month NOT BETWEEN 1 AND 12 OR v_day < 1 THEN + RETURN false; + END IF; + v_days_in_month := CASE + WHEN v_month IN (4, 6, 9, 11) THEN 30 + WHEN v_month = 2 THEN CASE + WHEN mod(v_year, 400) = 0 OR + (mod(v_year, 4) = 0 AND mod(v_year, 100) <> 0) THEN 29 + ELSE 28 END + ELSE 31 END; + RETURN v_day <= v_days_in_month; +END; +$_$; + +DROP FUNCTION public.record_operation_occurrence(uuid, text, date, date, uuid, text); +CREATE FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text DEFAULT NULL::text, p_done_by text DEFAULT NULL::text) RETURNS jsonb + LANGUAGE plpgsql SECURITY DEFINER + SET search_path TO 'public', 'pg_temp' + AS $$ +DECLARE + v_actor text := commune_private.data_api_user_id(); + v_commune_id uuid; + v_op public.commune_operations%ROWTYPE; + v_replay public.operation_occurrences%ROWTYPE; + v_actor_name text; + v_date date; v_assignee text; v_assignee_id text; v_turn_owner_id text; v_status text; v_last_done jsonb; + v_id uuid; v_description text; v_reason text; + v_holder_name text; v_held_by_name text; v_done_by_id text; +BEGIN + IF p_request_id IS NULL THEN + RAISE EXCEPTION 'A request id is required to update a task' USING ERRCODE = '22023'; + END IF; + -- The commune first, then the task, as in update_operation: the order a + -- member leaving takes them in. The other order lets the two wait on each + -- other until one fails as a deadlock. + SELECT commune_id INTO v_commune_id FROM public.commune_operations WHERE id = p_operation_id; + PERFORM 1 FROM public.communes WHERE id = v_commune_id FOR UPDATE; + -- The row lock serialises two people acting on the same task. + SELECT * INTO v_op FROM public.commune_operations WHERE id = p_operation_id FOR UPDATE; + IF v_op.id IS NULL OR NOT public.is_commune_member(v_op.commune_id) THEN + RAISE EXCEPTION 'Not a commune member' USING ERRCODE = '42501'; + END IF; + SELECT * INTO v_replay FROM public.operation_occurrences WHERE actor_id = v_actor AND request_id = p_request_id; + IF FOUND THEN + IF v_replay.operation_id <> p_operation_id OR v_replay.action IS DISTINCT FROM p_action THEN + RAISE EXCEPTION 'This request was already used for a different change' USING ERRCODE = '22023'; + END IF; + RETURN commune_private.occurrence_result(v_replay.id, true, p_operation_id); + END IF; + IF v_op.deleted_at IS NOT NULL THEN + RAISE EXCEPTION 'This task has been removed' USING ERRCODE = 'P0001'; + END IF; + IF p_action IS NULL OR p_action NOT IN ('completed', 'skipped', 'reopened') THEN + RAISE EXCEPTION 'Unknown task action' USING ERRCODE = '22023'; + END IF; + v_reason := commune_private.occurrence_reason(p_reason); + IF v_reason IS NOT NULL AND p_action <> 'skipped' THEN + RAISE EXCEPTION 'Only a skip or a handover takes a reason' USING ERRCODE = '22023'; + END IF; + IF p_done_by IS NOT NULL AND (p_done_by NOT IN ('me', 'assignee') OR p_action <> 'completed') THEN + RAISE EXCEPTION 'Only marking a task done says who did it: you, or the person who has it' USING ERRCODE = '22023'; + END IF; + -- A commune has no time zone, so the member's own date decides "today". + -- It may differ from the server's UTC date by at most one day. + IF p_today IS NULL OR p_today NOT BETWEEN current_date - 1 AND current_date + 1 THEN + RAISE EXCEPTION 'Check the date on this device and try again' USING ERRCODE = '22023'; + END IF; + IF v_op.kind <> 'task' THEN + RAISE EXCEPTION 'Only tasks can be completed, skipped or reopened' USING ERRCODE = '22023'; + END IF; + IF p_expected_date IS DISTINCT FROM v_op.date THEN + RAISE EXCEPTION 'This task has already moved on. Refresh to see who has it now.' USING ERRCODE = 'P0001'; + END IF; + SELECT coalesce(u.name, 'Member') INTO v_actor_name FROM commune_private.accounts(ARRAY[v_actor]) u; + v_actor_name := left(coalesce(v_actor_name, 'Member'), 80); + v_date := v_op.date; v_assignee := v_op.assignee; v_assignee_id := v_op.assignee_id; + v_turn_owner_id := v_op.turn_owner_id; v_status := v_op.status; v_last_done := v_op.last_done; + + IF p_action = 'completed' THEN + IF v_op.status <> 'open' THEN + RAISE EXCEPTION 'This task is already done' USING ERRCODE = 'P0001'; + END IF; + -- Who did it. Someone else's task needs an answer, so nothing is marked + -- done for a person without both names on it. + IF v_op.assignee_id IS NOT NULL THEN + v_holder_name := commune_private.member_name(v_op.assignee_id); + END IF; + IF v_op.assignee_id IS NOT NULL AND v_op.assignee_id <> v_actor THEN + IF p_done_by IS NULL THEN + RAISE EXCEPTION 'This is %''s turn. Say whether you did it or they did.', split_part(v_holder_name, ' ', 1) USING ERRCODE = '22023'; + END IF; + v_done_by_id := CASE p_done_by WHEN 'me' THEN v_actor ELSE v_op.assignee_id END; + ELSIF p_done_by = 'assignee' AND v_op.assignee_id IS NULL THEN + RAISE EXCEPTION 'Nobody has this task. Mark it done as yourself.' USING ERRCODE = '22023'; + ELSE + v_held_by_name := left(nullif(btrim(coalesce(v_op.assignee, '')), ''), 80); + -- A task held by a name with no account cannot say who that was. + v_done_by_id := CASE WHEN v_op.assignee_id IS NULL AND v_held_by_name IS NOT NULL THEN NULL ELSE v_actor END; + END IF; + v_last_done := CASE + WHEN v_done_by_id IS NULL THEN jsonb_strip_nulls(jsonb_build_object('by', v_held_by_name, 'date', p_today, + 'markedBy', CASE WHEN v_held_by_name <> v_actor_name THEN v_actor_name END)) + WHEN v_done_by_id <> v_actor THEN jsonb_build_object('by', v_holder_name, 'date', p_today, 'markedBy', v_actor_name) + WHEN v_op.assignee_id IS NOT NULL AND v_op.assignee_id <> v_actor THEN + jsonb_build_object('by', v_actor_name, 'date', p_today, 'covering', v_holder_name) + ELSE jsonb_build_object('by', v_actor_name, 'date', p_today) END; + IF v_op.repeat = 'none' THEN + v_status := 'done'; + ELSE + -- Next occurrence strictly after today. + LOOP + IF v_op.repeat = 'weekly' THEN + v_date := v_date + 7; + ELSE + -- Step month by month from the anchor day, clamped to each month's + -- length, so 31 January goes to 28 February and back to 31 March. + v_date := (date_trunc('month', v_date) + interval '1 month')::date + + (least(coalesce(v_op.anchor_day, extract(day FROM v_op.date)::int), + extract(day FROM (date_trunc('month', v_date) + interval '2 month - 1 day'))::int) - 1); + END IF; + EXIT WHEN v_date > p_today; + END LOOP; + IF v_op.rotation_ids IS NOT NULL THEN + -- The turn moves on from whoever's turn it was, not from the person + -- covering it. Members who have left are passed over. + v_assignee_id := coalesce( + commune_private.next_in_rotation(v_op.commune_id, v_op.rotation_ids, coalesce(v_op.turn_owner_id, v_op.assignee_id)), + v_op.turn_owner_id, v_op.assignee_id); + v_assignee := CASE WHEN v_assignee_id IS NULL THEN v_op.assignee ELSE commune_private.member_name(v_assignee_id) END; + ELSIF v_op.turn_owner_id IS NOT NULL THEN + v_assignee_id := v_op.turn_owner_id; + v_assignee := commune_private.member_name(v_assignee_id); + END IF; + END IF; + -- A cover lasts for one occurrence. + v_turn_owner_id := NULL; + v_description := CASE WHEN v_last_done ? 'markedBy' THEN 'Marked done for ' || (v_last_done->>'by') + WHEN v_last_done ? 'covering' THEN 'Completed, covering ' || (v_last_done->>'covering') + ELSE 'Completed' END + || CASE WHEN v_op.repeat = 'none' THEN '' + WHEN v_assignee_id IS DISTINCT FROM v_op.assignee_id AND v_assignee IS NOT NULL THEN '. Next: ' || v_assignee + ELSE '. Next due ' || to_char(v_date, 'FMDD Mon') END; + ELSIF p_action = 'skipped' THEN + IF v_op.status <> 'open' OR v_op.repeat = 'none' THEN + RAISE EXCEPTION 'Only an open repeating task can be skipped' USING ERRCODE = 'P0001'; + END IF; + v_date := greatest(v_op.date, p_today) + 7; + v_description := 'Skipped to ' || to_char(v_date, 'FMDD Mon') || coalesce(': ' || v_reason, ''); + ELSE + IF v_op.status <> 'done' THEN + RAISE EXCEPTION 'This task is already open' USING ERRCODE = 'P0001'; + END IF; + v_status := 'open'; + v_description := 'Reopened'; + END IF; + + PERFORM set_config('commune.occurrence_event', 'on', true); + UPDATE public.commune_operations SET date = v_date, assignee = v_assignee, assignee_id = v_assignee_id, + turn_owner_id = v_turn_owner_id, status = v_status, last_done = v_last_done, updated_at = now() + WHERE id = p_operation_id; + PERFORM set_config('commune.occurrence_event', 'off', true); + -- clock_timestamp(), taken while the row lock is held, orders occurrences + -- by when they were recorded rather than when their transaction began. + INSERT INTO public.operation_occurrences (operation_id, commune_id, action, due_on, assignee, actor_id, acted_on, acted_at, request_id, + prev_date, prev_assignee, prev_status, prev_last_done, next_date, next_assignee, next_status, next_last_done, + assignee_id, prev_assignee_id, next_assignee_id, reason, prev_turn_owner_id, next_turn_owner_id, done_by_id) + VALUES (p_operation_id, v_op.commune_id, p_action, v_op.date, v_op.assignee, v_actor, p_today, clock_timestamp(), p_request_id, + v_op.date, v_op.assignee, v_op.status, v_op.last_done, v_date, v_assignee, v_status, v_last_done, + v_op.assignee_id, v_op.assignee_id, v_assignee_id, v_reason, v_op.turn_owner_id, v_turn_owner_id, v_done_by_id) + RETURNING id INTO v_id; + INSERT INTO public.commune_events (commune_id, actor_id, kind, entity_id, title, description) + VALUES (v_op.commune_id, v_actor, 'operations', p_operation_id, v_op.title, left(v_description, 240)); + RETURN commune_private.occurrence_result(v_id, false, p_operation_id); +END; +$$; + +REVOKE ALL ON FUNCTION public.record_operation_occurrence(uuid, text, date, date, uuid, text, text) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION public.record_operation_occurrence(uuid, text, date, date, uuid, text, text) TO authenticated; + +-- The task history says who each completion credits. +CREATE OR REPLACE FUNCTION commune_private.snapshot_operations(p_commune_id uuid) RETURNS jsonb + LANGUAGE sql STABLE + SET search_path TO 'public', 'pg_temp' + AS $$ + WITH names AS MATERIALIZED (SELECT commune_private.member_names(p_commune_id) AS map) + SELECT coalesce(jsonb_agg(jsonb_build_object('id',o.id,'hubId',o.commune_id,'kind',o.kind,'category',o.category,'title',o.title,'date',o.date,'time',o.time,'allDay',o.all_day, + 'version',o.version,'createdBy',o.created_by, + 'assignee',CASE WHEN o.assignee_id IS NULL THEN o.assignee ELSE coalesce(names.map->>o.assignee_id, commune_private.member_name(o.assignee_id)) END, + 'assigneeId',o.assignee_id, + 'turnOwnerId',o.turn_owner_id, + 'turnOwner',CASE WHEN o.turn_owner_id IS NULL THEN NULL ELSE coalesce(names.map->>o.turn_owner_id, commune_private.member_name(o.turn_owner_id)) END, + 'rotation',CASE WHEN o.rotation_ids IS NULL THEN o.rotation ELSE + (SELECT jsonb_agg(coalesce(names.map->>e.value, commune_private.member_name(e.value)) ORDER BY e.ord) FROM jsonb_array_elements_text(o.rotation_ids) WITH ORDINALITY e(value, ord)) END, + 'rotationIds',o.rotation_ids, + 'lastDone',o.last_done,'repeat',o.repeat,'anchorDay',o.anchor_day,'status',o.status,'notes',o.notes, + -- The latest action, only while the task is still exactly as it left it. + 'undoOccurrenceId',(SELECT u.id FROM public.operation_occurrences u WHERE u.operation_id=o.id AND u.undone_at IS NULL + AND u.next_date=o.date AND u.next_assignee_id IS NOT DISTINCT FROM o.assignee_id + AND (o.assignee_id IS NOT NULL OR u.next_assignee IS NOT DISTINCT FROM o.assignee) + AND u.next_turn_owner_id IS NOT DISTINCT FROM o.turn_owner_id + AND u.next_status=o.status AND u.next_last_done IS NOT DISTINCT FROM o.last_done + AND NOT EXISTS(SELECT 1 FROM public.operation_occurrences later WHERE later.operation_id=o.id AND later.undone_at IS NULL AND later.id<>u.id AND later.acted_at>=u.acted_at) + ORDER BY u.acted_at DESC LIMIT 1), + 'history',coalesce((SELECT jsonb_agg(jsonb_build_object('id',h.id,'action',h.action,'dueOn',h.due_on, + 'assignee',CASE WHEN h.assignee_id IS NULL THEN h.assignee ELSE coalesce(names.map->>h.assignee_id, commune_private.member_name(h.assignee_id)) END,'assigneeId',h.assignee_id, + 'actorId',h.actor_id,'actedOn',h.acted_on,'actedAt',h.acted_at,'nextDate',h.next_date, + 'nextAssignee',CASE WHEN h.next_assignee_id IS NULL THEN h.next_assignee ELSE coalesce(names.map->>h.next_assignee_id, commune_private.member_name(h.next_assignee_id)) END,'nextAssigneeId',h.next_assignee_id, + 'reason',h.reason,'doneById',h.done_by_id, + 'undoneAt',h.undone_at,'undoneBy',h.undone_by) ORDER BY h.acted_at DESC, h.id) + FROM (SELECT * FROM public.operation_occurrences x WHERE x.operation_id=o.id ORDER BY x.acted_at DESC, x.id LIMIT 12) h),'[]'::jsonb) + ) ORDER BY o.date,o.time),'[]'::jsonb) + FROM public.commune_operations o CROSS JOIN names WHERE o.commune_id=p_commune_id AND o.deleted_at IS NULL; +$$; + +NOTIFY pgrst, 'reload schema'; + +COMMIT; diff --git a/infra/neon/schema.sql b/infra/neon/schema.sql index b130f91f..c2f50dfe 100644 --- a/infra/neon/schema.sql +++ b/infra/neon/schema.sql @@ -1252,7 +1252,7 @@ CREATE FUNCTION commune_private.snapshot_operations(p_commune_id uuid) RETURNS j 'assignee',CASE WHEN h.assignee_id IS NULL THEN h.assignee ELSE coalesce(names.map->>h.assignee_id, commune_private.member_name(h.assignee_id)) END,'assigneeId',h.assignee_id, 'actorId',h.actor_id,'actedOn',h.acted_on,'actedAt',h.acted_at,'nextDate',h.next_date, 'nextAssignee',CASE WHEN h.next_assignee_id IS NULL THEN h.next_assignee ELSE coalesce(names.map->>h.next_assignee_id, commune_private.member_name(h.next_assignee_id)) END,'nextAssigneeId',h.next_assignee_id, - 'reason',h.reason, + 'reason',h.reason,'doneById',h.done_by_id, 'undoneAt',h.undone_at,'undoneBy',h.undone_by) ORDER BY h.acted_at DESC, h.id) FROM (SELECT * FROM public.operation_occurrences x WHERE x.operation_id=o.id ORDER BY x.acted_at DESC, x.id LIMIT 12) h),'[]'::jsonb) ) ORDER BY o.date,o.time),'[]'::jsonb) @@ -2623,10 +2623,10 @@ $$; -- --- Name: record_operation_occurrence(uuid, text, date, date, uuid, text); Type: FUNCTION; Schema: public; Owner: - +-- Name: record_operation_occurrence(uuid, text, date, date, uuid, text, text); Type: FUNCTION; Schema: public; Owner: - -- -CREATE FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text DEFAULT NULL::text) RETURNS jsonb +CREATE FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text DEFAULT NULL::text, p_done_by text DEFAULT NULL::text) RETURNS jsonb LANGUAGE plpgsql SECURITY DEFINER SET search_path TO 'public', 'pg_temp' AS $$ @@ -2638,6 +2638,7 @@ DECLARE v_actor_name text; v_date date; v_assignee text; v_assignee_id text; v_turn_owner_id text; v_status text; v_last_done jsonb; v_id uuid; v_description text; v_reason text; + v_holder_name text; v_held_by_name text; v_done_by_id text; BEGIN IF p_request_id IS NULL THEN RAISE EXCEPTION 'A request id is required to update a task' USING ERRCODE = '22023'; @@ -2669,6 +2670,9 @@ BEGIN IF v_reason IS NOT NULL AND p_action <> 'skipped' THEN RAISE EXCEPTION 'Only a skip or a handover takes a reason' USING ERRCODE = '22023'; END IF; + IF p_done_by IS NOT NULL AND (p_done_by NOT IN ('me', 'assignee') OR p_action <> 'completed') THEN + RAISE EXCEPTION 'Only marking a task done says who did it: you, or the person who has it' USING ERRCODE = '22023'; + END IF; -- A commune has no time zone, so the member's own date decides "today". -- It may differ from the server's UTC date by at most one day. IF p_today IS NULL OR p_today NOT BETWEEN current_date - 1 AND current_date + 1 THEN @@ -2689,9 +2693,32 @@ BEGIN IF v_op.status <> 'open' THEN RAISE EXCEPTION 'This task is already done' USING ERRCODE = 'P0001'; END IF; + -- Who did it. Someone else's task needs an answer, so nothing is marked + -- done for a person without both names on it. + IF v_op.assignee_id IS NOT NULL THEN + v_holder_name := commune_private.member_name(v_op.assignee_id); + END IF; + IF v_op.assignee_id IS NOT NULL AND v_op.assignee_id <> v_actor THEN + IF p_done_by IS NULL THEN + RAISE EXCEPTION 'This is %''s turn. Say whether you did it or they did.', split_part(v_holder_name, ' ', 1) USING ERRCODE = '22023'; + END IF; + v_done_by_id := CASE p_done_by WHEN 'me' THEN v_actor ELSE v_op.assignee_id END; + ELSIF p_done_by = 'assignee' AND v_op.assignee_id IS NULL THEN + RAISE EXCEPTION 'Nobody has this task. Mark it done as yourself.' USING ERRCODE = '22023'; + ELSE + v_held_by_name := left(nullif(btrim(coalesce(v_op.assignee, '')), ''), 80); + -- A task held by a name with no account cannot say who that was. + v_done_by_id := CASE WHEN v_op.assignee_id IS NULL AND v_held_by_name IS NOT NULL THEN NULL ELSE v_actor END; + END IF; + v_last_done := CASE + WHEN v_done_by_id IS NULL THEN jsonb_strip_nulls(jsonb_build_object('by', v_held_by_name, 'date', p_today, + 'markedBy', CASE WHEN v_held_by_name <> v_actor_name THEN v_actor_name END)) + WHEN v_done_by_id <> v_actor THEN jsonb_build_object('by', v_holder_name, 'date', p_today, 'markedBy', v_actor_name) + WHEN v_op.assignee_id IS NOT NULL AND v_op.assignee_id <> v_actor THEN + jsonb_build_object('by', v_actor_name, 'date', p_today, 'covering', v_holder_name) + ELSE jsonb_build_object('by', v_actor_name, 'date', p_today) END; IF v_op.repeat = 'none' THEN v_status := 'done'; - v_last_done := jsonb_build_object('by', v_actor_name, 'date', p_today); ELSE -- Next occurrence strictly after today. LOOP @@ -2717,15 +2744,15 @@ BEGIN v_assignee_id := v_op.turn_owner_id; v_assignee := commune_private.member_name(v_assignee_id); END IF; - -- The person who held it, the cover included, is credited. - v_last_done := jsonb_build_object('by', coalesce(CASE WHEN v_op.assignee_id IS NOT NULL THEN commune_private.member_name(v_op.assignee_id) END, - nullif(btrim(v_op.assignee), ''), v_actor_name), 'date', p_today); END IF; -- A cover lasts for one occurrence. v_turn_owner_id := NULL; - v_description := CASE WHEN v_op.repeat = 'none' THEN 'Completed' - WHEN v_assignee_id IS DISTINCT FROM v_op.assignee_id AND v_assignee IS NOT NULL THEN 'Completed. Next: ' || v_assignee - ELSE 'Completed. Next due ' || to_char(v_date, 'FMDD Mon') END; + v_description := CASE WHEN v_last_done ? 'markedBy' THEN 'Marked done for ' || (v_last_done->>'by') + WHEN v_last_done ? 'covering' THEN 'Completed, covering ' || (v_last_done->>'covering') + ELSE 'Completed' END + || CASE WHEN v_op.repeat = 'none' THEN '' + WHEN v_assignee_id IS DISTINCT FROM v_op.assignee_id AND v_assignee IS NOT NULL THEN '. Next: ' || v_assignee + ELSE '. Next due ' || to_char(v_date, 'FMDD Mon') END; ELSIF p_action = 'skipped' THEN IF v_op.status <> 'open' OR v_op.repeat = 'none' THEN RAISE EXCEPTION 'Only an open repeating task can be skipped' USING ERRCODE = 'P0001'; @@ -2749,10 +2776,10 @@ BEGIN -- by when they were recorded rather than when their transaction began. INSERT INTO public.operation_occurrences (operation_id, commune_id, action, due_on, assignee, actor_id, acted_on, acted_at, request_id, prev_date, prev_assignee, prev_status, prev_last_done, next_date, next_assignee, next_status, next_last_done, - assignee_id, prev_assignee_id, next_assignee_id, reason, prev_turn_owner_id, next_turn_owner_id) + assignee_id, prev_assignee_id, next_assignee_id, reason, prev_turn_owner_id, next_turn_owner_id, done_by_id) VALUES (p_operation_id, v_op.commune_id, p_action, v_op.date, v_op.assignee, v_actor, p_today, clock_timestamp(), p_request_id, v_op.date, v_op.assignee, v_op.status, v_op.last_done, v_date, v_assignee, v_status, v_last_done, - v_op.assignee_id, v_op.assignee_id, v_assignee_id, v_reason, v_op.turn_owner_id, v_turn_owner_id) + v_op.assignee_id, v_op.assignee_id, v_assignee_id, v_reason, v_op.turn_owner_id, v_turn_owner_id, v_done_by_id) RETURNING id INTO v_id; INSERT INTO public.commune_events (commune_id, actor_id, kind, entity_id, title, description) VALUES (v_op.commune_id, v_actor, 'operations', p_operation_id, v_op.title, left(v_description, 240)); @@ -4348,7 +4375,14 @@ BEGIN END IF; IF EXISTS ( SELECT 1 FROM jsonb_object_keys(p_last_done) AS field(key) - WHERE field.key NOT IN ('by', 'date') + WHERE field.key NOT IN ('by', 'date', 'markedBy', 'covering') + ) THEN + RETURN false; + END IF; + IF EXISTS ( + SELECT 1 FROM jsonb_each(p_last_done) AS field(key, value) + WHERE field.key IN ('markedBy', 'covering') + AND (jsonb_typeof(field.value) <> 'string' OR length(btrim(field.value #>> '{}')) NOT BETWEEN 1 AND 80) ) THEN RETURN false; END IF; @@ -5146,8 +5180,10 @@ CREATE TABLE public.operation_occurrences ( reason text, prev_turn_owner_id text, next_turn_owner_id text, + done_by_id text, CONSTRAINT operation_occurrences_action_check CHECK ((action = ANY (ARRAY['completed'::text, 'skipped'::text, 'reopened'::text, 'handed_over'::text]))), CONSTRAINT operation_occurrences_check CHECK (((undone_at IS NULL) = (undone_by IS NULL))), + CONSTRAINT operation_occurrences_done_by_id_check CHECK (((done_by_id IS NULL) OR ((length(done_by_id) >= 1) AND (length(done_by_id) <= 128)))), CONSTRAINT operation_occurrences_reason_check CHECK (((reason IS NULL) OR ((length(reason) >= 1) AND (length(reason) <= 200)))) ); @@ -8041,11 +8077,11 @@ GRANT ALL ON FUNCTION public.record_fund_contribution_command(p_commitment_id uu -- --- Name: FUNCTION record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text); Type: ACL; Schema: public; Owner: - +-- Name: FUNCTION record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text, p_done_by text); Type: ACL; Schema: public; Owner: - -- -REVOKE ALL ON FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text) FROM PUBLIC; -GRANT ALL ON FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text) TO authenticated; +REVOKE ALL ON FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text, p_done_by text) FROM PUBLIC; +GRANT ALL ON FUNCTION public.record_operation_occurrence(p_operation_id uuid, p_action text, p_expected_date date, p_today date, p_request_id uuid, p_reason text, p_done_by text) TO authenticated; -- diff --git a/infra/neon/tests/account_lookups.sql b/infra/neon/tests/account_lookups.sql index aaaa5495..b30a0e2d 100644 --- a/infra/neon/tests/account_lookups.sql +++ b/infra/neon/tests/account_lookups.sql @@ -30,8 +30,8 @@ BEGIN RETURNING id INTO v_task; INSERT INTO public.commune_operations (commune_id, kind, category, title, date, assignee_id, created_by) VALUES (v_home, 'task', 'chore', 'Plants', current_date, v_gone, v_alice); - -- Bea hands the bins to Cal, then Cal leaves. - PERFORM public.record_operation_occurrence(v_task, 'completed', current_date, current_date, gen_random_uuid()); + -- Alice marks Bea's turn done for her, it passes to Cal, then Cal leaves. + PERFORM public.record_operation_occurrence(v_task, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'assignee'); PERFORM public.manage_commune_member(v_home, v_cal, 'remove'); PERFORM set_config('test.home', v_home::text, true); diff --git a/infra/neon/tests/grants.sql b/infra/neon/tests/grants.sql index 427f3085..91e89f58 100644 --- a/infra/neon/tests/grants.sql +++ b/infra/neon/tests/grants.sql @@ -49,7 +49,7 @@ BEGIN 'public.reverse_share_receipt(uuid,text)', 'public.correct_expense(uuid,integer,text,bigint,jsonb,date,text,text,text,uuid)', 'public.void_expense(uuid,integer,text,uuid)', - 'public.record_operation_occurrence(uuid,text,date,date,uuid,text)', + 'public.record_operation_occurrence(uuid,text,date,date,uuid,text,text)', 'public.hand_over_operation(uuid,text,date,text,date,text,uuid)', 'public.post_announcement(uuid,text,text,boolean,uuid)', 'public.acknowledge_announcement(uuid)', diff --git a/infra/neon/tests/operation_handovers.sql b/infra/neon/tests/operation_handovers.sql index 6a72fd59..1ff9e7d7 100644 --- a/infra/neon/tests/operation_handovers.sql +++ b/infra/neon/tests/operation_handovers.sql @@ -105,7 +105,7 @@ BEGIN RAISE EXCEPTION 'Undo did not restore the holder: %', row_to_json(v_row); END IF; PERFORM public.hand_over_operation(v_op, d, current_date, a, current_date, NULL, gen_random_uuid()); - v_result := public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid()); + v_result := public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'assignee'); SELECT * INTO v_row FROM public.commune_operations WHERE id = v_op; IF v_row.assignee_id <> b OR v_row.turn_owner_id IS NOT NULL OR v_row.last_done->>'by' <> 'Dana' OR v_row.date <> current_date + 7 THEN RAISE EXCEPTION 'Completion after a cover went wrong: %', row_to_json(v_row); diff --git a/infra/neon/tests/operation_people.sql b/infra/neon/tests/operation_people.sql index b9cde96f..93a28e7b 100644 --- a/infra/neon/tests/operation_people.sql +++ b/infra/neon/tests/operation_people.sql @@ -40,8 +40,9 @@ BEGIN RAISE EXCEPTION 'Names were not copied from ids: % %', v_row.assignee, v_row.rotation; END IF; - -- Completing passes it to the next id, not the first "Sam". - v_result := public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid()); + -- Completing passes it to the next id, not the first "Sam". Alice did + -- Sam's turn herself. + v_result := public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'me'); IF v_result->>'assigneeId' <> '00000000-0000-4000-8000-0000000000b2' THEN RAISE EXCEPTION 'Rotation did not advance by id: %', v_result; END IF; diff --git a/infra/neon/tests/task_done_by.sql b/infra/neon/tests/task_done_by.sql new file mode 100644 index 00000000..113c2dcd --- /dev/null +++ b/infra/neon/tests/task_done_by.sql @@ -0,0 +1,169 @@ +-- Who did a task (0041). Run only in an empty disposable local DB built from +-- infra/local/bootstrap.sql, every migration and after-core.sql (app_auth +-- stands in for Neon Auth). Everything is rolled back. +BEGIN; + +DO $$ +DECLARE v_home uuid; +BEGIN + INSERT INTO app_auth.users (id, name, email, email_verified) + VALUES ('00000000-0000-4000-8000-0000000000a1', 'Ama Owusu', 'ama@doneby.test', true), + ('00000000-0000-4000-8000-0000000000b2', 'Ben', 'ben@doneby.test', true), + ('00000000-0000-4000-8000-0000000000c3', 'Cleo', 'cleo@doneby.test', true); + PERFORM set_config('request.jwt.claims', json_build_object('sub', '00000000-0000-4000-8000-0000000000a1', 'role', 'authenticated')::text, true); + v_home := public.create_commune('Done by home', 'household', 'GBP'); + INSERT INTO public.commune_members (commune_id, user_id, role) + VALUES (v_home, '00000000-0000-4000-8000-0000000000b2', 'member'), (v_home, '00000000-0000-4000-8000-0000000000c3', 'member'); + PERFORM set_config('test.home', v_home::text, true); +END; +$$; + +SET ROLE authenticated; +DO $$ +DECLARE + v_home uuid := current_setting('test.home')::uuid; + a text := '00000000-0000-4000-8000-0000000000a1'; + b text := '00000000-0000-4000-8000-0000000000b2'; + c text := '00000000-0000-4000-8000-0000000000c3'; + v_op uuid; v_once uuid; v_loose uuid; v_result jsonb; v_row public.commune_operations%ROWTYPE; + v_history jsonb; v_failed boolean; +BEGIN + PERFORM set_config('request.jwt.claims', json_build_object('sub', a, 'role', 'authenticated')::text, true); + v_op := (public.create_operation(v_home, 'task', 'chore', 'Bins', current_date, NULL, true, a, jsonb_build_array(a, b, c), + 'weekly', '', gen_random_uuid())->>'id')::uuid; + + -- Ben marks Ama's turn done without saying who did it: refused, nothing moves. + PERFORM set_config('request.jwt.claims', json_build_object('sub', b, 'role', 'authenticated')::text, true); + v_failed := false; + BEGIN + PERFORM public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid()); + EXCEPTION WHEN invalid_parameter_value THEN + v_failed := SQLERRM = 'This is Ama''s turn. Say whether you did it or they did.'; + END; + IF NOT v_failed THEN RAISE EXCEPTION 'Someone else''s task was marked done without saying who did it'; END IF; + IF EXISTS (SELECT 1 FROM public.operation_occurrences WHERE operation_id = v_op) THEN + RAISE EXCEPTION 'A refused completion left an occurrence'; + END IF; + + -- Any other answer, or an answer with a skip, is refused. + FOREACH v_result IN ARRAY ARRAY['"someone"'::jsonb, '"skip"'::jsonb] LOOP + v_failed := false; + BEGIN + IF v_result #>> '{}' = 'skip' THEN + PERFORM public.record_operation_occurrence(v_op, 'skipped', current_date, current_date, gen_random_uuid(), NULL, 'me'); + ELSE + PERFORM public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid(), NULL, v_result #>> '{}'); + END IF; + EXCEPTION WHEN invalid_parameter_value THEN v_failed := true; + END; + IF NOT v_failed THEN RAISE EXCEPTION 'A wrong done-by answer was accepted: %', v_result; END IF; + END LOOP; + + -- Ben says Ama did it: Ama is credited, Ben is named as marking it, and + -- the turn passes to Ben as usual. + v_result := public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'assignee'); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_op; + IF v_row.last_done <> jsonb_build_object('by', 'Ama Owusu', 'date', current_date, 'markedBy', 'Ben') OR v_row.assignee_id <> b THEN + RAISE EXCEPTION 'Marking done for the holder went wrong: %', row_to_json(v_row); + END IF; + IF NOT EXISTS (SELECT 1 FROM public.operation_occurrences WHERE id = (v_result->>'id')::uuid AND actor_id = b AND done_by_id = a) THEN + RAISE EXCEPTION 'The occurrence did not keep who did it'; + END IF; + IF NOT EXISTS (SELECT 1 FROM public.commune_events WHERE entity_id = v_op AND actor_id = b AND description = 'Marked done for Ama Owusu. Next: Ben') THEN + RAISE EXCEPTION 'Activity did not say it was marked done for Ama'; + END IF; + SELECT o->'history' INTO v_history FROM jsonb_array_elements(public.commune_snapshot(v_home)->'operations') o WHERE o->>'id' = v_op::text; + IF v_history->0->>'doneById' <> a OR v_history->0->>'actorId' <> b THEN + RAISE EXCEPTION 'The history did not carry who did it: %', v_history; + END IF; + + -- Undo puts the earlier credit back. + PERFORM public.undo_operation_occurrence((v_result->>'id')::uuid); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_op; + IF v_row.last_done IS NOT NULL OR v_row.assignee_id <> a THEN + RAISE EXCEPTION 'Undo did not restore the task: %', row_to_json(v_row); + END IF; + + -- Ben did Ama's turn himself: Ben is credited, covering Ama. The turn + -- still moves on from Ama's place. + v_result := public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'me'); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_op; + IF v_row.last_done <> jsonb_build_object('by', 'Ben', 'date', current_date, 'covering', 'Ama Owusu') OR v_row.assignee_id <> b THEN + RAISE EXCEPTION 'Covering the holder went wrong: %', row_to_json(v_row); + END IF; + IF NOT EXISTS (SELECT 1 FROM public.operation_occurrences WHERE id = (v_result->>'id')::uuid AND done_by_id = b) + OR NOT EXISTS (SELECT 1 FROM public.commune_events WHERE entity_id = v_op AND description = 'Completed, covering Ama Owusu. Next: Ben') THEN + RAISE EXCEPTION 'Covering was not recorded'; + END IF; + + -- Ben's own turn needs no answer and credits Ben alone. + v_result := public.record_operation_occurrence(v_op, 'completed', v_row.date, current_date, gen_random_uuid()); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_op; + IF v_row.last_done <> jsonb_build_object('by', 'Ben', 'date', current_date) OR v_row.assignee_id <> c THEN + RAISE EXCEPTION 'Completing your own turn went wrong: %', row_to_json(v_row); + END IF; + + -- A one-off task works the same way. + v_once := (public.create_operation(v_home, 'task', 'chore', 'Return the key', current_date, NULL, true, c, NULL, + 'none', '', gen_random_uuid())->>'id')::uuid; + PERFORM public.record_operation_occurrence(v_once, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'assignee'); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_once; + IF v_row.status <> 'done' OR v_row.last_done <> jsonb_build_object('by', 'Cleo', 'date', current_date, 'markedBy', 'Ben') THEN + RAISE EXCEPTION 'A one-off marked done for its holder went wrong: %', row_to_json(v_row); + END IF; + + -- A task nobody holds is done by whoever marks it, and cannot be credited + -- to a holder it does not have. + v_loose := (public.create_operation(v_home, 'task', 'chore', 'Descale the kettle', current_date, NULL, true, NULL, NULL, + 'none', '', gen_random_uuid())->>'id')::uuid; + v_failed := false; + BEGIN + PERFORM public.record_operation_occurrence(v_loose, 'completed', current_date, current_date, gen_random_uuid(), NULL, 'assignee'); + EXCEPTION WHEN invalid_parameter_value THEN v_failed := true; + END; + IF NOT v_failed THEN RAISE EXCEPTION 'A task nobody holds was credited to its holder'; END IF; + PERFORM public.record_operation_occurrence(v_loose, 'completed', current_date, current_date, gen_random_uuid()); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_loose; + IF v_row.last_done <> jsonb_build_object('by', 'Ben', 'date', current_date) THEN + RAISE EXCEPTION 'An unheld task was not credited to whoever marked it: %', row_to_json(v_row); + END IF; + + -- last_done takes only these names, each a short piece of text. + IF public.valid_operation_last_done(jsonb_build_object('by', 'Ben', 'date', current_date, 'markedBy', '')) + OR public.valid_operation_last_done(jsonb_build_object('by', 'Ben', 'date', current_date, 'covering', 3)) + OR public.valid_operation_last_done(jsonb_build_object('by', 'Ben', 'date', current_date, 'note', 'x')) THEN + RAISE EXCEPTION 'last_done accepted a malformed name'; + END IF; +END; +$$; + +-- A task still held by a name with no account keeps crediting that name and +-- says who marked it. +RESET ROLE; +DO $$ +DECLARE + v_home uuid := current_setting('test.home')::uuid; + v_op uuid; v_row public.commune_operations%ROWTYPE; +BEGIN + -- Rows like this predate member ids (0020), so the checks on new rows are + -- bypassed to make one. + SET LOCAL session_replication_role = replica; + INSERT INTO public.commune_operations (commune_id, kind, category, title, date, repeat, assignee, created_by) + VALUES (v_home, 'task', 'chore', 'Plants', current_date, 'weekly', 'Grandma', '00000000-0000-4000-8000-0000000000a1') + RETURNING id INTO v_op; + SET LOCAL session_replication_role = origin; + PERFORM set_config('request.jwt.claims', json_build_object('sub', '00000000-0000-4000-8000-0000000000b2', 'role', 'authenticated')::text, true); + SET LOCAL ROLE authenticated; + PERFORM public.record_operation_occurrence(v_op, 'completed', current_date, current_date, gen_random_uuid()); + SELECT * INTO v_row FROM public.commune_operations WHERE id = v_op; + IF v_row.last_done <> jsonb_build_object('by', 'Grandma', 'date', current_date, 'markedBy', 'Ben') THEN + RAISE EXCEPTION 'A named holder lost the credit: %', row_to_json(v_row); + END IF; + IF NOT EXISTS (SELECT 1 FROM public.operation_occurrences WHERE operation_id = v_op AND done_by_id IS NULL) THEN + RAISE EXCEPTION 'A completion claimed to know who a named holder was'; + END IF; +END; +$$; + +SELECT 'task_done_by ok'; +ROLLBACK; diff --git a/infra/neon/tests/task_lock_order.mjs b/infra/neon/tests/task_lock_order.mjs index 4b20ccd3..1801150b 100644 --- a/infra/neon/tests/task_lock_order.mjs +++ b/infra/neon/tests/task_lock_order.mjs @@ -169,7 +169,7 @@ try { // is refused because Blake has left, and the task stays with Casey. const undo = await race({ name: "An undo", sql: "SELECT public.undo_operation_occurrence($1)", prepare: async (task) => [(await as(casey, `SELECT public.record_operation_occurrence($1, 'completed', current_date, - current_date, $2) ->> 'id' AS id`, [task, randomUUID()])).id] }); + current_date, $2, NULL, 'me') ->> 'id' AS id`, [task, randomUUID()])).id] }); assert.equal(undo.change.error?.code, "P0001", undo.change.error?.message ?? "The undo gave the task back to Blake"); assert.equal(undo.change.error.message, "The person who had this task has left, so it can no longer be undone. Edit the task instead.");