Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6cb62b0260 | ||
|
|
ba0b2032cc | ||
|
|
ed230961e0 |
@@ -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 {
|
||||||
|
|||||||
@@ -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>
|
||||||
|
|||||||
+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:
|
||||||
|
|||||||
+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:
|
||||||
|
|||||||
@@ -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