diff --git a/docs/DATA-MODEL.md b/docs/DATA-MODEL.md index 3e165c3..2f2f6d0 100644 --- a/docs/DATA-MODEL.md +++ b/docs/DATA-MODEL.md @@ -634,7 +634,9 @@ timestamps keeps the merge order-independent and idempotent either way, and remo `mergeProgress` is pure and `test/progress.test.ts` pins it. Check-in goals ride `progress`, so they need no export key of their own and merge with the record they belong to. `tasks` and `amounts` are additive export keys like `customGames`: a file without them is from a device that had none. Tasks merge by id with the later -`updatedAt` winning (`mergeTasks`), incoming records validated and current ones kept as stored; +`updatedAt` winning (`mergeTasks`); an incoming record is let in on its identity alone (a `mytask:` +id and an `updatedAt`) and stored exactly as it came, so a newer version's task — or a field this +build does not know — survives a trip through an older build, and an edit keeps such fields too; amounts merge per item **and per period** with the later `at` winning (`mergeAmounts`), so restoring an old backup cannot roll this week's number back, and a week only one side has survives. An export written before daily checklists existed simply has no `daily` key, which is not an error. Losing a user's marks to a bad import is unrecoverable, so the merge is diff --git a/src/client/components/Dailies.tsx b/src/client/components/Dailies.tsx index 99aa668..816e8ef 100644 --- a/src/client/components/Dailies.tsx +++ b/src/client/components/Dailies.tsx @@ -61,9 +61,11 @@ export interface DailyItem { goal?: GoalStatus | undefined; /** * How often it is due. Chores and events are daily and make up the header's - * "today" count; tasks are counted by their own period beside it. + * "today" count; tasks are counted by their own period beside it. An event + * whose goal is a number has no "today" — nothing ticks — so it counts with + * its week, or, over its whole run, in no tally at all. */ - cadence: "day" | "week" | "month"; + cadence: "day" | "week" | "month" | "event"; /** A task's period, for finding the one before it in Catch up. */ period?: Period | undefined; /** A task's unit, printed after its target. */ @@ -149,6 +151,7 @@ export function dailyGroups( for (const event of events) { if (event.game !== lane) continue; + const goal = goalFor(event.id); items.push({ key: event.id, game: lane, @@ -158,8 +161,8 @@ export function dailyGroups( today, resetsIn, notBefore: startOf(event), - goal: goalFor(event.id), - cadence: "day", + goal, + cadence: goal?.measure !== "amount" ? "day" : goal.scope === "week" ? "week" : "event", }); } @@ -221,6 +224,7 @@ export function dailyTally( const month = { total: 0, complete: 0 }; for (const i of items) { const met = i.goal?.met === true; + if (i.cadence === "event") continue; if (i.cadence !== "day") { const tally = i.cadence === "week" ? week : month; tally.total += 1; diff --git a/src/client/components/TaskForm.tsx b/src/client/components/TaskForm.tsx index 3c3bb58..26270f6 100644 --- a/src/client/components/TaskForm.tsx +++ b/src/client/components/TaskForm.tsx @@ -13,23 +13,32 @@ export interface TaskFields { title: string; unit: "week" | "month"; weekday: number; - monthDay: number; + /** As typed, so the field can be emptied and retyped; read through `parseMonthDay`. */ + monthDay: string | number; kind: "days" | "amount"; required: string; target: string; amountUnit: string; } +/** A typed day of the month, clamped to 1–31; null while empty or unreadable. */ +export function parseMonthDay(raw: string): number | null { + const n = Number.parseInt(raw, 10); + return Number.isFinite(n) ? Math.min(31, Math.max(1, n)) : null; +} + /** * The task these fields describe, or null while they do not describe one the * store would accept. Checked against `TaskSchema` itself, so the form can * never offer Save on something the store would then refuse. */ export function taskDraftFrom(f: TaskFields): TaskDraft | null { + const monthDay = parseMonthDay(String(f.monthDay)); + if (f.unit === "month" && monthDay === null) return null; const period = f.unit === "week" ? ({ unit: "week", startsOn: f.weekday } as const) - : ({ unit: "month", startsOn: f.monthDay } as const); + : ({ unit: "month", startsOn: monthDay ?? 1 } as const); const unit = f.amountUnit.trim(); const target = parseAmount(f.target); const measure = @@ -78,7 +87,7 @@ export function TaskForm({ title: initial?.title ?? "", unit: initial?.period.unit ?? "week", weekday: initial?.period.unit === "week" ? initial.period.startsOn : 1, - monthDay: initial?.period.unit === "month" ? initial.period.startsOn : 1, + monthDay: String(initial?.period.unit === "month" ? initial.period.startsOn : 1), kind: initial?.measure.kind ?? "days", required: initial?.measure.kind === "days" ? String(initial.measure.required) : "1", target: initial?.measure.kind === "amount" ? String(initial.measure.target) : "", @@ -156,7 +165,7 @@ export function TaskForm({ min={1} max={31} value={f.monthDay} - onChange={(e) => set({ monthDay: Number.parseInt(e.target.value, 10) || 1 })} + onChange={(e) => set({ monthDay: e.target.value })} className={`tnum w-14 ${field}`} /> diff --git a/src/client/state/useTasks.ts b/src/client/state/useTasks.ts index 5339b82..166ba52 100644 --- a/src/client/state/useTasks.ts +++ b/src/client/state/useTasks.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useMemo, useState } from "react"; import type { Measure, Task } from "../../shared/task.ts"; -import { mintTaskId, TaskSchema } from "../../shared/task.ts"; +import { mintTaskId, TaskId, TaskSchema } from "../../shared/task.ts"; import type { Period } from "../../shared/period.ts"; import { KEYS, readJson, writeJson } from "./storage.ts"; @@ -39,24 +39,39 @@ const updatedAtOf = (v: unknown): string => { /** * Import: union by id, the later `updatedAt` winning, nothing removed. * - * Incoming records are validated — a file is untrusted input — and one that - * does not parse is skipped. Records already on the device are kept as they - * are, readable or not. + * A record is let in on its identity alone — a `mytask:` id and a timestamp — + * and stored **exactly as it came**, not as this build parses it. A backup is + * where a newer version's task (or a field this build does not know) has to + * survive a trip through an older one; parsing on the way in would strip the + * field or drop the task, and re-importing could never bring either back. + * Whether it can be *shown* is `visibleTasks`' question. */ export function mergeTasks(current: StoredTasks, incoming: unknown): StoredTasks { if (typeof incoming !== "object" || incoming === null) return current; const next: StoredTasks = { ...current }; for (const value of Object.values(incoming as Record)) { - const parsed = TaskSchema.safeParse(value); - if (!parsed.success) continue; - const id = parsed.data.id; - if (next[id] === undefined || parsed.data.updatedAt > updatedAtOf(next[id])) { - next[id] = parsed.data; + if (typeof value !== "object" || value === null) continue; + const id = (value as { id?: unknown }).id; + if (!TaskId.safeParse(id).success || updatedAtOf(value) === "") continue; + const key = id as string; + if (next[key] === undefined || updatedAtOf(value) > updatedAtOf(next[key])) { + next[key] = value; } } return next; } +/** + * A stored task with the reader's edit applied, or null if it is not one this + * build can edit. Built on the stored record, not on its parse, so a field a + * newer version added rides through the edit untouched. + */ +export function editedTask(stored: unknown, draft: TaskDraft, at: string): unknown { + if (!TaskSchema.safeParse(stored).success) return null; + const next = { ...(stored as Record), ...draft, updatedAt: at }; + return TaskSchema.safeParse(next).success ? next : null; +} + /** * The reader's weekly and monthly tasks (PRD F17). * @@ -86,14 +101,8 @@ export function useTasks() { const editTask = useCallback((id: string, draft: TaskDraft) => { setRaw((prev) => { - const existing = TaskSchema.safeParse(prev[id]); - if (!existing.success) return prev; - const next = TaskSchema.parse({ - ...existing.data, - ...draft, - updatedAt: new Date().toISOString(), - }); - return { ...prev, [id]: next }; + const next = editedTask(prev[id], draft, new Date().toISOString()); + return next === null ? prev : { ...prev, [id]: next }; }); }, []); diff --git a/test/custom-ui.test.tsx b/test/custom-ui.test.tsx index e00be35..2d61ec3 100644 --- a/test/custom-ui.test.tsx +++ b/test/custom-ui.test.tsx @@ -10,7 +10,7 @@ import { strandedNotice, } from "../src/client/components/CustomForms.tsx"; import { eventCaption, YourOwn } from "../src/client/components/YourOwn.tsx"; -import { TaskForm, taskDraftFrom } from "../src/client/components/TaskForm.tsx"; +import { parseMonthDay, TaskForm, taskDraftFrom } from "../src/client/components/TaskForm.tsx"; import { EventRow } from "../src/client/components/EventRow.tsx"; import { EventDetail } from "../src/client/components/EventDetail.tsx"; import { AUTHOR, Colophon, REPO_URL, RESET_CLOCK_CREDIT } from "../src/client/components/Colophon.tsx"; @@ -1311,3 +1311,12 @@ describe("the reader's weekly and monthly tasks in settings", () => { }); }); }); + +describe("review fixes: the month-day field", () => { + test("an emptied field is mid-edit, not day 1", () => { + expect(parseMonthDay("")).toBeNull(); + expect(parseMonthDay("5")).toBe(5); + expect(parseMonthDay("40")).toBe(31); + expect(parseMonthDay("0")).toBe(1); + }); +}); diff --git a/test/tasks-store.test.ts b/test/tasks-store.test.ts index 4eb9545..d35a3af 100644 --- a/test/tasks-store.test.ts +++ b/test/tasks-store.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { mergeAmounts, readableAmounts } from "../src/client/state/useAmounts.ts"; -import { mergeTasks, visibleTasks } from "../src/client/state/useTasks.ts"; +import { editedTask, mergeTasks, visibleTasks } from "../src/client/state/useTasks.ts"; /** * The two stores weekly and monthly tasks add. Both are the only copy of what @@ -91,3 +91,28 @@ describe("amounts", () => { expect(merged).toEqual({}); }); }); + +describe("review fixes: a task's data outlives this build", () => { + test("merge keeps fields this build does not know", () => { + const withExtra = { ...task(A, "Weekly", "2026-10-05T00:00:00.000Z"), notes: "keep me" }; + expect(mergeTasks({}, { [A]: withExtra })[A]).toEqual(withExtra); + }); + + test("a newer version's task in a backup reaches a fresh device", () => { + expect(mergeTasks({}, { [FUTURE.id]: FUTURE })[FUTURE.id]).toEqual(FUTURE); + }); + + test("but junk without a task id or a timestamp still stays out", () => { + expect(mergeTasks({}, { x: { id: "myevent:abcdef1234", updatedAt: "2026" } })).toEqual({}); + expect(mergeTasks({}, { x: { id: "mytask:abcdef1234" } })).toEqual({}); + }); + + test("editing keeps fields this build does not know", () => { + const stored = { ...task(A, "Weekly", "2026-10-05T00:00:00.000Z"), notes: "keep me" }; + const next = editedTask(stored, { + game: "zzz", title: "Renamed", period: { unit: "week", startsOn: 1 }, + measure: { kind: "days", required: 1 }, + }, "2026-10-06T00:00:00.000Z"); + expect(next).toMatchObject({ title: "Renamed", notes: "keep me", updatedAt: "2026-10-06T00:00:00.000Z" }); + }); +}); diff --git a/test/views.test.tsx b/test/views.test.tsx index 8c803ce..221c4ef 100644 --- a/test/views.test.tsx +++ b/test/views.test.tsx @@ -1457,3 +1457,27 @@ describe("GoalControl with points", () => { expect(html).toContain("points each week, weeks start"); }); }); + +describe("review fixes: an event's points goal is not a daily", () => { + test("it is counted with the week, not as waiting today", () => { + const NOW = Date.parse("2026-10-15T12:00:00.000Z"); + const ev = { + id: "p1", game: "zzz", title: "Points event", type: "other", summary: null, + startsAt: "2026-10-01T00:00:00.000Z", startPrecision: "day", endsAt: null, + endPrecision: "unknown", regionScoped: false, regionEnds: null, + sourceUrl: "https://example.test", + } as never; + const week: GoalStatus = { + scope: "week", measure: "amount", periodStart: "2026-10-12", required: 2100, count: 100, + met: false, needed: 2000, open: 4, shortfall: false, + }; + const total: GoalStatus = { ...week, scope: "event", periodStart: "2026-10-01" }; + const tally = (goal: GoalStatus) => { + const [group] = dailyGroups(["zzz"], [ev], NOW, "europe", (id) => metaFor(id, {}), + (e) => Date.parse(e.startsAt), false, () => goal); + return dailyTally(group!.items, () => []); + }; + expect(tally(week)).toMatchObject({ total: 0, week: { total: 1, complete: 0 } }); + expect(tally(total)).toMatchObject({ total: 0, week: { total: 0 }, month: { total: 0 } }); + }); +});