Compare commits
8
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
77cc8428d0 | ||
|
|
33fa3b7c17 | ||
|
|
a329d10c0a | ||
|
|
773bee6191 | ||
|
|
1bf2122a4a | ||
|
|
02f4d8c8aa | ||
|
|
305b03e0b1 | ||
|
|
0d07fb4f46 |
@@ -1,5 +1,83 @@
|
|||||||
# NestQuest Release Notes
|
# NestQuest Release Notes
|
||||||
|
|
||||||
|
## Version 0.9.2 — 2026-09-26
|
||||||
|
|
||||||
|
### Scope
|
||||||
|
|
||||||
|
Patch release carrying the admin "Delete task" capability onto `main`.
|
||||||
|
|
||||||
|
- **Delete a task in the admin PWA:** the definition edit sheet gains an
|
||||||
|
admin-only Delete action with hybrid semantics (D-017, owner decision "delete
|
||||||
|
if there's no history, retire/deactivate if there is"). If the definition has
|
||||||
|
**no completion history**, it is hard-deleted together with its instances,
|
||||||
|
assignees, windows and its schedule rule (when unreferenced), all in one
|
||||||
|
transaction under the connection lock. If it has **any completion history** (a
|
||||||
|
completion or an un-completion), nothing is deleted: it is retired by
|
||||||
|
deactivation instead, preserving the definition, rule, assignees, windows and
|
||||||
|
all history, with future uncompleted instances cleared as with any
|
||||||
|
deactivation. (PR #262.)
|
||||||
|
- **API:** `DELETE /api/v1/admin/quest-definitions/{id}` lives on the admin plane
|
||||||
|
(`require_admin`; a non-admin is refused and an unknown id is 404). The response
|
||||||
|
reports `deleted`/`retired` and carries the retired definition (`null` when
|
||||||
|
deleted). The kids' panel gains no control, and `QuestDefinitionsDao` now has
|
||||||
|
exactly one, history-guarded, delete method.
|
||||||
|
|
||||||
|
### Upgrade note
|
||||||
|
|
||||||
|
The database schema is unchanged at version **9**; no migration or backup step is
|
||||||
|
needed beyond the 0.8.0 note. No runtime dependency is added.
|
||||||
|
|
||||||
|
### Requirements
|
||||||
|
|
||||||
|
Home Assistant 2024.6.0 or newer. Install through HACS from the
|
||||||
|
`Maniacs-Mansion/NestQuest` GitHub mirror (kept current by the Gitea push
|
||||||
|
mirror).
|
||||||
|
|
||||||
|
## Version 0.9.1 — 2026-09-26
|
||||||
|
|
||||||
|
### Scope
|
||||||
|
|
||||||
|
Patch release carrying the owner-reported UI-bug fixes for the kids' panel, plus a
|
||||||
|
household-timezone correction, onto `main`.
|
||||||
|
|
||||||
|
- **Household-local time:** a new `timezone` setting (an IANA name such as
|
||||||
|
`America/New_York`; empty keeps the API host's local time) is threaded into the
|
||||||
|
API's clock reads, so overdue and the daily rollover use the household's day
|
||||||
|
rather than a UTC host's. **Set it once after upgrading** (see the Upgrade
|
||||||
|
note); the admin UI has no field for it yet. (PR #254.)
|
||||||
|
- **Overdue and rollover correctness:** quests are no longer marked overdue
|
||||||
|
before their due time, and the missed-sweep scheduler fires at the configured
|
||||||
|
rollover across DST transitions — its delay is now a true elapsed-time
|
||||||
|
difference, with documented handling of nonexistent/ambiguous local times.
|
||||||
|
(PRs #254, #257.)
|
||||||
|
- **Complete works on the panel:** a TypeScript ES2022 `useDefineForClassFields`
|
||||||
|
default emitted native class fields that shadowed Lit's reactive accessors, so
|
||||||
|
the confirm dialog never re-rendered and a tap appeared to do nothing; the
|
||||||
|
panel now opens the confirm and completes. (PR #254.)
|
||||||
|
- **Panel polish:** the party-board hint line no longer hides behind the dock
|
||||||
|
(#244); the Quest Log header crest renders with its shield classes (#252); the
|
||||||
|
party-board wordmark is the new compass-rose monogram plus Cinzel Decorative
|
||||||
|
text, replacing the d20 dice (#253); and the quest meta line keeps its `meta`
|
||||||
|
class when not late (#255).
|
||||||
|
|
||||||
|
### Upgrade note
|
||||||
|
|
||||||
|
The database schema is unchanged at version **9**. After upgrading, set the
|
||||||
|
household timezone once so date, overdue and rollover behaviour match the house
|
||||||
|
(otherwise it falls back to the API host's local time, which may be UTC):
|
||||||
|
|
||||||
|
```
|
||||||
|
PATCH /api/v1/admin/settings {"timezone": "America/New_York"}
|
||||||
|
```
|
||||||
|
|
||||||
|
No runtime dependency is added.
|
||||||
|
|
||||||
|
### Requirements
|
||||||
|
|
||||||
|
Home Assistant 2024.6.0 or newer. Install through HACS from the
|
||||||
|
`Maniacs-Mansion/NestQuest` GitHub mirror (kept current by the Gitea push
|
||||||
|
mirror).
|
||||||
|
|
||||||
## Version 0.9.0 — 2026-09-26
|
## Version 0.9.0 — 2026-09-26
|
||||||
|
|
||||||
### Scope
|
### Scope
|
||||||
|
|||||||
@@ -147,6 +147,29 @@ export async function updateDefinition(
|
|||||||
return readJson<QuestDefinition>(await sendJson(`${DEFINITIONS_PATH}/${id}`, "PATCH", body));
|
return readJson<QuestDefinition>(await sendJson(`${DEFINITIONS_PATH}/${id}`, "PATCH", body));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** `DELETE /quest-definitions/{id}`: exactly one of `deleted` / `retired` is true. */
|
||||||
|
export interface DefinitionDeleteResult {
|
||||||
|
definition_id: number;
|
||||||
|
deleted: boolean;
|
||||||
|
retired: boolean;
|
||||||
|
/** The retired (now inactive) definition; null when it was deleted. */
|
||||||
|
definition: QuestDefinition | null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Delete a task if it has no completion history; otherwise the API retires
|
||||||
|
* (deactivates) it. Retirement preserves the definition row, schedule rule,
|
||||||
|
* assignees, windows and ALL durable completion history. Like any
|
||||||
|
* deactivation it regenerates, so future UNCOMPLETED instances are cleared
|
||||||
|
* (rolling, regenerable rows) and not recreated while inactive; durable
|
||||||
|
* history lives only in the append-only `completion_events`.
|
||||||
|
*/
|
||||||
|
export async function deleteDefinition(id: number): Promise<DefinitionDeleteResult> {
|
||||||
|
return readJson<DefinitionDeleteResult>(
|
||||||
|
await apiFetch(`${DEFINITIONS_PATH}/${id}`, { method: "DELETE" }),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export const OCCURRENCES_PREVIEW_PATH = `${DEFINITIONS_PATH}/occurrences-preview`;
|
export const OCCURRENCES_PREVIEW_PATH = `${DEFINITIONS_PATH}/occurrences-preview`;
|
||||||
|
|
||||||
export interface OccurrencesPreviewOptions {
|
export interface OccurrencesPreviewOptions {
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ export interface HouseholdSettings {
|
|||||||
afternoon_reminder_enabled: boolean;
|
afternoon_reminder_enabled: boolean;
|
||||||
end_of_day_report_enabled: boolean;
|
end_of_day_report_enabled: boolean;
|
||||||
celebration_enabled: boolean;
|
celebration_enabled: boolean;
|
||||||
|
/** IANA zone name (e.g. "America/New_York"); "" means the API host's local time. */
|
||||||
|
timezone: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Only the supplied fields are updated; the API answers with all of them. */
|
/** Only the supplied fields are updated; the API answers with all of them. */
|
||||||
|
|||||||
@@ -569,6 +569,26 @@
|
|||||||
color: #fff;
|
color: #fff;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.defs-save--danger {
|
||||||
|
background: var(--nq-a-danger);
|
||||||
|
}
|
||||||
|
|
||||||
|
.defs-delete {
|
||||||
|
display: block;
|
||||||
|
width: 100%;
|
||||||
|
margin-top: 10px;
|
||||||
|
padding: 11px 0;
|
||||||
|
border: 1px solid var(--nq-a-danger-bg);
|
||||||
|
border-radius: var(--nq-a-density-radius);
|
||||||
|
background: var(--nq-a-surface);
|
||||||
|
color: var(--nq-a-danger-strong);
|
||||||
|
font-family: inherit;
|
||||||
|
font-size: 13px;
|
||||||
|
font-weight: 700;
|
||||||
|
cursor: pointer;
|
||||||
|
}
|
||||||
|
|
||||||
|
.defs-delete:disabled,
|
||||||
.defs-cancel:disabled,
|
.defs-cancel:disabled,
|
||||||
.defs-save:disabled {
|
.defs-save:disabled {
|
||||||
opacity: 0.6;
|
opacity: 0.6;
|
||||||
|
|||||||
@@ -121,6 +121,8 @@ function fakeApi(initial: QuestDefinition[]) {
|
|||||||
const calls: Call[] = [];
|
const calls: Call[] = [];
|
||||||
let nextWrite: Response | null = null;
|
let nextWrite: Response | null = null;
|
||||||
let preview: PreviewHandler = async () => jsonResponse({ dates: [] });
|
let preview: PreviewHandler = async () => jsonResponse({ dates: [] });
|
||||||
|
// Definitions with completion history: the API retires these on DELETE.
|
||||||
|
const withHistory = new Set<number>();
|
||||||
const fetchMock = vi.fn(async (input: RequestInfo | URL, init: RequestInit = {}) => {
|
const fetchMock = vi.fn(async (input: RequestInfo | URL, init: RequestInit = {}) => {
|
||||||
const path = new URL(String(input), "http://localhost").pathname;
|
const path = new URL(String(input), "http://localhost").pathname;
|
||||||
const method = init.method ?? "GET";
|
const method = init.method ?? "GET";
|
||||||
@@ -187,6 +189,16 @@ function fakeApi(initial: QuestDefinition[]) {
|
|||||||
);
|
);
|
||||||
return jsonResponse(definitions.find((d) => d.id === id));
|
return jsonResponse(definitions.find((d) => d.id === id));
|
||||||
}
|
}
|
||||||
|
if (method === "DELETE" && match) {
|
||||||
|
const id = Number(match[1]);
|
||||||
|
if (withHistory.has(id)) {
|
||||||
|
definitions = definitions.map((d) => (d.id === id ? { ...d, is_active: false } : d));
|
||||||
|
const retired = definitions.find((d) => d.id === id);
|
||||||
|
return jsonResponse({ definition_id: id, deleted: false, retired: true, definition: retired });
|
||||||
|
}
|
||||||
|
definitions = definitions.filter((d) => d.id !== id);
|
||||||
|
return jsonResponse({ definition_id: id, deleted: true, retired: false, definition: null });
|
||||||
|
}
|
||||||
return jsonResponse({ detail: "unexpected" }, 500);
|
return jsonResponse({ detail: "unexpected" }, 500);
|
||||||
});
|
});
|
||||||
return {
|
return {
|
||||||
@@ -203,6 +215,9 @@ function fakeApi(initial: QuestDefinition[]) {
|
|||||||
failNextWrite: (response: Response) => {
|
failNextWrite: (response: Response) => {
|
||||||
nextWrite = response;
|
nextWrite = response;
|
||||||
},
|
},
|
||||||
|
markHistory: (id: number) => {
|
||||||
|
withHistory.add(id);
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -578,14 +593,15 @@ describe("DefinitionsTab", () => {
|
|||||||
fireEvent.click(screen.getByTestId("definition-10"));
|
fireEvent.click(screen.getByTestId("definition-10"));
|
||||||
const s = sheet();
|
const s = sheet();
|
||||||
const title = within(s).getByRole("textbox", { name: "Title" });
|
const title = within(s).getByRole("textbox", { name: "Title" });
|
||||||
const save = within(s).getByRole("button", { name: "Save changes" });
|
// The edit sheet's last focusable control is its Delete button.
|
||||||
|
const last = within(s).getByRole("button", { name: "Delete task" });
|
||||||
|
|
||||||
save.focus();
|
last.focus();
|
||||||
fireEvent.keyDown(s, { key: "Tab" });
|
fireEvent.keyDown(s, { key: "Tab" });
|
||||||
expect(document.activeElement).toBe(title);
|
expect(document.activeElement).toBe(title);
|
||||||
|
|
||||||
fireEvent.keyDown(s, { key: "Tab", shiftKey: true });
|
fireEvent.keyDown(s, { key: "Tab", shiftKey: true });
|
||||||
expect(document.activeElement).toBe(save);
|
expect(document.activeElement).toBe(last);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("Escape does not close the sheet while saving", async () => {
|
it("Escape does not close the sheet while saving", async () => {
|
||||||
@@ -694,6 +710,73 @@ describe("DefinitionsTab", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("the Delete control is only offered for an existing task", async () => {
|
||||||
|
await renderReady();
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: "New" }));
|
||||||
|
expect(within(sheet()).queryByRole("button", { name: "Delete task" })).toBeNull();
|
||||||
|
fireEvent.click(within(sheet()).getByRole("button", { name: "Cancel" }));
|
||||||
|
fireEvent.click(screen.getByTestId("definition-10"));
|
||||||
|
expect(within(sheet()).getByRole("button", { name: "Delete task" })).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("Delete asks first, explaining delete-vs-retire; Go back writes nothing", async () => {
|
||||||
|
await renderReady();
|
||||||
|
fireEvent.click(screen.getByTestId("definition-10"));
|
||||||
|
fireEvent.click(within(sheet()).getByRole("button", { name: "Delete task" }));
|
||||||
|
const confirm = screen.getByTestId("delete-confirm");
|
||||||
|
expect(confirm.textContent).toContain("If it has no completion history it is deleted");
|
||||||
|
expect(confirm.textContent).toContain("otherwise it is retired");
|
||||||
|
expect(api.writes()).toHaveLength(0);
|
||||||
|
fireEvent.click(within(confirm).getByRole("button", { name: "Go back" }));
|
||||||
|
expect(screen.queryByTestId("delete-confirm")).toBeNull();
|
||||||
|
expect(api.writes()).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("confirming Delete on a task without history removes its row", async () => {
|
||||||
|
await renderReady();
|
||||||
|
fireEvent.click(screen.getByTestId("definition-10"));
|
||||||
|
fireEvent.click(within(sheet()).getByRole("button", { name: "Delete task" }));
|
||||||
|
fireEvent.click(
|
||||||
|
within(screen.getByTestId("delete-confirm")).getByRole("button", { name: "Delete task" }),
|
||||||
|
);
|
||||||
|
await waitFor(() => expect(screen.queryByTestId("definition-10")).toBeNull());
|
||||||
|
expect(screen.queryByRole("dialog")).toBeNull();
|
||||||
|
expect(api.writes()).toEqual([
|
||||||
|
{ method: "DELETE", path: "/api/v1/admin/quest-definitions/10", body: null },
|
||||||
|
]);
|
||||||
|
expect(screen.getByTestId("definition-11")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("confirming Delete on a task with history shows it retired (inactive)", async () => {
|
||||||
|
await renderReady();
|
||||||
|
api.markHistory(10);
|
||||||
|
fireEvent.click(screen.getByTestId("definition-10"));
|
||||||
|
fireEvent.click(within(sheet()).getByRole("button", { name: "Delete task" }));
|
||||||
|
fireEvent.click(
|
||||||
|
within(screen.getByTestId("delete-confirm")).getByRole("button", { name: "Delete task" }),
|
||||||
|
);
|
||||||
|
await waitFor(() => expect(screen.queryByRole("dialog")).toBeNull());
|
||||||
|
const row = screen.getByTestId("definition-10");
|
||||||
|
expect(row.className).toContain("defs-row--inactive");
|
||||||
|
expect(meta(row)).toContain("· inactive");
|
||||||
|
expect(api.writes().map((c) => c.method)).toEqual(["DELETE"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("a failed Delete shows the API's detail and keeps the sheet open", async () => {
|
||||||
|
await renderReady();
|
||||||
|
api.failNextWrite(jsonResponse({ detail: "Quest definition not found" }, 404));
|
||||||
|
fireEvent.click(screen.getByTestId("definition-10"));
|
||||||
|
fireEvent.click(within(sheet()).getByRole("button", { name: "Delete task" }));
|
||||||
|
fireEvent.click(
|
||||||
|
within(screen.getByTestId("delete-confirm")).getByRole("button", { name: "Delete task" }),
|
||||||
|
);
|
||||||
|
expect((await screen.findByTestId("sheet-error")).textContent).toBe(
|
||||||
|
"Quest definition not found",
|
||||||
|
);
|
||||||
|
expect(screen.getByRole("dialog")).toBeTruthy();
|
||||||
|
expect(screen.getByTestId("definition-10")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
it("scroll column carries the 92px bottom padding", async () => {
|
it("scroll column carries the 92px bottom padding", async () => {
|
||||||
await renderReady();
|
await renderReady();
|
||||||
expect(screen.getByTestId("definitions-scroll").style.paddingBottom).toBe("92px");
|
expect(screen.getByTestId("definitions-scroll").style.paddingBottom).toBe("92px");
|
||||||
|
|||||||
@@ -13,12 +13,14 @@ import { ApiForbiddenError } from "../api/client";
|
|||||||
import {
|
import {
|
||||||
ApiRequestError,
|
ApiRequestError,
|
||||||
createDefinition,
|
createDefinition,
|
||||||
|
deleteDefinition,
|
||||||
fetchChildren,
|
fetchChildren,
|
||||||
fetchDefinitions,
|
fetchDefinitions,
|
||||||
previewOccurrences,
|
previewOccurrences,
|
||||||
updateDefinition,
|
updateDefinition,
|
||||||
type AdminChild,
|
type AdminChild,
|
||||||
type DefinitionCreateBody,
|
type DefinitionCreateBody,
|
||||||
|
type DefinitionDeleteResult,
|
||||||
type DefinitionEditBody,
|
type DefinitionEditBody,
|
||||||
type DefinitionRule,
|
type DefinitionRule,
|
||||||
type QuestDefinition,
|
type QuestDefinition,
|
||||||
@@ -273,6 +275,12 @@ function saveErrorMessage(error: unknown): string {
|
|||||||
return "The task could not be saved. Check your connection and try again.";
|
return "The task could not be saved. Check your connection and try again.";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function deleteErrorMessage(error: unknown): string {
|
||||||
|
if (error instanceof ApiForbiddenError) return error.message;
|
||||||
|
if (error instanceof ApiRequestError && error.detail) return error.detail;
|
||||||
|
return "The task could not be deleted. Check your connection and try again.";
|
||||||
|
}
|
||||||
|
|
||||||
function toggle<T>(list: T[], value: T): T[] {
|
function toggle<T>(list: T[], value: T): T[] {
|
||||||
return list.includes(value) ? list.filter((item) => item !== value) : [...list, value];
|
return list.includes(value) ? list.filter((item) => item !== value) : [...list, value];
|
||||||
}
|
}
|
||||||
@@ -449,11 +457,13 @@ function EditSheet({
|
|||||||
activeChildren,
|
activeChildren,
|
||||||
onCancel,
|
onCancel,
|
||||||
onSaved,
|
onSaved,
|
||||||
|
onDeleted,
|
||||||
}: {
|
}: {
|
||||||
target: "new" | QuestDefinition;
|
target: "new" | QuestDefinition;
|
||||||
activeChildren: AdminChild[];
|
activeChildren: AdminChild[];
|
||||||
onCancel: () => void;
|
onCancel: () => void;
|
||||||
onSaved: () => void;
|
onSaved: () => void;
|
||||||
|
onDeleted: (result: DefinitionDeleteResult) => void;
|
||||||
}) {
|
}) {
|
||||||
const [draft, setDraft] = useState<Draft>(() =>
|
const [draft, setDraft] = useState<Draft>(() =>
|
||||||
target === "new" ? newDraft(activeChildren) : draftFrom(target, activeChildren),
|
target === "new" ? newDraft(activeChildren) : draftFrom(target, activeChildren),
|
||||||
@@ -473,6 +483,7 @@ function EditSheet({
|
|||||||
const [pickerOpen, setPickerOpen] = useState(false);
|
const [pickerOpen, setPickerOpen] = useState(false);
|
||||||
const [saving, setSaving] = useState(false);
|
const [saving, setSaving] = useState(false);
|
||||||
const [confirming, setConfirming] = useState(false);
|
const [confirming, setConfirming] = useState(false);
|
||||||
|
const [confirmingDelete, setConfirmingDelete] = useState(false);
|
||||||
const [error, setError] = useState<string | null>(null);
|
const [error, setError] = useState<string | null>(null);
|
||||||
// A ref, not state: two taps in one frame must not both see `saving === false`.
|
// A ref, not state: two taps in one frame must not both see `saving === false`.
|
||||||
const inFlight = useRef(false);
|
const inFlight = useRef(false);
|
||||||
@@ -490,8 +501,8 @@ function EditSheet({
|
|||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (confirming) confirmRef.current?.focus();
|
if (confirming || confirmingDelete) confirmRef.current?.focus();
|
||||||
}, [confirming]);
|
}, [confirming, confirmingDelete]);
|
||||||
|
|
||||||
function onKeyDown(event: ReactKeyboardEvent<HTMLDivElement>) {
|
function onKeyDown(event: ReactKeyboardEvent<HTMLDivElement>) {
|
||||||
if (event.key === "Escape") {
|
if (event.key === "Escape") {
|
||||||
@@ -559,6 +570,21 @@ function EditSheet({
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function remove() {
|
||||||
|
if (target === "new" || inFlight.current) return;
|
||||||
|
setConfirmingDelete(false);
|
||||||
|
inFlight.current = true;
|
||||||
|
setSaving(true);
|
||||||
|
setError(null);
|
||||||
|
try {
|
||||||
|
onDeleted(await deleteDefinition(target.id));
|
||||||
|
} catch (err) {
|
||||||
|
setError(deleteErrorMessage(err));
|
||||||
|
inFlight.current = false;
|
||||||
|
setSaving(false);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="defs-sheet-layer">
|
<div className="defs-sheet-layer">
|
||||||
<div
|
<div
|
||||||
@@ -906,15 +932,56 @@ function EditSheet({
|
|||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
) : (
|
) : confirmingDelete ? (
|
||||||
<div className="defs-sheet-footer">
|
<div
|
||||||
<button type="button" className="defs-cancel" onClick={onCancel} disabled={saving}>
|
className="defs-confirm"
|
||||||
Cancel
|
role="group"
|
||||||
</button>
|
aria-labelledby="defs-delete-confirm-text"
|
||||||
<button type="button" className="defs-save" onClick={() => save()} disabled={saving}>
|
data-testid="delete-confirm"
|
||||||
{saving ? "Saving…" : "Save changes"}
|
ref={confirmRef}
|
||||||
</button>
|
tabIndex={-1}
|
||||||
|
>
|
||||||
|
<p id="defs-delete-confirm-text">
|
||||||
|
Delete this task? If it has no completion history it is deleted; otherwise it is
|
||||||
|
retired (deactivated) and its history is kept.
|
||||||
|
</p>
|
||||||
|
<div className="defs-sheet-footer">
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
className="defs-cancel"
|
||||||
|
onClick={() => setConfirmingDelete(false)}
|
||||||
|
>
|
||||||
|
Go back
|
||||||
|
</button>
|
||||||
|
<button type="button" className="defs-save defs-save--danger" onClick={remove}>
|
||||||
|
Delete task
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
) : (
|
||||||
|
<>
|
||||||
|
<div className="defs-sheet-footer">
|
||||||
|
<button type="button" className="defs-cancel" onClick={onCancel} disabled={saving}>
|
||||||
|
Cancel
|
||||||
|
</button>
|
||||||
|
<button type="button" className="defs-save" onClick={() => save()} disabled={saving}>
|
||||||
|
{saving ? "Saving…" : "Save changes"}
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
{target !== "new" ? (
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
className="defs-delete"
|
||||||
|
disabled={saving}
|
||||||
|
onClick={() => {
|
||||||
|
setError(null);
|
||||||
|
setConfirmingDelete(true);
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
Delete task
|
||||||
|
</button>
|
||||||
|
) : null}
|
||||||
|
</>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
@@ -1038,6 +1105,21 @@ export default function DefinitionsTab() {
|
|||||||
setSheet(null);
|
setSheet(null);
|
||||||
setAttempt((n) => n + 1);
|
setAttempt((n) => n + 1);
|
||||||
}}
|
}}
|
||||||
|
onDeleted={(result) => {
|
||||||
|
setSheet(null);
|
||||||
|
setState((current) =>
|
||||||
|
current.kind !== "ready"
|
||||||
|
? current
|
||||||
|
: {
|
||||||
|
...current,
|
||||||
|
definitions: result.definition
|
||||||
|
? current.definitions.map((d) =>
|
||||||
|
d.id === result.definition_id ? result.definition! : d,
|
||||||
|
)
|
||||||
|
: current.definitions.filter((d) => d.id !== result.definition_id),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}}
|
||||||
/>
|
/>
|
||||||
) : null}
|
) : null}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import { StrictMode } from "react";
|
||||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||||
import App from "../App";
|
import App from "../App";
|
||||||
@@ -21,6 +22,7 @@ const DEFAULT_SETTINGS: HouseholdSettings = {
|
|||||||
afternoon_reminder_enabled: false,
|
afternoon_reminder_enabled: false,
|
||||||
end_of_day_report_enabled: true,
|
end_of_day_report_enabled: true,
|
||||||
celebration_enabled: false,
|
celebration_enabled: false,
|
||||||
|
timezone: "America/Chicago",
|
||||||
};
|
};
|
||||||
|
|
||||||
function child(
|
function child(
|
||||||
@@ -370,10 +372,11 @@ describe("SettingsScreen — preferences", () => {
|
|||||||
fireEvent.click(within(form).getByRole("button", { name: "Save preferences" }));
|
fireEvent.click(within(form).getByRole("button", { name: "Save preferences" }));
|
||||||
}
|
}
|
||||||
|
|
||||||
it("renders the current value of the ten settings form fields (timezone is API-only)", async () => {
|
it("renders the current value of all eleven settings form fields", async () => {
|
||||||
const { form } = await renderPreferences();
|
const { form } = await renderPreferences();
|
||||||
expect(field(form, "Planning horizon (days)").value).toBe("14");
|
expect(field(form, "Planning horizon (days)").value).toBe("14");
|
||||||
expect(field(form, "Day rollover").value).toBe("03:00");
|
expect(field(form, "Day rollover").value).toBe("03:00");
|
||||||
|
expect(field(form, "Timezone").value).toBe("America/Chicago");
|
||||||
expect(field(form, "Notify target").value).toBe("notify.family");
|
expect(field(form, "Notify target").value).toBe("notify.family");
|
||||||
expect(field(form, "Morning summary time").value).toBe("07:00");
|
expect(field(form, "Morning summary time").value).toBe("07:00");
|
||||||
expect(field(form, "Afternoon reminder time").value).toBe("15:30");
|
expect(field(form, "Afternoon reminder time").value).toBe("15:30");
|
||||||
@@ -496,6 +499,46 @@ describe("SettingsScreen — preferences", () => {
|
|||||||
expect(writes()).toHaveLength(1);
|
expect(writes()).toHaveLength(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("a changed timezone patches exactly the timezone", async () => {
|
||||||
|
const { form } = await renderPreferences();
|
||||||
|
const input = field(form, "Timezone");
|
||||||
|
expect(input.getAttribute("list")).toBe("settings-timezones");
|
||||||
|
fireEvent.change(input, { target: { value: " Europe/London " } });
|
||||||
|
save(form);
|
||||||
|
await screen.findByText("Preferences saved.");
|
||||||
|
expect(writes()).toEqual([{ method: "PATCH", path: SETTINGS_PATH, body: { timezone: "Europe/London" } }]);
|
||||||
|
const reloaded = screen.getByRole("form", { name: "Preferences" });
|
||||||
|
expect(field(reloaded, "Timezone").value).toBe("Europe/London");
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each(["New York", "America/New York", "EST5EDT?", "/Chicago"])(
|
||||||
|
"an invalid timezone %j is rejected before any request",
|
||||||
|
async (value) => {
|
||||||
|
const { form } = await renderPreferences();
|
||||||
|
fireEvent.change(field(form, "Timezone"), { target: { value } });
|
||||||
|
save(form);
|
||||||
|
expect(screen.getByTestId("preferences-message").textContent).toBe(
|
||||||
|
"Timezone must be an IANA zone like America/New_York, or UTC.",
|
||||||
|
);
|
||||||
|
expect(field(form, "Timezone").value).toBe(value);
|
||||||
|
expect(writes()).toEqual([]);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
it("a 422 on an unknown timezone shows the API detail and keeps the edit", async () => {
|
||||||
|
const { form } = await renderPreferences();
|
||||||
|
fireEvent.change(field(form, "Timezone"), { target: { value: "Mars/Olympus_Mons" } });
|
||||||
|
writeResponse = () =>
|
||||||
|
jsonResponse({ detail: "timezone must be an IANA time zone name such as 'America/New_York'" }, 422);
|
||||||
|
save(form);
|
||||||
|
const message = await screen.findByTestId("preferences-message");
|
||||||
|
expect(message.textContent).toBe(
|
||||||
|
"The preferences could not be saved: timezone must be an IANA time zone name such as 'America/New_York'",
|
||||||
|
);
|
||||||
|
expect(field(form, "Timezone").value).toBe("Mars/Olympus_Mons");
|
||||||
|
expect(field(form, "Timezone").disabled).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
it("a failed load shows the reason and Try again refetches", async () => {
|
it("a failed load shows the reason and Try again refetches", async () => {
|
||||||
let fail = true;
|
let fail = true;
|
||||||
fetchMock.mockImplementation(async (url: string, init?: RequestInit) => {
|
fetchMock.mockImplementation(async (url: string, init?: RequestInit) => {
|
||||||
@@ -512,3 +555,114 @@ describe("SettingsScreen — preferences", () => {
|
|||||||
await screen.findByRole("form", { name: "Preferences" });
|
await screen.findByRole("form", { name: "Preferences" });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("SettingsScreen — timezone auto-set", () => {
|
||||||
|
let zoneSpy: ReturnType<typeof vi.spyOn> | null;
|
||||||
|
|
||||||
|
function mockBrowserZone(timeZone: string | undefined) {
|
||||||
|
const real = Intl.DateTimeFormat.prototype.resolvedOptions;
|
||||||
|
zoneSpy = vi
|
||||||
|
.spyOn(Intl.DateTimeFormat.prototype, "resolvedOptions")
|
||||||
|
.mockImplementation(function (this: Intl.DateTimeFormat) {
|
||||||
|
return { ...real.call(this), timeZone } as Intl.ResolvedDateTimeFormatOptions;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
sessionStorage.clear();
|
||||||
|
storeTokens({ access_token: "at-123", expires_in: 600 });
|
||||||
|
children = [child(1, "Declan", 1)];
|
||||||
|
settings = { ...DEFAULT_SETTINGS, timezone: "" };
|
||||||
|
writeResponse = null;
|
||||||
|
zoneSpy = null;
|
||||||
|
fetchMock = vi.fn(async (url: string, init?: RequestInit) => routeFetch(url, init));
|
||||||
|
vi.stubGlobal("fetch", fetchMock);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
zoneSpy?.mockRestore();
|
||||||
|
vi.unstubAllGlobals();
|
||||||
|
});
|
||||||
|
|
||||||
|
async function renderForm() {
|
||||||
|
render(<SettingsScreen onClose={vi.fn()} />);
|
||||||
|
await screen.findByTestId("settings-child-1");
|
||||||
|
return screen.findByRole("form", { name: "Preferences" });
|
||||||
|
}
|
||||||
|
|
||||||
|
it("an unset timezone is set once, on load, to the browser's zone and shown", async () => {
|
||||||
|
mockBrowserZone("America/New_York");
|
||||||
|
const form = await renderForm();
|
||||||
|
expect(field(form, "Timezone").value).toBe("America/New_York");
|
||||||
|
expect(writes()).toEqual([
|
||||||
|
{ method: "PATCH", path: SETTINGS_PATH, body: { timezone: "America/New_York" } },
|
||||||
|
]);
|
||||||
|
expect(settings.timezone).toBe("America/New_York");
|
||||||
|
|
||||||
|
// A later save of another field does not re-send the timezone.
|
||||||
|
fireEvent.change(field(form, "Planning horizon (days)"), { target: { value: "21" } });
|
||||||
|
fireEvent.click(within(form).getByRole("button", { name: "Save preferences" }));
|
||||||
|
await screen.findByText("Preferences saved.");
|
||||||
|
expect(writes()).toHaveLength(2);
|
||||||
|
expect(writes()[1].body).toEqual({ horizon_days: 21 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("StrictMode's doubled effects still auto-set exactly once", async () => {
|
||||||
|
mockBrowserZone("America/New_York");
|
||||||
|
render(
|
||||||
|
<StrictMode>
|
||||||
|
<SettingsScreen onClose={vi.fn()} />
|
||||||
|
</StrictMode>,
|
||||||
|
);
|
||||||
|
const form = await screen.findByRole("form", { name: "Preferences" });
|
||||||
|
expect(field(form, "Timezone").value).toBe("America/New_York");
|
||||||
|
expect(writes()).toEqual([
|
||||||
|
{ method: "PATCH", path: SETTINGS_PATH, body: { timezone: "America/New_York" } },
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("an admin clearing the timezone is not re-set by the browser zone", async () => {
|
||||||
|
mockBrowserZone("America/New_York");
|
||||||
|
const form = await renderForm();
|
||||||
|
fireEvent.change(field(form, "Timezone"), { target: { value: "" } });
|
||||||
|
fireEvent.click(within(form).getByRole("button", { name: "Save preferences" }));
|
||||||
|
await screen.findByText("Preferences saved.");
|
||||||
|
expect(writes().map((w) => w.body)).toEqual([{ timezone: "America/New_York" }, { timezone: "" }]);
|
||||||
|
const reloaded = screen.getByRole("form", { name: "Preferences" });
|
||||||
|
expect(field(reloaded, "Timezone").value).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("a set timezone is shown and never overwritten by the browser zone", async () => {
|
||||||
|
mockBrowserZone("America/New_York");
|
||||||
|
settings = { ...DEFAULT_SETTINGS, timezone: "Europe/Dublin" };
|
||||||
|
const form = await renderForm();
|
||||||
|
expect(field(form, "Timezone").value).toBe("Europe/Dublin");
|
||||||
|
expect(writes()).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["nothing", undefined],
|
||||||
|
["an empty zone", ""],
|
||||||
|
["an unknown zone", "Etc/Unknown"],
|
||||||
|
["a non-IANA zone", "not a zone"],
|
||||||
|
])("detection returning %s leaves the field empty and sends nothing", async (_case, zone) => {
|
||||||
|
mockBrowserZone(zone);
|
||||||
|
const form = await renderForm();
|
||||||
|
expect(field(form, "Timezone").value).toBe("");
|
||||||
|
expect(screen.queryByTestId("preferences-message")).toBeNull();
|
||||||
|
expect(writes()).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("a rejected auto-set shows why and leaves the field empty for manual entry", async () => {
|
||||||
|
mockBrowserZone("America/New_York");
|
||||||
|
writeResponse = () => jsonResponse({ detail: "timezone is not known" }, 422);
|
||||||
|
const form = await renderForm();
|
||||||
|
expect(field(form, "Timezone").value).toBe("");
|
||||||
|
expect(field(form, "Timezone").disabled).toBe(false);
|
||||||
|
expect(screen.getByTestId("preferences-message").textContent).toBe(
|
||||||
|
"The timezone could not be set to America/New_York: timezone is not known",
|
||||||
|
);
|
||||||
|
expect(writes()).toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -5,7 +5,8 @@
|
|||||||
* household's child profiles. Every write refetches the list.
|
* household's child profiles. Every write refetches the list.
|
||||||
*
|
*
|
||||||
* Preferences: the household settings (planning horizon, day rollover,
|
* Preferences: the household settings (planning horizon, day rollover,
|
||||||
* notifications). Save patches only the changed fields.
|
* timezone, notifications). Save patches only the changed fields. An unset
|
||||||
|
* timezone is set once, on load, to the browser's own zone.
|
||||||
*
|
*
|
||||||
* One write at a time across both sections.
|
* One write at a time across both sections.
|
||||||
*/
|
*/
|
||||||
@@ -252,6 +253,56 @@ const NOTIFICATIONS: { label: string; time: TimeField; toggle: ToggleField }[] =
|
|||||||
|
|
||||||
const HH_MM = /^([01]\d|2[0-3]):[0-5]\d$/;
|
const HH_MM = /^([01]\d|2[0-3]):[0-5]\d$/;
|
||||||
|
|
||||||
|
/** "UTC" or an IANA-looking "Area/Location" (the API checks it resolves). */
|
||||||
|
const IANA_ZONE = /^(UTC|[A-Za-z]+(\/[A-Za-z0-9_+-]+)+)$/;
|
||||||
|
|
||||||
|
/** The Timezone field's suggestions; any other IANA zone can be typed. */
|
||||||
|
export const COMMON_TIMEZONES = [
|
||||||
|
"UTC",
|
||||||
|
"America/New_York",
|
||||||
|
"America/Chicago",
|
||||||
|
"America/Denver",
|
||||||
|
"America/Phoenix",
|
||||||
|
"America/Los_Angeles",
|
||||||
|
"America/Anchorage",
|
||||||
|
"Pacific/Honolulu",
|
||||||
|
"America/Halifax",
|
||||||
|
"America/St_Johns",
|
||||||
|
"America/Toronto",
|
||||||
|
"America/Vancouver",
|
||||||
|
"America/Mexico_City",
|
||||||
|
"America/Sao_Paulo",
|
||||||
|
"Europe/London",
|
||||||
|
"Europe/Dublin",
|
||||||
|
"Europe/Paris",
|
||||||
|
"Europe/Berlin",
|
||||||
|
"Europe/Madrid",
|
||||||
|
"Europe/Rome",
|
||||||
|
"Europe/Athens",
|
||||||
|
"Africa/Johannesburg",
|
||||||
|
"Asia/Dubai",
|
||||||
|
"Asia/Kolkata",
|
||||||
|
"Asia/Singapore",
|
||||||
|
"Asia/Shanghai",
|
||||||
|
"Asia/Tokyo",
|
||||||
|
"Australia/Perth",
|
||||||
|
"Australia/Sydney",
|
||||||
|
"Pacific/Auckland",
|
||||||
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The browser's own IANA zone — the location-derived default for an unset
|
||||||
|
* household timezone — or "" when it is missing or not a usable zone name.
|
||||||
|
*/
|
||||||
|
export function detectTimezone(): string {
|
||||||
|
try {
|
||||||
|
const zone = Intl.DateTimeFormat().resolvedOptions().timeZone;
|
||||||
|
return typeof zone === "string" && zone !== "Etc/Unknown" && IANA_ZONE.test(zone) ? zone : "";
|
||||||
|
} catch {
|
||||||
|
return "";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/** The form's working copy: the number is text so it can be mid-edit. */
|
/** The form's working copy: the number is text so it can be mid-edit. */
|
||||||
type PreferencesDraft = Omit<HouseholdSettings, "horizon_days"> & { horizon_days: string };
|
type PreferencesDraft = Omit<HouseholdSettings, "horizon_days"> & { horizon_days: string };
|
||||||
|
|
||||||
@@ -275,11 +326,16 @@ function buildChanges(draft: PreferencesDraft, loaded: HouseholdSettings): Setti
|
|||||||
for (const [key, label] of times) {
|
for (const [key, label] of times) {
|
||||||
if (!HH_MM.test(draft[key])) return `${label} must be a time like 07:30.`;
|
if (!HH_MM.test(draft[key])) return `${label} must be a time like 07:30.`;
|
||||||
}
|
}
|
||||||
|
const timezone = draft.timezone.trim();
|
||||||
|
if (timezone && !IANA_ZONE.test(timezone)) {
|
||||||
|
return "Timezone must be an IANA zone like America/New_York, or UTC.";
|
||||||
|
}
|
||||||
|
|
||||||
const next: HouseholdSettings = {
|
const next: HouseholdSettings = {
|
||||||
...draft,
|
...draft,
|
||||||
horizon_days: horizonDays,
|
horizon_days: horizonDays,
|
||||||
notify_target: draft.notify_target.trim(),
|
notify_target: draft.notify_target.trim(),
|
||||||
|
timezone,
|
||||||
};
|
};
|
||||||
const changes: SettingsChanges = {};
|
const changes: SettingsChanges = {};
|
||||||
for (const key of Object.keys(loaded) as (keyof HouseholdSettings)[]) {
|
for (const key of Object.keys(loaded) as (keyof HouseholdSettings)[]) {
|
||||||
@@ -357,6 +413,25 @@ function PreferencesForm({
|
|||||||
/>
|
/>
|
||||||
</label>
|
</label>
|
||||||
{timeInput("day_rollover_time", "Day rollover")}
|
{timeInput("day_rollover_time", "Day rollover")}
|
||||||
|
<label className="settings-field">
|
||||||
|
<span className="settings-label">Timezone</span>
|
||||||
|
<input
|
||||||
|
className="settings-input"
|
||||||
|
type="text"
|
||||||
|
list="settings-timezones"
|
||||||
|
placeholder="Host local time"
|
||||||
|
autoComplete="off"
|
||||||
|
spellCheck={false}
|
||||||
|
value={draft.timezone}
|
||||||
|
disabled={disabled}
|
||||||
|
onChange={(event) => update({ timezone: event.target.value })}
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
<datalist id="settings-timezones">
|
||||||
|
{COMMON_TIMEZONES.map((zone) => (
|
||||||
|
<option key={zone} value={zone} />
|
||||||
|
))}
|
||||||
|
</datalist>
|
||||||
<label className="settings-field">
|
<label className="settings-field">
|
||||||
<span className="settings-label">Notify target</span>
|
<span className="settings-label">Notify target</span>
|
||||||
<input
|
<input
|
||||||
@@ -495,6 +570,8 @@ export default function SettingsScreen({ onClose }: { onClose: () => void }) {
|
|||||||
} | null>(null);
|
} | null>(null);
|
||||||
// A ref, not state: two taps in one frame must not both see `busy === null`.
|
// A ref, not state: two taps in one frame must not both see `busy === null`.
|
||||||
const inFlight = useRef(false);
|
const inFlight = useRef(false);
|
||||||
|
// The unset-timezone auto-set fires at most once per screen.
|
||||||
|
const timezoneAutoSet = useRef(false);
|
||||||
const screenRef = useRef<HTMLDivElement>(null);
|
const screenRef = useRef<HTMLDivElement>(null);
|
||||||
const titleRef = useRef<HTMLHeadingElement>(null);
|
const titleRef = useRef<HTMLHeadingElement>(null);
|
||||||
const addRef = useRef<HTMLButtonElement>(null);
|
const addRef = useRef<HTMLButtonElement>(null);
|
||||||
@@ -539,8 +616,27 @@ export default function SettingsScreen({ onClose }: { onClose: () => void }) {
|
|||||||
let cancelled = false;
|
let cancelled = false;
|
||||||
setPreferences({ kind: "loading" });
|
setPreferences({ kind: "loading" });
|
||||||
fetchSettings()
|
fetchSettings()
|
||||||
.then((settings) => {
|
.then(async (settings) => {
|
||||||
if (!cancelled) setPreferences({ kind: "ready", settings });
|
if (cancelled) return;
|
||||||
|
// Unset: adopt the browser's zone before the form shows, so nothing
|
||||||
|
// the admin enters can be overwritten by it.
|
||||||
|
const detected = settings.timezone === "" ? detectTimezone() : "";
|
||||||
|
if (!detected || timezoneAutoSet.current) {
|
||||||
|
setPreferences({ kind: "ready", settings });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
timezoneAutoSet.current = true;
|
||||||
|
try {
|
||||||
|
const saved = await updateSettings({ timezone: detected });
|
||||||
|
if (!cancelled) setPreferences({ kind: "ready", settings: saved });
|
||||||
|
} catch (error) {
|
||||||
|
if (cancelled) return;
|
||||||
|
setPreferences({ kind: "ready", settings });
|
||||||
|
setPreferencesMessage({
|
||||||
|
kind: "error",
|
||||||
|
text: errorMessage(error, `The timezone could not be set to ${detected}`),
|
||||||
|
});
|
||||||
|
}
|
||||||
})
|
})
|
||||||
.catch((error: unknown) => {
|
.catch((error: unknown) => {
|
||||||
if (cancelled) return;
|
if (cancelled) return;
|
||||||
|
|||||||
+74
-4
@@ -200,6 +200,25 @@ integration's rollover listener shares:
|
|||||||
and the read-only-over-the-domain-tables guarantee all live in the
|
and the read-only-over-the-domain-tables guarantee all live in the
|
||||||
core; the response reports how many transitions were published.
|
core; the response reports how many transitions were published.
|
||||||
|
|
||||||
|
The quest-definition DELETE route (task 3efb07b4) is the hybrid admin
|
||||||
|
delete (owner decision 2026-09-26: "delete if there's no history,
|
||||||
|
retire/deactivate if there is"):
|
||||||
|
|
||||||
|
- ``DELETE /quest-definitions/{definition_id}`` is a thin adapter over
|
||||||
|
:func:`nestquest_core.quest_definitions.delete_quest_definition`. A
|
||||||
|
definition with NO completion event on any instance is hard-deleted
|
||||||
|
(instances, assignees, windows, the row and its unreferenced rule, in
|
||||||
|
one core transaction); one WITH history is retired by deactivation.
|
||||||
|
Retirement preserves the definition row, schedule rule, assignees,
|
||||||
|
windows and ALL durable completion history; like any deactivation it
|
||||||
|
regenerates, so future UNCOMPLETED instances are cleared (rolling,
|
||||||
|
regenerable rows) and none are recreated while the definition is
|
||||||
|
inactive — the retired task stops generating and shows no open work.
|
||||||
|
Durable instance history lives only in the append-only
|
||||||
|
``completion_events``. The response reports which happened
|
||||||
|
(``deleted`` / ``retired``) and carries the retired definition, or
|
||||||
|
``null`` when deleted. An unknown id is 404.
|
||||||
|
|
||||||
The occurrence-preview route (task 5f843564) lets the admin PWA's
|
The occurrence-preview route (task 5f843564) lets the admin PWA's
|
||||||
Definitions edit sheet show the next dates a rule fires on WITHOUT
|
Definitions edit sheet show the next dates a rule fires on WITHOUT
|
||||||
reimplementing the recurrence rules client-side:
|
reimplementing the recurrence rules client-side:
|
||||||
@@ -453,6 +472,19 @@ class AdminQuestDefinitionListResponse(BaseModel):
|
|||||||
definitions: list[AdminQuestDefinitionResponse]
|
definitions: list[AdminQuestDefinitionResponse]
|
||||||
|
|
||||||
|
|
||||||
|
class AdminQuestDefinitionDeleteResponse(BaseModel):
|
||||||
|
"""The body ``DELETE /api/v1/admin/quest-definitions/{id}`` answers.
|
||||||
|
|
||||||
|
Exactly one of ``deleted`` / ``retired`` is true. ``definition`` is
|
||||||
|
the retired (now inactive) definition, or null when it was deleted.
|
||||||
|
"""
|
||||||
|
|
||||||
|
definition_id: int
|
||||||
|
deleted: bool
|
||||||
|
retired: bool
|
||||||
|
definition: AdminQuestDefinitionResponse | None
|
||||||
|
|
||||||
|
|
||||||
class AdminChildCreateRequest(BaseModel):
|
class AdminChildCreateRequest(BaseModel):
|
||||||
"""The body of ``POST /api/v1/admin/children``.
|
"""The body of ``POST /api/v1/admin/children``.
|
||||||
|
|
||||||
@@ -1611,16 +1643,16 @@ async def admin_set_quest_definition_active(
|
|||||||
body: AdminQuestDefinitionActiveRequest,
|
body: AdminQuestDefinitionActiveRequest,
|
||||||
request: Request,
|
request: Request,
|
||||||
) -> AdminQuestDefinitionResponse:
|
) -> AdminQuestDefinitionResponse:
|
||||||
"""Deactivate (or reactivate) a definition; it is NEVER deleted.
|
"""Deactivate (or reactivate) a definition; it is never deleted here.
|
||||||
|
|
||||||
Requires a valid admin JWT (the router's shared
|
Requires a valid admin JWT (the router's shared
|
||||||
:func:`~api.auth.require_admin` dependency — the ONE check; this
|
:func:`~api.auth.require_admin` dependency — the ONE check; this
|
||||||
handler performs NO auth of its own). Thin adapter: ``is_active``
|
handler performs NO auth of its own). Thin adapter: ``is_active``
|
||||||
goes straight to
|
goes straight to
|
||||||
:func:`nestquest_core.quest_definitions.set_quest_definition_active`
|
:func:`nestquest_core.quest_definitions.set_quest_definition_active`
|
||||||
— deactivation is the only removal path (the row, rule, assignees
|
— the row, rule, assignees and windows all survive, so completion
|
||||||
and windows all survive, so completion history keeps its
|
history keeps its references (the hybrid DELETE route is the only
|
||||||
references). An unknown definition id is mapped to 404
|
hard-delete path, and only for a definition without history). An unknown definition id is mapped to 404
|
||||||
(:func:`_raise_quest_definition_error`); the body model already
|
(:func:`_raise_quest_definition_error`); the body model already
|
||||||
makes the core's real-bool check unreachable from this route.
|
makes the core's real-bool check unreachable from this route.
|
||||||
"""
|
"""
|
||||||
@@ -1634,6 +1666,44 @@ async def admin_set_quest_definition_active(
|
|||||||
return _quest_definition_response(bundle)
|
return _quest_definition_response(bundle)
|
||||||
|
|
||||||
|
|
||||||
|
@router.delete(
|
||||||
|
"/quest-definitions/{definition_id}",
|
||||||
|
summary="Delete a quest definition, or retire it if it has history",
|
||||||
|
response_model=AdminQuestDefinitionDeleteResponse,
|
||||||
|
)
|
||||||
|
async def admin_delete_quest_definition(
|
||||||
|
definition_id: int, request: Request
|
||||||
|
) -> AdminQuestDefinitionDeleteResponse:
|
||||||
|
"""Hard-delete a history-free definition; retire one with history.
|
||||||
|
|
||||||
|
Requires a valid admin JWT (the router's shared
|
||||||
|
:func:`~api.auth.require_admin` dependency — the ONE check; this
|
||||||
|
handler performs NO auth of its own). Thin adapter: the id goes
|
||||||
|
straight to
|
||||||
|
:func:`nestquest_core.quest_definitions.delete_quest_definition`,
|
||||||
|
which decides delete-vs-retire atomically in the core. An unknown
|
||||||
|
definition id is mapped to 404
|
||||||
|
(:func:`_raise_quest_definition_error`).
|
||||||
|
"""
|
||||||
|
state: DatabaseState = request.app.state.db
|
||||||
|
try:
|
||||||
|
result = await core_quest_definitions.delete_quest_definition(
|
||||||
|
state.database, definition_id
|
||||||
|
)
|
||||||
|
except ValueError as error:
|
||||||
|
_raise_quest_definition_error(error)
|
||||||
|
return AdminQuestDefinitionDeleteResponse(
|
||||||
|
definition_id=result.definition_id,
|
||||||
|
deleted=result.deleted,
|
||||||
|
retired=result.retired,
|
||||||
|
definition=(
|
||||||
|
None
|
||||||
|
if result.definition is None
|
||||||
|
else _quest_definition_response(result.definition)
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.get(
|
@router.get(
|
||||||
"/children/{child_id}/presence-patterns",
|
"/children/{child_id}/presence-patterns",
|
||||||
summary="List a child's repeating presence patterns",
|
summary="List a child's repeating presence patterns",
|
||||||
|
|||||||
@@ -13,8 +13,12 @@ Guardrail mapping:
|
|||||||
- ``schedule_rules`` deletion is delete-if-unreferenced: a rule still
|
- ``schedule_rules`` deletion is delete-if-unreferenced: a rule still
|
||||||
referenced by any quest definition is rejected (the done-condition),
|
referenced by any quest definition is rejected (the done-condition),
|
||||||
not cascaded.
|
not cascaded.
|
||||||
- ``quest_definitions`` has no delete path at all: definitions are
|
- ``quest_definitions`` has exactly one delete path,
|
||||||
deactivated, never hard-deleted, and history must survive.
|
:meth:`QuestDefinitionsDao.delete_if_no_history` (owner decision
|
||||||
|
2026-09-26): a definition with NO completion event on any of its
|
||||||
|
instances is hard-deleted with its instances, assignees, windows and
|
||||||
|
(if unreferenced) rule; a definition WITH history is never deleted —
|
||||||
|
it is retired by deactivation, so history always survives.
|
||||||
- Assignment is multi-assignee (D-008): :meth:`add_assignee` and
|
- Assignment is multi-assignee (D-008): :meth:`add_assignee` and
|
||||||
:meth:`remove_assignee` change future instances only — the DAO writes
|
:meth:`remove_assignee` change future instances only — the DAO writes
|
||||||
the assignee table and nothing else; assignment edits never rewrite
|
the assignee table and nothing else; assignment edits never rewrite
|
||||||
@@ -25,8 +29,8 @@ Guardrail mapping:
|
|||||||
:meth:`QuestDefinitionsDao.add_assignee` (active child); no
|
:meth:`QuestDefinitionsDao.add_assignee` (active child); no
|
||||||
permission checks here, that is the Feature 09 gate's job.
|
permission checks here, that is the Feature 09 gate's job.
|
||||||
|
|
||||||
Concurrency: ``delete_rule_if_unreferenced`` and the assignee
|
Concurrency: ``delete_rule_if_unreferenced``,
|
||||||
mutations perform their checks and writes inside one transaction under
|
``delete_if_no_history`` and the assignee mutations perform their checks and writes inside one transaction under
|
||||||
the connection-scoped lock shared with :mod:`.dao_children`, so a
|
the connection-scoped lock shared with :mod:`.dao_children`, so a
|
||||||
definition referencing the rule (or an assignee appearing) cannot slip
|
definition referencing the rule (or an assignee appearing) cannot slip
|
||||||
in between check and write on this connection.
|
in between check and write on this connection.
|
||||||
@@ -608,26 +612,42 @@ class ScheduleRulesDao:
|
|||||||
"""
|
"""
|
||||||
async with _connection_lock(self._database):
|
async with _connection_lock(self._database):
|
||||||
async with self._database.transaction():
|
async with self._database.transaction():
|
||||||
row = await self._database.fetch_one(
|
return await _delete_rule_if_unreferenced_in_transaction(
|
||||||
"SELECT 1 FROM quest_definitions "
|
self._database, rule_id
|
||||||
"WHERE schedule_rule_id = ? LIMIT 1",
|
|
||||||
(rule_id,),
|
|
||||||
)
|
)
|
||||||
if row is not None:
|
|
||||||
return False
|
|
||||||
result = await self._database.execute(
|
async def _delete_rule_if_unreferenced_in_transaction(
|
||||||
"DELETE FROM schedule_rules WHERE id = ?",
|
database: NestQuestDatabase, rule_id: int
|
||||||
(rule_id,),
|
) -> bool:
|
||||||
)
|
"""Delete-if-unreferenced body; the CALLER holds lock + transaction.
|
||||||
return result.rowcount > 0
|
|
||||||
|
Shared by :meth:`ScheduleRulesDao.delete_if_unreferenced` and
|
||||||
|
:meth:`QuestDefinitionsDao.delete_if_no_history`, so the rule's
|
||||||
|
reference check runs in whichever transaction removed the last
|
||||||
|
definition that could reference it.
|
||||||
|
"""
|
||||||
|
row = await database.fetch_one(
|
||||||
|
"SELECT 1 FROM quest_definitions "
|
||||||
|
"WHERE schedule_rule_id = ? LIMIT 1",
|
||||||
|
(rule_id,),
|
||||||
|
)
|
||||||
|
if row is not None:
|
||||||
|
return False
|
||||||
|
result = await database.execute(
|
||||||
|
"DELETE FROM schedule_rules WHERE id = ?",
|
||||||
|
(rule_id,),
|
||||||
|
)
|
||||||
|
return result.rowcount > 0
|
||||||
|
|
||||||
|
|
||||||
class QuestDefinitionsDao:
|
class QuestDefinitionsDao:
|
||||||
"""Typed async access to ``quest_definitions`` and its assignees.
|
"""Typed async access to ``quest_definitions`` and its assignees.
|
||||||
|
|
||||||
No delete method exists: definitions are deactivated via
|
The only delete method is :meth:`delete_if_no_history`: a
|
||||||
:meth:`set_active`, never hard-deleted, so completion history keeps
|
definition with completion history is never hard-deleted (it is
|
||||||
its references (feature guardrail). Assignment is multi-assignee
|
deactivated via :meth:`set_active` instead), so completion history
|
||||||
|
always keeps its references (feature guardrail, D-005). Assignment is multi-assignee
|
||||||
(D-008): the definition row carries no child; assignees live in
|
(D-008): the definition row carries no child; assignees live in
|
||||||
``quest_definition_assignees`` and are managed explicitly so the
|
``quest_definition_assignees`` and are managed explicitly so the
|
||||||
operations stay separately loggable and permission-gateable.
|
operations stay separately loggable and permission-gateable.
|
||||||
@@ -1140,6 +1160,54 @@ class QuestDefinitionsDao:
|
|||||||
)
|
)
|
||||||
return result.rowcount
|
return result.rowcount
|
||||||
|
|
||||||
|
async def delete_if_no_history(self, definition_id: int) -> bool | None:
|
||||||
|
"""Hard-delete a definition that has NO completion history.
|
||||||
|
|
||||||
|
Returns None when the definition does not exist, False when any
|
||||||
|
of its instances has a completion event (nothing is touched —
|
||||||
|
the caller retires it by deactivation instead), and True when it
|
||||||
|
was deleted: its ``quest_instances``, assignee and window rows,
|
||||||
|
the definition row, and its schedule rule if no other definition
|
||||||
|
references it.
|
||||||
|
|
||||||
|
The history check and every DELETE run inside ONE transaction
|
||||||
|
under the connection lock, so a completion recorded concurrently
|
||||||
|
cannot land between the check and the delete (D-005: an instance
|
||||||
|
with a completion event is never deleted).
|
||||||
|
"""
|
||||||
|
async with _connection_lock(self._database):
|
||||||
|
async with self._database.transaction():
|
||||||
|
row = await self._database.fetch_one(
|
||||||
|
"SELECT schedule_rule_id FROM quest_definitions "
|
||||||
|
"WHERE id = ?",
|
||||||
|
(definition_id,),
|
||||||
|
)
|
||||||
|
if row is None:
|
||||||
|
return None
|
||||||
|
schedule_rule_id = row[0]
|
||||||
|
history = await self._database.fetch_one(
|
||||||
|
"SELECT 1 FROM completion_events "
|
||||||
|
"JOIN quest_instances "
|
||||||
|
"ON quest_instances.id = completion_events.instance_id "
|
||||||
|
"WHERE quest_instances.definition_id = ? LIMIT 1",
|
||||||
|
(definition_id,),
|
||||||
|
)
|
||||||
|
if history is not None:
|
||||||
|
return False
|
||||||
|
for statement in (
|
||||||
|
"DELETE FROM quest_instances WHERE definition_id = ?",
|
||||||
|
"DELETE FROM quest_definition_assignees "
|
||||||
|
"WHERE definition_id = ?",
|
||||||
|
"DELETE FROM quest_definition_windows "
|
||||||
|
"WHERE definition_id = ?",
|
||||||
|
"DELETE FROM quest_definitions WHERE id = ?",
|
||||||
|
):
|
||||||
|
await self._database.execute(statement, (definition_id,))
|
||||||
|
await _delete_rule_if_unreferenced_in_transaction(
|
||||||
|
self._database, schedule_rule_id
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
async def add_assignee(
|
async def add_assignee(
|
||||||
self, definition_id: int, child_id: int
|
self, definition_id: int, child_id: int
|
||||||
) -> list[ChildRecord]:
|
) -> list[ChildRecord]:
|
||||||
|
|||||||
@@ -7,7 +7,9 @@ all-or-nothing create/edit here, and never touch the DAO SQL directly.
|
|||||||
:func:`edit_quest_definition` the edit path (which may also replace the
|
:func:`edit_quest_definition` the edit path (which may also replace the
|
||||||
whole assignee set), :func:`assign_child` and
|
whole assignee set), :func:`assign_child` and
|
||||||
:func:`unassign_child` the single-child assignment paths, and
|
:func:`unassign_child` the single-child assignment paths, and
|
||||||
:func:`set_quest_definition_active` the deactivate/reactivate path.
|
:func:`set_quest_definition_active` the deactivate/reactivate path,
|
||||||
|
and :func:`delete_quest_definition` the hybrid admin delete (hard-delete
|
||||||
|
when there is no completion history, retire by deactivation otherwise).
|
||||||
There is no permission check here (that is Feature 09).
|
There is no permission check here (that is Feature 09).
|
||||||
|
|
||||||
Validation policy mirrors :mod:`.children`:
|
Validation policy mirrors :mod:`.children`:
|
||||||
@@ -80,6 +82,21 @@ class CreatedQuestDefinition:
|
|||||||
windows: list[QuestDefinitionWindowRecord]
|
windows: list[QuestDefinitionWindowRecord]
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class DeletedQuestDefinition:
|
||||||
|
"""What :func:`delete_quest_definition` actually did.
|
||||||
|
|
||||||
|
Exactly one of ``deleted`` / ``retired`` is True. ``definition`` is
|
||||||
|
the retired (deactivated) definition's updated view, or None when
|
||||||
|
the definition was hard-deleted and no longer exists.
|
||||||
|
"""
|
||||||
|
|
||||||
|
definition_id: int
|
||||||
|
deleted: bool
|
||||||
|
retired: bool
|
||||||
|
definition: CreatedQuestDefinition | None
|
||||||
|
|
||||||
|
|
||||||
def _now_stamp() -> str:
|
def _now_stamp() -> str:
|
||||||
"""Return the module's strict UTC second-precision stamp."""
|
"""Return the module's strict UTC second-precision stamp."""
|
||||||
return datetime.datetime.now(datetime.timezone.utc).isoformat(
|
return datetime.datetime.now(datetime.timezone.utc).isoformat(
|
||||||
@@ -504,12 +521,19 @@ async def set_quest_definition_active(
|
|||||||
) -> CreatedQuestDefinition:
|
) -> CreatedQuestDefinition:
|
||||||
"""Deactivate or reactivate a definition; returns the updated view.
|
"""Deactivate or reactivate a definition; returns the updated view.
|
||||||
|
|
||||||
Flipping the flag is the only removal path (Feature 06 guardrail):
|
Flipping the flag is the only removal path for a definition with
|
||||||
|
completion history (Feature 06 guardrail; a history-free definition
|
||||||
|
may instead be hard-deleted by :func:`delete_quest_definition`):
|
||||||
a deactivated definition stops future instance generation but its
|
a deactivated definition stops future instance generation but its
|
||||||
row, schedule rule, assignees and windows all survive, and existing
|
row, schedule rule, assignees and windows all survive, and its
|
||||||
instances plus completion history are never touched. Reactivation
|
durable completion history (the append-only ``completion_events``)
|
||||||
re-enables future generation; generating or removing instances is
|
is never touched. Like every Feature 06 config change, the flip is
|
||||||
Feature 07 and does not happen here.
|
followed by :func:`~.materialize.regenerate_for_definition`: the
|
||||||
|
definition's future (today onward) UNCOMPLETED instances are cleared
|
||||||
|
— they are rolling, regenerable rows — and, while the definition is
|
||||||
|
inactive, none are re-materialized. Completed and past instances
|
||||||
|
are kept. Reactivation re-enables generation, and the same
|
||||||
|
regeneration re-materializes the rolling horizon.
|
||||||
|
|
||||||
``definition_id`` must be a plain int (bools and floats rejected,
|
``definition_id`` must be a plain int (bools and floats rejected,
|
||||||
since SQLite would bind ``True`` onto definition 1) and
|
since SQLite would bind ``True`` onto definition 1) and
|
||||||
@@ -562,6 +586,68 @@ async def set_quest_definition_active(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def delete_quest_definition(
|
||||||
|
database: NestQuestDatabase,
|
||||||
|
definition_id: int,
|
||||||
|
*,
|
||||||
|
today: datetime.date | None = None,
|
||||||
|
horizon_days: int | None = None,
|
||||||
|
) -> DeletedQuestDefinition:
|
||||||
|
"""Delete a definition if it has no history; otherwise retire it.
|
||||||
|
|
||||||
|
Owner decision (2026-09-26): "delete if there's no history,
|
||||||
|
retire/deactivate if there is". When NO instance of the definition
|
||||||
|
has a completion event, the definition, its instances, assignees,
|
||||||
|
windows and (if no other definition references it) its schedule
|
||||||
|
rule are hard-deleted in ONE transaction by
|
||||||
|
:meth:`~.dao_rules.QuestDefinitionsDao.delete_if_no_history` — the
|
||||||
|
history check is inside that transaction, so a completion recorded
|
||||||
|
concurrently can never be deleted (D-005). When history exists,
|
||||||
|
the definition is retired through :func:`set_quest_definition_active`
|
||||||
|
(``False``). Retirement preserves the definition row, schedule
|
||||||
|
rule, assignees, windows and ALL durable completion history. Like
|
||||||
|
any deactivation it regenerates, so future UNCOMPLETED instances are
|
||||||
|
cleared (they are rolling, regenerable rows) and none are recreated
|
||||||
|
while the definition is inactive — the retired task stops generating
|
||||||
|
and no longer shows open work. Durable instance history lives only
|
||||||
|
in the append-only ``completion_events``.
|
||||||
|
|
||||||
|
``definition_id`` must be a plain int (bools and floats rejected).
|
||||||
|
Raises ValueError naming the field when the id is malformed or the
|
||||||
|
definition does not exist. ``today`` / ``horizon_days`` are threaded
|
||||||
|
into the retire path's regeneration, as in
|
||||||
|
:func:`set_quest_definition_active`.
|
||||||
|
"""
|
||||||
|
_validate_definition_id(definition_id)
|
||||||
|
deleted = await QuestDefinitionsDao(database).delete_if_no_history(
|
||||||
|
definition_id
|
||||||
|
)
|
||||||
|
if deleted is None:
|
||||||
|
raise ValueError(
|
||||||
|
f"definition_id: quest definition {definition_id} does not exist"
|
||||||
|
)
|
||||||
|
if deleted:
|
||||||
|
return DeletedQuestDefinition(
|
||||||
|
definition_id=definition_id,
|
||||||
|
deleted=True,
|
||||||
|
retired=False,
|
||||||
|
definition=None,
|
||||||
|
)
|
||||||
|
retired = await set_quest_definition_active(
|
||||||
|
database,
|
||||||
|
definition_id,
|
||||||
|
False,
|
||||||
|
today=today,
|
||||||
|
horizon_days=horizon_days,
|
||||||
|
)
|
||||||
|
return DeletedQuestDefinition(
|
||||||
|
definition_id=definition_id,
|
||||||
|
deleted=False,
|
||||||
|
retired=True,
|
||||||
|
definition=retired,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _bundle_snapshot(
|
def _bundle_snapshot(
|
||||||
snapshot: QuestDefinitionSnapshot,
|
snapshot: QuestDefinitionSnapshot,
|
||||||
) -> CreatedQuestDefinition:
|
) -> CreatedQuestDefinition:
|
||||||
|
|||||||
@@ -15,5 +15,5 @@
|
|||||||
"iot_class": "local_polling",
|
"iot_class": "local_polling",
|
||||||
"issue_tracker": "https://github.com/Maniacs-Mansion/NestQuest/issues",
|
"issue_tracker": "https://github.com/Maniacs-Mansion/NestQuest/issues",
|
||||||
"requirements": [],
|
"requirements": [],
|
||||||
"version": "0.9.0"
|
"version": "0.9.2"
|
||||||
}
|
}
|
||||||
|
|||||||
+18
-1
@@ -64,7 +64,8 @@ Header `Tasks` / `7 definitions · 3 children` + gradient `New` button. Filter c
|
|||||||
(All / one per child): active `#18181D` fill white text, inactive white with border.
|
(All / one per child): active `#18181D` fill white text, inactive white with border.
|
||||||
Rows: glyph 17px · title 13.5px/700 · meta 11px/600 `<assignees> · <recurrence> · <windows>`
|
Rows: glyph 17px · title 13.5px/700 · meta 11px/600 `<assignees> · <recurrence> · <windows>`
|
||||||
· `chevron-right`. An inactive definition renders the whole row at
|
· `chevron-right`. An inactive definition renders the whole row at
|
||||||
`--nq-a-ink-tertiary` with `· inactive` in the meta — never hard-deleted (glossary).
|
`--nq-a-ink-tertiary` with `· inactive` in the meta. A definition with completion
|
||||||
|
history is never hard-deleted (glossary, D-017): Delete retires it to this inactive state.
|
||||||
The row being edited takes `--nq-a-selected` with brand-blue glyph, `Editing` meta and
|
The row being edited takes `--nq-a-selected` with brand-blue glyph, `Editing` meta and
|
||||||
`chevron-down`.
|
`chevron-down`.
|
||||||
|
|
||||||
@@ -83,6 +84,22 @@ Bottom sheet, `--nq-a-radius-sheet`, `--nq-a-sheet-shadow`, 38×4px grabber, pad
|
|||||||
Footer: `Cancel` (flex 1, bordered) + `Save changes` (flex 2, gradient), both 13px vertical
|
Footer: `Cancel` (flex 1, bordered) + `Save changes` (flex 2, gradient), both 13px vertical
|
||||||
padding. Sheet enters over the dimmed list in `--nq-dur-modal`.
|
padding. Sheet enters over the dimmed list in `--nq-dur-modal`.
|
||||||
|
|
||||||
|
Delete (editing an existing definition only, never on `New`): a full-width bordered
|
||||||
|
`Delete task` button below the footer, danger-ink text. Tapping it swaps the footer for a
|
||||||
|
danger-tinted confirm — "Delete this task? If it has no completion history it is deleted;
|
||||||
|
otherwise it is retired (deactivated) and its history is kept." — with `Go back` and a
|
||||||
|
danger-filled `Delete task`. Confirming calls `DELETE /api/v1/admin/quest-definitions/{id}`
|
||||||
|
(admin plane only; the kids' panel has no equivalent control). Hybrid outcome (D-017): no
|
||||||
|
completion event on any instance → the definition, its instances, assignees, windows and
|
||||||
|
unreferenced rule are hard-deleted and the row leaves the list; otherwise the definition
|
||||||
|
is retired (deactivated) and the row stays, shown inactive. Retirement preserves the
|
||||||
|
definition row, schedule rule, assignees, windows and all durable completion history. Like
|
||||||
|
any deactivation it regenerates, so future uncompleted instances are cleared (they are
|
||||||
|
rolling, regenerable rows) and none are recreated while the definition is inactive — the
|
||||||
|
retired task stops generating and no longer shows open work. Durable instance history lives
|
||||||
|
only in the append-only `completion_events`. An error keeps the sheet open with the API's
|
||||||
|
detail.
|
||||||
|
|
||||||
### 3.3 Icon picker
|
### 3.3 Icon picker
|
||||||
Each definition carries one optional icon, shown on both surfaces. The picker offers three
|
Each definition carries one optional icon, shown on both surfaces. The picker offers three
|
||||||
sources:
|
sources:
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[project]
|
[project]
|
||||||
name = "nestquest"
|
name = "nestquest"
|
||||||
version = "0.9.0"
|
version = "0.9.2"
|
||||||
description = "NestQuest Home Assistant Integration"
|
description = "NestQuest Home Assistant Integration"
|
||||||
readme = "README.md"
|
readme = "README.md"
|
||||||
requires-python = ">=3.11"
|
requires-python = ">=3.11"
|
||||||
|
|||||||
@@ -817,3 +817,141 @@ async def test_set_active_malformed_body_is_422(
|
|||||||
headers=_admin_headers(),
|
headers=_admin_headers(),
|
||||||
)
|
)
|
||||||
assert response.status_code == 422
|
assert response.status_code == 422
|
||||||
|
|
||||||
|
|
||||||
|
# --- delete: hybrid (delete if no history, retire if there is) --------------
|
||||||
|
|
||||||
|
_dao_instances = importlib.import_module("nestquest_core.dao_instances")
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_route_lives_on_the_admin_router() -> None:
|
||||||
|
"""The DELETE route is on the admin router, which carries the ONE
|
||||||
|
require_admin dependency — no second auth check is added."""
|
||||||
|
from api.auth import require_admin
|
||||||
|
from api.routes_admin import router as admin_router
|
||||||
|
|
||||||
|
assert require_admin in [d.dependency for d in admin_router.dependencies]
|
||||||
|
matches = [
|
||||||
|
route
|
||||||
|
for route in admin_router.routes
|
||||||
|
if route.path == "/api/v1/admin/quest-definitions/{definition_id}"
|
||||||
|
and "DELETE" in route.methods
|
||||||
|
]
|
||||||
|
assert len(matches) == 1
|
||||||
|
|
||||||
|
|
||||||
|
async def test_delete_refuses_non_admin_and_absent(
|
||||||
|
admin_questdefs: SimpleNamespace,
|
||||||
|
) -> None:
|
||||||
|
"""A non-admin JWT is 403 and an absent credential 401: nothing is
|
||||||
|
deleted or retired."""
|
||||||
|
client = admin_questdefs.client
|
||||||
|
ada = (await _create_children(client, ("Ada",)))[0]
|
||||||
|
definition_id = (await _create_definition(client, [ada])).json()["id"]
|
||||||
|
non_admin = make_token(groups=("some-other-group",))
|
||||||
|
|
||||||
|
forbidden = await client.delete(
|
||||||
|
f"/api/v1/admin/quest-definitions/{definition_id}",
|
||||||
|
headers={"Authorization": f"Bearer {non_admin}"},
|
||||||
|
)
|
||||||
|
assert forbidden.status_code == 403
|
||||||
|
unauthenticated = await client.delete(
|
||||||
|
f"/api/v1/admin/quest-definitions/{definition_id}"
|
||||||
|
)
|
||||||
|
assert unauthenticated.status_code == 401
|
||||||
|
|
||||||
|
record = await _dao_rules.QuestDefinitionsDao(
|
||||||
|
admin_questdefs.database
|
||||||
|
).get(definition_id)
|
||||||
|
assert record is not None
|
||||||
|
assert record.is_active is True
|
||||||
|
|
||||||
|
|
||||||
|
async def test_delete_without_history_hard_deletes(
|
||||||
|
admin_questdefs: SimpleNamespace,
|
||||||
|
) -> None:
|
||||||
|
"""No completion history: 200 ``deleted`` and the row and its
|
||||||
|
unreferenced rule are gone."""
|
||||||
|
client = admin_questdefs.client
|
||||||
|
ada = (await _create_children(client, ("Ada",)))[0]
|
||||||
|
definition_id = (await _create_definition(client, [ada])).json()["id"]
|
||||||
|
dao = _dao_rules.QuestDefinitionsDao(admin_questdefs.database)
|
||||||
|
rule_id = (await dao.get(definition_id)).schedule_rule_id
|
||||||
|
|
||||||
|
response = await client.delete(
|
||||||
|
f"/api/v1/admin/quest-definitions/{definition_id}",
|
||||||
|
headers=_admin_headers(),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json() == {
|
||||||
|
"definition_id": definition_id,
|
||||||
|
"deleted": True,
|
||||||
|
"retired": False,
|
||||||
|
"definition": None,
|
||||||
|
}
|
||||||
|
assert await dao.get(definition_id) is None
|
||||||
|
assert await _dao_rules.ScheduleRulesDao(
|
||||||
|
admin_questdefs.database
|
||||||
|
).get(rule_id) is None
|
||||||
|
|
||||||
|
|
||||||
|
async def test_delete_with_history_retires(
|
||||||
|
admin_questdefs: SimpleNamespace,
|
||||||
|
) -> None:
|
||||||
|
"""Completion history present: 200 ``retired`` with the now-inactive
|
||||||
|
definition; the row, instance and history all remain."""
|
||||||
|
client = admin_questdefs.client
|
||||||
|
database = admin_questdefs.database
|
||||||
|
ada = (await _create_children(client, ("Ada",)))[0]
|
||||||
|
definition_id = (await _create_definition(client, [ada])).json()["id"]
|
||||||
|
instance = await _dao_instances.QuestInstancesDao(database).upsert(
|
||||||
|
definition_id,
|
||||||
|
ada,
|
||||||
|
datetime.date.today().isoformat(),
|
||||||
|
"2026-09-14T12:00:00+00:00",
|
||||||
|
window="morning",
|
||||||
|
)
|
||||||
|
events = _dao_instances.CompletionEventsDao(database)
|
||||||
|
event = await events.append(
|
||||||
|
instance.id,
|
||||||
|
ada,
|
||||||
|
"completed",
|
||||||
|
"user",
|
||||||
|
"2026-09-14T12:00:00+00:00",
|
||||||
|
True,
|
||||||
|
actor_user_id="admin-1",
|
||||||
|
)
|
||||||
|
|
||||||
|
response = await client.delete(
|
||||||
|
f"/api/v1/admin/quest-definitions/{definition_id}",
|
||||||
|
headers=_admin_headers(),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
payload = response.json()
|
||||||
|
assert payload["definition_id"] == definition_id
|
||||||
|
assert payload["deleted"] is False
|
||||||
|
assert payload["retired"] is True
|
||||||
|
assert payload["definition"]["id"] == definition_id
|
||||||
|
assert payload["definition"]["is_active"] is False
|
||||||
|
assert payload["definition"]["title"] == "Tidy the den"
|
||||||
|
record = await _dao_rules.QuestDefinitionsDao(database).get(definition_id)
|
||||||
|
assert record is not None
|
||||||
|
assert record.is_active is False
|
||||||
|
assert await _dao_instances.QuestInstancesDao(database).get_by_id(
|
||||||
|
instance.id
|
||||||
|
) == instance
|
||||||
|
assert await events.list_by_instance(instance.id) == [event]
|
||||||
|
|
||||||
|
|
||||||
|
async def test_delete_unknown_definition_is_404(
|
||||||
|
admin_questdefs: SimpleNamespace,
|
||||||
|
) -> None:
|
||||||
|
"""A DELETE naming a definition that does not exist is 404."""
|
||||||
|
response = await admin_questdefs.client.delete(
|
||||||
|
"/api/v1/admin/quest-definitions/999",
|
||||||
|
headers=_admin_headers(),
|
||||||
|
)
|
||||||
|
assert response.status_code == 404
|
||||||
|
assert response.json()["detail"] == "Quest definition not found"
|
||||||
|
|||||||
@@ -996,8 +996,9 @@ def test_definition_set_active_missing_returns_zero(tmp_path) -> None:
|
|||||||
assert _with_db(tmp_path, "definition-set-active-missing.db")(_body) == 0
|
assert _with_db(tmp_path, "definition-set-active-missing.db")(_body) == 0
|
||||||
|
|
||||||
|
|
||||||
def test_definition_no_delete_method_exists(tmp_path) -> None:
|
def test_definition_only_delete_method_is_history_guarded(tmp_path) -> None:
|
||||||
"""Guardrail: definitions are deactivated, never hard-deleted."""
|
"""Guardrail: the ONLY definition delete path is the history-guarded
|
||||||
|
one (owner decision 2026-09-26); there is no unconditional delete."""
|
||||||
async def _body(database, rules, definitions, children, child):
|
async def _body(database, rules, definitions, children, child):
|
||||||
import inspect
|
import inspect
|
||||||
|
|
||||||
@@ -1008,7 +1009,9 @@ def test_definition_no_delete_method_exists(tmp_path) -> None:
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
assert "delete" not in methods
|
assert "delete" not in methods
|
||||||
assert not any(name.startswith("delete") for name in methods)
|
assert {
|
||||||
|
name for name in methods if name.startswith("delete")
|
||||||
|
} == {"delete_if_no_history"}
|
||||||
return methods
|
return methods
|
||||||
|
|
||||||
_with_db(tmp_path, "definition-no-delete.db")(_body)
|
_with_db(tmp_path, "definition-no-delete.db")(_body)
|
||||||
|
|||||||
@@ -22,8 +22,10 @@ from custom_components.nestquest.core.db import NestQuestDatabase
|
|||||||
from custom_components.nestquest.core.migrations import apply_migrations
|
from custom_components.nestquest.core.migrations import apply_migrations
|
||||||
from custom_components.nestquest.core.quest_definitions import (
|
from custom_components.nestquest.core.quest_definitions import (
|
||||||
CreatedQuestDefinition,
|
CreatedQuestDefinition,
|
||||||
|
DeletedQuestDefinition,
|
||||||
assign_child,
|
assign_child,
|
||||||
create_quest_definition,
|
create_quest_definition,
|
||||||
|
delete_quest_definition,
|
||||||
edit_quest_definition,
|
edit_quest_definition,
|
||||||
list_active_definitions,
|
list_active_definitions,
|
||||||
list_definitions_firing_on,
|
list_definitions_firing_on,
|
||||||
@@ -2126,3 +2128,188 @@ def test_wrappers_forward_horizon_days(tmp_path, mode) -> None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
_with_db(tmp_path, f"wrapper-horizon-{mode}.db")(_body)
|
_with_db(tmp_path, f"wrapper-horizon-{mode}.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# delete_quest_definition: hybrid delete (owner decision 2026-09-26)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
async def _count(database, sql, definition_id) -> int:
|
||||||
|
row = await database.fetch_one(sql, (definition_id,))
|
||||||
|
return row[0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_without_history_hard_deletes_everything(tmp_path) -> None:
|
||||||
|
"""No completion event on any instance: the definition, its
|
||||||
|
instances, assignees, windows and unreferenced rule are all gone."""
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database,
|
||||||
|
"Brush teeth",
|
||||||
|
_daily_rule(),
|
||||||
|
[child.id],
|
||||||
|
["morning", "evening"],
|
||||||
|
)
|
||||||
|
definition_id = created.definition.id
|
||||||
|
rule_id = created.definition.schedule_rule_id
|
||||||
|
instances_sql = (
|
||||||
|
"SELECT COUNT(*) FROM quest_instances WHERE definition_id = ?"
|
||||||
|
)
|
||||||
|
# Create materialized future instances, so the delete has
|
||||||
|
# instance rows to remove (not just an empty table).
|
||||||
|
assert await _count(database, instances_sql, definition_id) > 0
|
||||||
|
|
||||||
|
result = await delete_quest_definition(database, definition_id)
|
||||||
|
|
||||||
|
assert result == DeletedQuestDefinition(
|
||||||
|
definition_id=definition_id,
|
||||||
|
deleted=True,
|
||||||
|
retired=False,
|
||||||
|
definition=None,
|
||||||
|
)
|
||||||
|
assert await definitions.get(definition_id) is None
|
||||||
|
assert await rules.get(rule_id) is None
|
||||||
|
assert await _count(database, instances_sql, definition_id) == 0
|
||||||
|
for table in (
|
||||||
|
"quest_definition_assignees",
|
||||||
|
"quest_definition_windows",
|
||||||
|
):
|
||||||
|
assert await _count(
|
||||||
|
database,
|
||||||
|
f"SELECT COUNT(*) FROM {table} WHERE definition_id = ?",
|
||||||
|
definition_id,
|
||||||
|
) == 0
|
||||||
|
# The child itself is untouched.
|
||||||
|
assert (await children.get(child.id)) is not None
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "delete-no-history.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_without_history_keeps_a_shared_rule(tmp_path) -> None:
|
||||||
|
"""The rule is deleted only when no other definition references it."""
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
rule = await rules.create("daily", "2026-09-14")
|
||||||
|
first = await definitions.create(
|
||||||
|
"First", rule.id, NOW, assignee_child_ids=[child.id]
|
||||||
|
)
|
||||||
|
second = await definitions.create(
|
||||||
|
"Second", rule.id, NOW, assignee_child_ids=[child.id]
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await delete_quest_definition(database, first.id)
|
||||||
|
|
||||||
|
assert result.deleted is True
|
||||||
|
assert await definitions.get(first.id) is None
|
||||||
|
assert await definitions.get(second.id) is not None
|
||||||
|
assert await rules.get(rule.id) is not None
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "delete-shared-rule.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_with_history_retires_and_preserves_everything(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
"""A completion event on ANY instance: the definition is retired
|
||||||
|
(deactivated) and its row, rule, assignees, windows and completion
|
||||||
|
history all remain. The completed instance survives; only future
|
||||||
|
UNCOMPLETED instances are cleared by the deactivation's regeneration."""
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
definition_id = created.definition.id
|
||||||
|
rule_id = created.definition.schedule_rule_id
|
||||||
|
instances = QuestInstancesDao(database)
|
||||||
|
events = CompletionEventsDao(database)
|
||||||
|
instance = await instances.upsert(
|
||||||
|
definition_id, child.id, _today_iso(), NOW, window="morning"
|
||||||
|
)
|
||||||
|
event = await events.append(
|
||||||
|
instance.id,
|
||||||
|
child.id,
|
||||||
|
"completed",
|
||||||
|
"user",
|
||||||
|
NOW,
|
||||||
|
True,
|
||||||
|
actor_user_id="admin-1",
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await delete_quest_definition(database, definition_id)
|
||||||
|
|
||||||
|
assert result.deleted is False
|
||||||
|
assert result.retired is True
|
||||||
|
assert result.definition_id == definition_id
|
||||||
|
assert isinstance(result.definition, CreatedQuestDefinition)
|
||||||
|
assert result.definition.definition.is_active is False
|
||||||
|
stored = await definitions.get(definition_id)
|
||||||
|
assert stored is not None
|
||||||
|
assert stored.is_active is False
|
||||||
|
assert await rules.get(rule_id) is not None
|
||||||
|
assert [c.id for c in await definitions.list_assignees(
|
||||||
|
definition_id
|
||||||
|
)] == [child.id]
|
||||||
|
assert [w.window for w in await definitions.list_windows(
|
||||||
|
definition_id
|
||||||
|
)] == ["morning"]
|
||||||
|
assert await instances.get_by_id(instance.id) == instance
|
||||||
|
assert await events.list_by_instance(instance.id) == [event]
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "delete-with-history.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_with_uncompleted_history_still_retires(tmp_path) -> None:
|
||||||
|
"""ANY completion event counts as history — even when the latest
|
||||||
|
event is an un-completion, the definition is retired, not deleted."""
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
definition_id = created.definition.id
|
||||||
|
instances = QuestInstancesDao(database)
|
||||||
|
events = CompletionEventsDao(database)
|
||||||
|
instance = await instances.upsert(
|
||||||
|
definition_id, child.id, _today_iso(), NOW, window="morning"
|
||||||
|
)
|
||||||
|
await events.append(
|
||||||
|
instance.id, child.id, "completed", "user", NOW, True,
|
||||||
|
actor_user_id="admin-1",
|
||||||
|
)
|
||||||
|
await events.append(
|
||||||
|
instance.id, child.id, "uncompleted", "user", NOW, None,
|
||||||
|
actor_user_id="admin-1",
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await delete_quest_definition(database, definition_id)
|
||||||
|
|
||||||
|
assert result.retired is True
|
||||||
|
assert await instances.get_by_id(instance.id) == instance
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "delete-uncompleted-history.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_nonexistent_definition_names_field(tmp_path) -> None:
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
with pytest.raises(ValueError, match=r"^definition_id:"):
|
||||||
|
await delete_quest_definition(database, 999)
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "delete-missing.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad", [True, 1.0, "1", None])
|
||||||
|
def test_delete_rejects_non_int_definition_id(tmp_path, bad) -> None:
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
with pytest.raises(ValueError, match="definition_id"):
|
||||||
|
await delete_quest_definition(database, bad)
|
||||||
|
assert await definitions.get(created.definition.id) is not None
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "delete-bad-id.db")(_body)
|
||||||
|
|||||||
Reference in New Issue
Block a user