Correct the PRD's claim of an until control, and stop the form erasing one
The PRD said a reader's rule could optionally stop on a date; the until control was deliberately descoped from the form during planning, and the PRD was written as though it shipped. Corrected the PRD to describe what actually ships, and noted that the field still exists in the schema — reachable today only by importing a file that carries one. That gap had a second-order bug behind it: because the edit form rebuilt `repeat` from scratch on every save, hard-coding `until: null`, a rule that already carried a non-null `until` had it silently reset to eternal on any edit at all, including a pure title fix. Extracted the `repeat`-building logic into `repeatFrom`, which now carries `initial?.repeat?.until` forward, and is exported so the behaviour is provable without a submit nothing in this suite's renderToStaticMarkup tests can click. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d433eeee50
commit
e3c4fae1b9
+11
-3
@@ -324,9 +324,17 @@ Four constraints, each protecting something that already exists:
|
||||
no server to restore from.
|
||||
|
||||
A reader's event may also state how it comes round again — every N days, weeks
|
||||
or months, optionally stopping on a date. The rule is stored; its occurrences
|
||||
are derived, and each one is an ordinary event everywhere in the app: its own
|
||||
countdown, its own completion, its own daily checklist.
|
||||
or months. The rule is stored; its occurrences are derived, and each one is an
|
||||
ordinary event everywhere in the app: its own countdown, its own completion,
|
||||
its own daily checklist.
|
||||
|
||||
**The schedule can stop on a date (`until`), but the form has no control for
|
||||
setting one.** The field exists in the schema — descoped from the form during
|
||||
planning rather than removed from the data — so a rule already carrying one,
|
||||
reachable today only by importing a file that has it, keeps it: editing such a
|
||||
rule preserves its `until` rather than resetting it to "never" on save. There
|
||||
is simply no way for a reader to give a rule an end date through the form
|
||||
itself.
|
||||
|
||||
**An occurrence need not state its end.** With none, it runs until the next one
|
||||
opens. That is not the app inventing a date to fill a form — it is entailed by
|
||||
|
||||
@@ -72,6 +72,29 @@ export function cadenceLabel(repeat: Repeat | null): string | null {
|
||||
return `every ${repeat.interval} ${repeat.unit}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The `repeat` a save would write, given what the form's own controls state
|
||||
* plus whatever `until` the record being edited already carries.
|
||||
*
|
||||
* The form has no control for `until` — descoped from the control surface
|
||||
* during planning, not from the schema — so it is never this function's to
|
||||
* set. But a rule reaching the form already carrying one, reachable today
|
||||
* only by importing a file that has it, has to keep it: rebuilding `repeat`
|
||||
* from the unit and interval fields alone would silently turn a terminating
|
||||
* rule eternal on a save as unrelated as a title fix. Exported, and separated
|
||||
* from the component's own state wiring, so that survival is provable without
|
||||
* a submit nothing in this test suite can click.
|
||||
*/
|
||||
export function repeatFrom(
|
||||
unit: RepeatUnit | "never",
|
||||
interval: number,
|
||||
existingUntil: string | null,
|
||||
): Repeat | null {
|
||||
const intervalValid = Number.isInteger(interval) && interval >= 1 && interval <= 365;
|
||||
if (unit === "never" || !intervalValid) return null;
|
||||
return { unit, interval, until: existingUntil };
|
||||
}
|
||||
|
||||
function labelClass(): string {
|
||||
return "block text-xs font-medium text-muted";
|
||||
}
|
||||
@@ -232,10 +255,7 @@ export function EventForm({
|
||||
const interval = Number(repeatInterval);
|
||||
const intervalValid =
|
||||
Number.isInteger(interval) && interval >= 1 && interval <= 365;
|
||||
const repeat =
|
||||
repeatUnit === "never" || !intervalValid
|
||||
? null
|
||||
: { unit: repeatUnit, interval, until: null };
|
||||
const repeat = repeatFrom(repeatUnit, interval, initial?.repeat?.until ?? null);
|
||||
|
||||
// The same predicate the schema refines on, so the form cannot start
|
||||
// refusing saves the schema would accept or promising ones it will reject.
|
||||
|
||||
@@ -3,6 +3,7 @@ import { renderToStaticMarkup } from "react-dom/server";
|
||||
import {
|
||||
cadenceLabel,
|
||||
EventForm,
|
||||
repeatFrom,
|
||||
strandedNotice,
|
||||
} from "../src/client/components/CustomForms.tsx";
|
||||
import { YourOwn } from "../src/client/components/YourOwn.tsx";
|
||||
@@ -422,6 +423,41 @@ describe("stating a repeat", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("repeatFrom", () => {
|
||||
test("a fresh rule has no `until` to carry — there is no control for one", () => {
|
||||
expect(repeatFrom("weeks", 2, null)).toEqual({ unit: "weeks", interval: 2, until: null });
|
||||
});
|
||||
|
||||
test("an edit preserves the `until` already on the record", () => {
|
||||
// The form has no field for this, so the only way it can end up on the
|
||||
// save is by surviving from what was already there — an import-sourced
|
||||
// rule, since that is the only door `until` has today.
|
||||
const existing = "2027-01-01T00:00:00.000Z";
|
||||
expect(repeatFrom("weeks", 2, existing)).toEqual({
|
||||
unit: "weeks",
|
||||
interval: 2,
|
||||
until: existing,
|
||||
});
|
||||
// It survives a schedule change too — changing the unit or interval is a
|
||||
// different edit from changing when the series stops, and the reader
|
||||
// never touched the latter.
|
||||
expect(repeatFrom("months", 1, existing)).toEqual({
|
||||
unit: "months",
|
||||
interval: 1,
|
||||
until: existing,
|
||||
});
|
||||
});
|
||||
|
||||
test("turning the repeat off drops it, existing `until` included", () => {
|
||||
expect(repeatFrom("never", 2, "2027-01-01T00:00:00.000Z")).toBeNull();
|
||||
});
|
||||
|
||||
test("an invalid interval is refused the same way regardless of `until`", () => {
|
||||
expect(repeatFrom("weeks", 0, "2027-01-01T00:00:00.000Z")).toBeNull();
|
||||
expect(repeatFrom("weeks", 400, null)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("what a reschedule costs", () => {
|
||||
test("says nothing when nothing would be stranded", () => {
|
||||
expect(strandedNotice(0)).toBe(null);
|
||||
|
||||
Reference in New Issue
Block a user