fix(tasks): keep a task whole through import and edit, and fix two counts

Review found that import and edit rebuilt a task from this build's parse.
That stripped any field it didn't know, and import dropped a newer
version's task entirely. Both now keep the record as stored and validate
only to display it.

An event whose goal is points can't be ticked, so it no longer counts as
waiting today: it counts with its week. The month-day field reset
itself to 1 when emptied; it now keeps a draft.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Lucas WintherandClaude Opus 5.5 committed 2026-10-13 13:54:29 +02:00
1 parent 54d8424d1e
commit 5a4c746a44
7 files changed
+110 -28

No files matched your search

+3 -1
View File
@@ -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
+8 -4
View File
@@ -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;
+13 -4
View File
@@ -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}`}
/>
</label>
+26 -17
View File
@@ -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<string, unknown>)) {
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<string, unknown>), ...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 };
});
}, []);
+10 -1
View File
@@ -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);
});
});
+26 -1
View File
@@ -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" });
});
});
+24
View File
@@ -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 } });
});
});