Reject an out-of-range planning horizon before saving
A digit string too long for a safe integer became Infinity, which JSON.stringify sends as null, and the API reads null as "reset to default" -- so saving could silently replace the household's horizon. Require a safe integer >= 1 and send nothing otherwise (F19-01). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -432,14 +432,18 @@ describe("SettingsScreen — preferences", () => {
|
||||
|
||||
it("a 422 shows the API detail naming the field and keeps the edit", async () => {
|
||||
const { form, onClose } = await renderPreferences();
|
||||
fireEvent.change(field(form, "Planning horizon (days)"), { target: { value: "0" } });
|
||||
writeResponse = () => jsonResponse({ detail: "horizon_days must be >= 1" }, 422);
|
||||
fireEvent.change(field(form, "Notify target"), { target: { value: "notify.nobody" } });
|
||||
writeResponse = () => jsonResponse({ detail: "notify_target is not a notify service" }, 422);
|
||||
save(form);
|
||||
const message = await screen.findByTestId("preferences-message");
|
||||
expect(message.textContent).toBe("The preferences could not be saved: horizon_days must be >= 1");
|
||||
expect(message.textContent).toBe(
|
||||
"The preferences could not be saved: notify_target is not a notify service",
|
||||
);
|
||||
expect(message.getAttribute("role")).toBe("alert");
|
||||
expect(field(form, "Planning horizon (days)").value).toBe("0");
|
||||
expect(writes()).toEqual([{ method: "PATCH", path: SETTINGS_PATH, body: { horizon_days: 0 } }]);
|
||||
expect(field(form, "Notify target").value).toBe("notify.nobody");
|
||||
expect(writes()).toEqual([
|
||||
{ method: "PATCH", path: SETTINGS_PATH, body: { notify_target: "notify.nobody" } },
|
||||
]);
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -453,12 +457,17 @@ describe("SettingsScreen — preferences", () => {
|
||||
expect(checkbox(form, "Celebration").checked).toBe(true);
|
||||
});
|
||||
|
||||
it("a malformed horizon is rejected before any request", async () => {
|
||||
it.each([
|
||||
["empty", ""],
|
||||
["zero", "0"],
|
||||
["oversized (Infinity would serialize as null and reset the field)", "9".repeat(400)],
|
||||
["past the safe-integer range", "123456789012345678901"],
|
||||
])("a %s horizon is rejected before any request", async (_case, value) => {
|
||||
const { form } = await renderPreferences();
|
||||
fireEvent.change(field(form, "Planning horizon (days)"), { target: { value: "" } });
|
||||
fireEvent.change(field(form, "Planning horizon (days)"), { target: { value } });
|
||||
save(form);
|
||||
expect(screen.getByTestId("preferences-message").textContent).toBe(
|
||||
"Planning horizon must be a whole number of days.",
|
||||
"Planning horizon must be a whole number of days, at least 1.",
|
||||
);
|
||||
expect(writes()).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -262,7 +262,12 @@ function preferencesDraft(settings: HouseholdSettings): PreferencesDraft {
|
||||
/** Only the fields that differ from `loaded`, or a validation message. */
|
||||
function buildChanges(draft: PreferencesDraft, loaded: HouseholdSettings): SettingsChanges | string {
|
||||
const horizon = draft.horizon_days.trim();
|
||||
if (!/^\d+$/.test(horizon)) return "Planning horizon must be a whole number of days.";
|
||||
const horizonDays = Number(horizon);
|
||||
// A digit string too long for a safe integer would become Infinity, which
|
||||
// JSON serializes as null, and the API reads null as "reset to default".
|
||||
if (!/^\d+$/.test(horizon) || !Number.isSafeInteger(horizonDays) || horizonDays < 1) {
|
||||
return "Planning horizon must be a whole number of days, at least 1.";
|
||||
}
|
||||
const times: [TimeField, string][] = [
|
||||
["day_rollover_time", "Day rollover"],
|
||||
...NOTIFICATIONS.map((n): [TimeField, string] => [n.time, `${n.label} time`]),
|
||||
@@ -273,7 +278,7 @@ function buildChanges(draft: PreferencesDraft, loaded: HouseholdSettings): Setti
|
||||
|
||||
const next: HouseholdSettings = {
|
||||
...draft,
|
||||
horizon_days: Number(horizon),
|
||||
horizon_days: horizonDays,
|
||||
notify_target: draft.notify_target.trim(),
|
||||
};
|
||||
const changes: SettingsChanges = {};
|
||||
|
||||
Reference in New Issue
Block a user