From 7a26845abedcc2ec241a1235376b999777de62ee Mon Sep 17 00:00:00 2001 From: Joshua Date: Wed, 23 Sep 2026 01:44:50 -0400 Subject: [PATCH] Admin routes: quest definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin plane gains quest-definition routes on the existing admin router, mirroring the children routes' thin-adapter pattern: the body model carries the shape, the handler builds ONE ScheduleRule from the request's rule object through the recurrence model's own from_dict (rule validation stays the model's construction-time job), and every call goes to ONE nestquest_core.quest_definitions business function — the title/window/assignee policies, the whole-set window replacement and the never-hard-delete policy all live in the core, never in a handler. - POST /quest-definitions: title + rule + assignees + windows (+ optional description/icon), persisted atomically by the core. - PATCH /quest-definitions/{id}: only supplied fields are forwarded (exclude_unset); an explicit null description/icon clears it, a null rule/windows is rejected; assignment stays unsettable here. - PATCH /quest-definitions/{id}/active: the only removal path — the flag flips, the row, rule, assignees and windows survive. Error mapping stays deliberately narrow: the core's definition_id:- prefixed not-exist error is 404; every other ValueError (including RuleValidationError) is 422 with the core's message, so a rejected rule, window, assignee or title from the BODY never disguises itself as a missing resource and an unexpected error is never swallowed as a 4xx. Task: b586a2f1-1532-4194-a19a-8f8386d22590 (Feature 16). --- api/nestquest_core.py | 16 + api/routes_admin.py | 356 +++++++++++++++++- tests/test_api_admin_questdef.py | 611 +++++++++++++++++++++++++++++++ 3 files changed, 975 insertions(+), 8 deletions(-) create mode 100644 tests/test_api_admin_questdef.py diff --git a/api/nestquest_core.py b/api/nestquest_core.py index 45e795a..1ce326e 100644 --- a/api/nestquest_core.py +++ b/api/nestquest_core.py @@ -128,6 +128,20 @@ core_settings = importlib.import_module(f"{_CORE_MODULE_NAME}.settings") #: the admin plane's children routes call. core_children = importlib.import_module(f"{_CORE_MODULE_NAME}.children") +#: The quest-definition business layer (``core.quest_definitions`` — +#: ``create_quest_definition``, ``edit_quest_definition``, +#: ``set_quest_definition_active``) the admin plane's quest-definition +#: routes call. +core_quest_definitions = importlib.import_module( + f"{_CORE_MODULE_NAME}.quest_definitions" +) + +#: The recurrence model (``core.recurrence.ScheduleRule`` and +#: ``RuleValidationError``) the quest-definition routes build request +#: rules with — the model validates at construction, the routes never +#: re-implement rule validation. +core_recurrence = importlib.import_module(f"{_CORE_MODULE_NAME}.recurrence") + __all__ = [ "core", "core_children", @@ -135,6 +149,8 @@ __all__ = [ "core_db", "core_events", "core_migrations", + "core_quest_definitions", + "core_recurrence", "core_snapshot", "core_settings", ] diff --git a/api/routes_admin.py b/api/routes_admin.py index ff88551..3ed4a71 100644 --- a/api/routes_admin.py +++ b/api/routes_admin.py @@ -18,20 +18,47 @@ check all live THERE, never in a handler — and serializes the returned payload (``id``, ``display_name``, ``colour``, ``avatar_ref``, ``sort_order``, ``is_active``). -Error mapping (every children route, deliberately narrow): +The quest-definition routes follow the same pattern over +:mod:`nestquest_core.quest_definitions`: the body model carries the +shape, the handler builds ONE +:class:`~nestquest_core.recurrence.ScheduleRule` from the request's +``rule`` object through the model's own ``from_dict`` (rule validation +is the MODEL's construction-time job — a bad combination raises +:class:`~nestquest_core.recurrence.RuleValidationError`, never +hand-rolled here), and the call goes to +``create_quest_definition`` / ``edit_quest_definition`` / +``set_quest_definition_active``. The title policy, the window-name +and due-time policy, the assignee checks, the whole-set window +replacement and the never-hard-delete policy (deactivation is the only +removal path) all live in the core. Assignment is NOT settable +through the API: the core's edit path takes no assignees. The edit +route forwards only SUPPLIED fields (``exclude_unset``), like the +children edit. + +Error mapping (every children and quest-definition route, deliberately +narrow): - Malformed input the body model or the path parser rejects (a missing - or wrong-typed field, a non-integer child id) is FastAPI's 422 before - any handler code runs. + or wrong-typed field, a non-integer child or definition id) is + FastAPI's 422 before any handler code runs. - A core ``ValueError`` reporting a NON-EXISTENT child (edit or set_active on an unknown id) is mapped to 404. +- A core ``ValueError`` reporting a NON-EXISTENT DEFINITION (edit or + set_active on an unknown id — the core prefixes that error with + ``definition_id:``) is mapped to 404. Create's rejected-assignee + error names a child from the BODY, not a path id, so it maps to 422. - Every OTHER core ``ValueError`` — an empty display name, an explicit ``null`` colour/avatar_ref (clearing is not supported), a reorder list that is not a complete permutation of the children (missing, - duplicate, or unknown ids) — is mapped to 422: the body was - well-formed JSON but semantically invalid, the same failure class - Pydantic reports. The core rejects these BEFORE any write, so a 422 - never leaves a half-applied change behind. + duplicate, or unknown ids); a rejected rule + (:class:`~nestquest_core.recurrence.RuleValidationError`: unknown + ``rule_type``, a shape-missing field such as a weekly rule without + weekdays, a malformed date, a forbidden field), a bad window name or + due time, a rejected window or assignee list (empty, duplicate, + wrong-typed) — is mapped to 422: the body was well-formed JSON but + semantically invalid, the same failure class Pydantic reports. The + core rejects these BEFORE any write, so a 422 never leaves a + half-applied change behind. PATCH edit semantics: the body model's fields are optional and only SUPPLIED fields are forwarded (``model_dump(exclude_unset=True)``), so @@ -49,7 +76,11 @@ from pydantic import BaseModel from api.auth import require_admin from api.database import DatabaseState -from api.nestquest_core import core_children +from api.nestquest_core import ( + core_children, + core_quest_definitions, + core_recurrence, +) #: All admin-plane routes share this router; the ONE admin dependency #: (service-token refusal + JWT verification + nestquest-admins group @@ -91,6 +122,60 @@ class AdminChildListResponse(BaseModel): children: list[AdminChildResponse] +class AdminRuleResponse(BaseModel): + """A definition's schedule rule as the admin plane serializes it. + + Mirrors :meth:`nestquest_core.recurrence.ScheduleRule.to_dict`: the + model-name ``rule_type`` (e.g. ``monthly_weekday``), the weekday + set as a sorted list, and ``None`` for every field the rule's shape + does not use. + """ + + rule_type: str + interval: int + weekday_set: list[int] | None + day_of_month: int | None + nth_weekday: int | None + nth_weekday_weekday: int | None + month: int | None + start_date: str + end_date: str | None + + +class AdminAssigneeResponse(BaseModel): + """One assigned child: the id and display name the PWA renders.""" + + id: int + display_name: str + + +class AdminDefinitionWindowResponse(BaseModel): + """One window declaration: its name and optional HH:MM due time.""" + + window: str + due_time: str | None + + +class AdminQuestDefinitionResponse(BaseModel): + """One quest definition as the admin plane serializes it. + + Mirrors the core's + :class:`~nestquest_core.quest_definitions.CreatedQuestDefinition` + bundle: the stored row (minus the internal ``schedule_rule_id``/ + ``due_time``/``created_at`` columns) plus its decoded rule, + assignees and windows. + """ + + id: int + title: str + description: str | None + icon: str | None + is_active: bool + rule: AdminRuleResponse + assignees: list[AdminAssigneeResponse] + windows: list[AdminDefinitionWindowResponse] + + class AdminChildCreateRequest(BaseModel): """The body of ``POST /api/v1/admin/children``. @@ -138,6 +223,77 @@ class AdminChildReorderRequest(BaseModel): ordered_ids: list[int] +class AdminRuleRequest(BaseModel): + """The ``rule`` object of a quest-definition create or edit body. + + The fields of :class:`~nestquest_core.recurrence.ScheduleRule` — + nothing more: the shape is the model's, the defaults are the + model's (``interval`` 1, ``start_date`` 1970-01-01). Only + ``rule_type`` is required. The COMBINATION policy (which fields a + shape requires and forbids, the 1..31/1..12/0..6 ranges, the strict + ISO dates) is enforced by the model's constructor, which the route + builds through ``from_dict`` — this model only guards the SHAPE. + """ + + rule_type: str + interval: int = 1 + weekday_set: list[int] | None = None + day_of_month: int | None = None + nth_weekday: int | None = None + nth_weekday_weekday: int | None = None + month: int | None = None + start_date: str = "1970-01-01" + end_date: str | None = None + + +class AdminQuestDefinitionCreateRequest(BaseModel): + """The body of ``POST /api/v1/admin/quest-definitions``. + + ``title``, ``rule``, ``assignee_child_ids`` and ``windows`` are + required (the core rejects an empty title, a non-list or empty + assignee/window list, and unknown window names); ``description`` + and ``icon`` are optional and default to NULL through the core. + ``windows`` entries are a window name (``morning``) or a two-item + ``[window, due_time]`` pair whose due time is a strict 24-hour + HH:MM (or ``null`` for none) — both forms the core normalizes. + """ + + title: str + rule: AdminRuleRequest + assignee_child_ids: list[int] + windows: list[str | tuple[str, str | None]] + description: str | None = None + icon: str | None = None + + +class AdminQuestDefinitionEditRequest(BaseModel): + """The body of ``PATCH /api/v1/admin/quest-definitions/{definition_id}``. + + Every field is optional; ONLY the fields the request supplies are + forwarded to :func:`nestquest_core.quest_definitions.edit_quest_definition` + (the route dumps the model with ``exclude_unset``), so an omitted + field stays unchanged. An explicit ``null`` ``description``/ + ``icon`` IS passed and CLEARS the stored value (the core supports + clearing here, unlike the children edit); an explicit ``null`` + ``rule`` or ``windows`` is passed and rejected by the core. A + supplied ``windows`` list REPLACES the whole window set, and + ``assignees`` are deliberately absent — assignment is not settable + through this route. + """ + + title: str | None = None + description: str | None = None + icon: str | None = None + rule: AdminRuleRequest | None = None + windows: list[str | tuple[str, str | None]] | None = None + + +class AdminQuestDefinitionActiveRequest(BaseModel): + """The body of ``PATCH /api/v1/admin/quest-definitions/{id}/active``.""" + + is_active: bool + + #: 404 detail for a child id the core layer reports as non-existent. _CHILD_NOT_FOUND_DETAIL = "Child not found" @@ -172,6 +328,76 @@ def _child_response(record: core_children.ChildRecord) -> AdminChildResponse: ) +#: 404 detail for a definition id the core layer reports as non-existent. +_QUEST_DEFINITION_NOT_FOUND_DETAIL = "Quest definition not found" + + +def _raise_quest_definition_error(error: ValueError) -> NoReturn: + """Map a core quest-definitions ``ValueError`` onto 404 or 422. + + The mapping mirrors :func:`_raise_child_error` and stays narrow: + the core names a missing definition by prefixing its error with + ``definition_id:`` (edit or set_active on an unknown id — its id + validator's message reads ``definition_id must be an integer`` + WITHOUT the colon, so a malformed id stays 422), and that one case + is the quest-definition routes' 404. Every other ``ValueError`` — + including :class:`~nestquest_core.recurrence.RuleValidationError`, + which subclasses it — becomes 422 with the core's message as the + detail: create's rejected assignee names a child from the BODY (not + a path id, so not 404), a rejected rule/window/title names the + field and what to fix, and none of it leaks more than the rule it + enforces. + """ + if str(error).startswith("definition_id:"): + raise HTTPException( + status_code=404, detail=_QUEST_DEFINITION_NOT_FOUND_DETAIL + ) from error + raise HTTPException(status_code=422, detail=str(error)) from error + + +def _rule_from_request(rule: AdminRuleRequest) -> core_recurrence.ScheduleRule: + """Build the core ScheduleRule from the request's ``rule`` object. + + The recurrence model validates AT CONSTRUCTION — through its own + ``from_dict`` here, which raises + :class:`~nestquest_core.recurrence.RuleValidationError` (a + ``ValueError``) for any bad combination — so the route re-implements + no rule policy of its own. + """ + return core_recurrence.ScheduleRule.from_dict( + rule.model_dump(exclude_unset=True) + ) + + +def _rule_response(rule: core_recurrence.ScheduleRule) -> AdminRuleResponse: + """Serialize one core ScheduleRule via its lossless dict form.""" + return AdminRuleResponse(**rule.to_dict()) + + +def _quest_definition_response( + bundle: core_quest_definitions.CreatedQuestDefinition, +) -> AdminQuestDefinitionResponse: + """Serialize one core CreatedQuestDefinition into the payload.""" + return AdminQuestDefinitionResponse( + id=bundle.definition.id, + title=bundle.definition.title, + description=bundle.definition.description, + icon=bundle.definition.icon, + is_active=bundle.definition.is_active, + rule=_rule_response(bundle.rule), + assignees=[ + AdminAssigneeResponse(id=child.id, display_name=child.display_name) + for child in bundle.assignees + ], + windows=[ + AdminDefinitionWindowResponse( + window=window.window, due_time=window.due_time + ) + for window in bundle.windows + ], + ) + + @router.get("/ping", summary="Admin plane probe") async def admin_ping( claims: Annotated[dict[str, object], Depends(require_admin)], @@ -341,4 +567,118 @@ async def admin_reorder_children( return {"status": "ok"} +@router.post( + "/quest-definitions", + summary="Create a quest definition", + status_code=201, + response_model=AdminQuestDefinitionResponse, +) +async def admin_create_quest_definition( + body: AdminQuestDefinitionCreateRequest, request: Request +) -> AdminQuestDefinitionResponse: + """Create a quest definition (rule + assignees + windows) and return it. + + 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 body model + carries the shape, the request's ``rule`` becomes ONE + :class:`~nestquest_core.recurrence.ScheduleRule` (validated by the + model's constructor, :func:`_rule_from_request`), and the title + policy, the assignee and window policies and the atomic + rule+definition+assignees+windows persistence all live in + :func:`nestquest_core.quest_definitions.create_quest_definition`. + Create names no existing definition, so a missing definition can + never be its error: every rejection here is 422 + (:func:`_raise_quest_definition_error`). + """ + state: DatabaseState = request.app.state.db + try: + bundle = await core_quest_definitions.create_quest_definition( + state.database, + body.title, + _rule_from_request(body.rule), + body.assignee_child_ids, + body.windows, + description=body.description, + icon=body.icon, + ) + except ValueError as error: + _raise_quest_definition_error(error) + return _quest_definition_response(bundle) + + +@router.patch( + "/quest-definitions/{definition_id}", + summary="Edit a quest definition", + response_model=AdminQuestDefinitionResponse, +) +async def admin_edit_quest_definition( + definition_id: int, + body: AdminQuestDefinitionEditRequest, + request: Request, +) -> AdminQuestDefinitionResponse: + """Edit a quest definition; only supplied fields change. + + 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 supplied + fields are forwarded with ``exclude_unset`` semantics so an omitted + field is never passed to + :func:`nestquest_core.quest_definitions.edit_quest_definition` and + stays unchanged; a supplied ``windows`` list replaces the WHOLE + window set in the core, and a supplied ``rule`` is rebuilt into a + ScheduleRule (:func:`_rule_from_request`, validated by the model). + An explicit ``null`` ``rule``/``windows`` reaches the core as None + and is rejected there (422), while an explicit ``null`` + ``description``/``icon`` clears the stored value — the core edit + path supports clearing, unlike the children edit. An unknown + definition id is mapped to 404 and any other core ``ValueError`` + to 422 (:func:`_raise_quest_definition_error`). + """ + state: DatabaseState = request.app.state.db + supplied = body.model_dump(exclude_unset=True) + try: + if isinstance(body.rule, AdminRuleRequest): + supplied["rule"] = _rule_from_request(body.rule) + bundle = await core_quest_definitions.edit_quest_definition( + state.database, definition_id, **supplied + ) + except ValueError as error: + _raise_quest_definition_error(error) + return _quest_definition_response(bundle) + + +@router.patch( + "/quest-definitions/{definition_id}/active", + summary="Set a quest definition's active flag", + response_model=AdminQuestDefinitionResponse, +) +async def admin_set_quest_definition_active( + definition_id: int, + body: AdminQuestDefinitionActiveRequest, + request: Request, +) -> AdminQuestDefinitionResponse: + """Deactivate (or reactivate) a definition; it is NEVER deleted. + + 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: ``is_active`` + goes straight to + :func:`nestquest_core.quest_definitions.set_quest_definition_active` + — deactivation is the only removal path (the row, rule, assignees + and windows all survive, so completion history keeps its + references). An unknown definition id is mapped to 404 + (:func:`_raise_quest_definition_error`); the body model already + makes the core's real-bool check unreachable from this route. + """ + state: DatabaseState = request.app.state.db + try: + bundle = await core_quest_definitions.set_quest_definition_active( + state.database, definition_id, body.is_active + ) + except ValueError as error: + _raise_quest_definition_error(error) + return _quest_definition_response(bundle) + + __all__ = ["router"] diff --git a/tests/test_api_admin_questdef.py b/tests/test_api_admin_questdef.py new file mode 100644 index 0000000..0e378fd --- /dev/null +++ b/tests/test_api_admin_questdef.py @@ -0,0 +1,611 @@ +"""Admin-plane quest-definition route tests (task b586a2f1). + +These tests exercise the admin plane's quest-definition routes against +the ASGI app with httpx, reusing the shared local-RSA-key/stubbed-JWKS +harness (:mod:`tests.admin_jwt_harness`) so an admin JWT genuinely +authenticates — signature, issuer, audience, expiry and the +nestquest-admins group — with NO network access. + +Every done-condition case is proven against the DATABASE, read through +the SAME core copy the app uses (``nestquest_core.*``; see the two-copy +caveat in api/nestquest_core.py): + +- Authentication runs through the router's ONE ``require_admin`` + dependency: an admin JWT passes, a JWT without the group is 403, an + absent credential is 401 — and no definition is written. +- ``POST /api/v1/admin/quest-definitions``: the created definition + EXISTS with its title, description, icon, decoded rule, assignees + and windows exactly as posted. +- ``PATCH /api/v1/admin/quest-definitions/{id}``: an edit PERSISTS the + supplied fields and leaves omitted fields untouched (exclude_unset + semantics); a supplied ``windows`` list replaces the whole set; + assignment is not settable through the edit. +- ``PATCH /api/v1/admin/quest-definitions/{id}/active`` with + ``is_active: false`` sets the flag FALSE and the definition STILL + EXISTS with its rule, assignees and windows (never hard-deleted); + reactivation restores it. + +Error mapping (documented in api/routes_admin.py): a core ValueError +for a non-existent DEFINITION is 404; every other core ValueError — a +rejected rule (RuleValidationError), a bad window name or due time, a +rejected assignee list (an unknown assignee arrives in the BODY, so it +is 422, never 404) — is 422, the same failure class FastAPI reports +for malformed bodies. +""" +from __future__ import annotations + +import importlib +from types import SimpleNamespace + +import pytest + +from tests.admin_jwt_harness import ( + ADMIN_KEY, + KID, + AdminRunner, + jwks_for, + make_token, +) + +# The quest-definition business layer and its DAO through the SAME core +# copy the app uses (nestquest_core.*, NOT the conftest-stubbed +# custom_components copy) — used for the tests' database assertions +# only; the routes reach them through api.nestquest_core. +_quest_definitions = importlib.import_module( + "nestquest_core.quest_definitions" +) +_dao_rules = importlib.import_module("nestquest_core.dao_rules") + + +def _admin_headers() -> dict[str, str]: + """The Authorization header of a valid nestquest-admins JWT.""" + return {"Authorization": f"Bearer {make_token()}"} + + +def _weekly_rule_body() -> dict[str, object]: + """A valid weekly rule body: Mondays and Wednesdays from 2026-03-02.""" + return { + "rule_type": "weekly", + "interval": 1, + "weekday_set": [0, 2], + "start_date": "2026-03-02", + } + + +def _weekly_rule_dict() -> dict[str, object]: + """The lossless dict form _weekly_rule_body() must decode back to.""" + return { + "rule_type": "weekly", + "interval": 1, + "weekday_set": [0, 2], + "day_of_month": None, + "nth_weekday": None, + "nth_weekday_weekday": None, + "month": None, + "start_date": "2026-03-02", + "end_date": None, + } + + +async def _create_children( + client: object, names: tuple[str, ...] +) -> list[int]: + """Create ACTIVE children through the admin children route.""" + ids = [] + for name in names: + created = await client.post( + "/api/v1/admin/children", + json={"display_name": name}, + headers=_admin_headers(), + ) + assert created.status_code == 201 + ids.append(created.json()["id"]) + return ids + + +async def _create_definition( + client: object, assignee_ids: list[int] +) -> object: + """POST a valid two-window definition for the given assignees.""" + return await client.post( + "/api/v1/admin/quest-definitions", + json={ + "title": "Tidy the den", + "rule": _weekly_rule_body(), + "assignee_child_ids": assignee_ids, + "windows": ["morning", ["evening", "19:30"]], + "description": "Put everything back where it belongs.", + "icon": "toy-box", + }, + headers=_admin_headers(), + ) + + +async def _definitions_by_id(database: object) -> dict[int, object]: + """Every ACTIVE definition bundle through the app's core copy.""" + bundles = await _quest_definitions.list_active_definitions(database) + return {bundle.definition.id: bundle for bundle in bundles} + + +@pytest.fixture +async def admin_questdefs(temp_db_path: str) -> SimpleNamespace: + """An admin-plane app client plus its live database. + + The same runner the auth and children tests use (stubbed JWKS, no + network); the fixture also exposes the app's ONE database connection + so tests assert the routes' effects on the stored rows themselves. + """ + runner = AdminRunner(temp_db_path, jwks_for(ADMIN_KEY, KID)) + async with runner as client: + yield SimpleNamespace( + client=client, + database=runner.app.state.db.database, + ) + + +# --- authentication: the ONE require_admin dependency guards every route --- + + +async def test_quest_definition_routes_refuse_non_admin_and_absent( + admin_questdefs: SimpleNamespace, +) -> None: + """A non-admin JWT is 403, an absent credential is 401: nothing written.""" + client = admin_questdefs.client + body = {"title": "Sweep the porch"} + non_admin = make_token(groups=("some-other-group",)) + + forbidden = await client.post( + "/api/v1/admin/quest-definitions", + json=body, + headers={"Authorization": f"Bearer {non_admin}"}, + ) + assert forbidden.status_code == 403 + assert forbidden.json()["detail"] == ( + "Admin access requires membership in the nestquest-admins group" + ) + + unauthenticated_create = await client.post( + "/api/v1/admin/quest-definitions", json=body + ) + assert unauthenticated_create.status_code == 401 + + unauthenticated_edit = await client.patch( + "/api/v1/admin/quest-definitions/1", json={"title": "X"} + ) + assert unauthenticated_edit.status_code == 401 + + unauthenticated_active = await client.patch( + "/api/v1/admin/quest-definitions/1/active", json={"is_active": False} + ) + assert unauthenticated_active.status_code == 401 + + # No route ran its handler: no definition was written. + assert await _definitions_by_id(admin_questdefs.database) == {} + + +# --- create ----------------------------------------------------------------- + + +async def test_create_persists_definition_with_assignees_and_windows( + admin_questdefs: SimpleNamespace, +) -> None: + """POST /quest-definitions persists rule, assignees and windows.""" + client = admin_questdefs.client + ada, bo = await _create_children(client, ("Ada", "Bo")) + + response = await client.post( + "/api/v1/admin/quest-definitions", + json={ + "title": "Tidy the den", + "rule": _weekly_rule_body(), + "assignee_child_ids": [ada, bo], + "windows": ["morning", ["evening", "19:30"]], + "description": "Put everything back where it belongs.", + "icon": "toy-box", + }, + headers=_admin_headers(), + ) + assert response.status_code == 201 + payload = response.json() + definition_id = payload["id"] + # The serialized shape is the documented definition payload. + assert set(payload) == { + "id", + "title", + "description", + "icon", + "is_active", + "rule", + "assignees", + "windows", + } + assert payload["title"] == "Tidy the den" + assert payload["description"] == "Put everything back where it belongs." + assert payload["icon"] == "toy-box" + assert payload["is_active"] is True + assert payload["rule"] == _weekly_rule_dict() + assert {a["id"]: a["display_name"] for a in payload["assignees"]} == { + ada: "Ada", + bo: "Bo", + } + assert {w["window"]: w["due_time"] for w in payload["windows"]} == { + "morning": None, + "evening": "19:30", + } + + # Database proof through the app's core copy: the definition, its + # decoded rule, its assignees and its windows all exist as posted. + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.title == "Tidy the den" + assert stored.definition.description == ( + "Put everything back where it belongs." + ) + assert stored.definition.icon == "toy-box" + assert stored.definition.is_active is True + assert stored.rule.to_dict() == _weekly_rule_dict() + assert sorted(child.id for child in stored.assignees) == [ada, bo] + assert {(w.window, w.due_time) for w in stored.windows} == { + ("morning", None), + ("evening", "19:30"), + } + + +async def test_create_with_required_fields_only_uses_core_defaults( + admin_questdefs: SimpleNamespace, +) -> None: + """Omitted optional fields default to NULL through the core layer.""" + client = admin_questdefs.client + ada = (await _create_children(client, ("Ada",)))[0] + + response = await client.post( + "/api/v1/admin/quest-definitions", + json={ + "title": "Water the plants", + "rule": _weekly_rule_body(), + "assignee_child_ids": [ada], + "windows": ["morning"], + }, + headers=_admin_headers(), + ) + assert response.status_code == 201 + payload = response.json() + assert payload["description"] is None + assert payload["icon"] is None + stored = (await _definitions_by_id(admin_questdefs.database))[ + payload["id"] + ] + assert stored.definition.description is None + assert stored.definition.icon is None + + +async def test_create_without_required_fields_is_422( + admin_questdefs: SimpleNamespace, +) -> None: + """A body missing any required field is 422 and creates nothing.""" + client = admin_questdefs.client + bodies: list[dict[str, object]] = [ + {}, # everything missing + {"title": "Tidy the den"}, # rule, assignees, windows missing + { + "title": "Tidy the den", + "rule": _weekly_rule_body(), + }, # assignees and windows missing + { + "title": "Tidy the den", + "rule": _weekly_rule_body(), + "assignee_child_ids": [1], + }, # windows missing + ] + for body in bodies: + response = await client.post( + "/api/v1/admin/quest-definitions", + json=body, + headers=_admin_headers(), + ) + assert response.status_code == 422, body + assert await _definitions_by_id(admin_questdefs.database) == {} + + +@pytest.mark.parametrize( + "override", + [ + {"title": ""}, # empty: the core rejects it + {"title": " "}, # whitespace-only: empty to a parent + {"assignee_child_ids": []}, # empty assignee list + {"assignee_child_ids": [999, 1000]}, # unknown children: 422 not 404 + {"assignee_child_ids": ["Ada"]}, # wrong-typed ids + {"windows": []}, # empty window set + {"windows": "morning"}, # not a list + {"windows": ["attic"]}, # not a const.QUEST_WINDOWS name + {"windows": [["morning", "25:00"]]}, # not a 24-hour time + {"windows": [["morning", "8:00"]]}, # not a strict HH:MM + {"windows": [["morning", None], "morning"]}, # duplicate window + {"rule": {"rule_type": "weekly"}}, # weekly without weekday_set + {"rule": {"rule_type": "fortnightly"}}, # unknown rule_type + {"rule": {"rule_type": "daily", "interval": 0}}, # interval < 1 + {"rule": {"rule_type": "weekly", "weekday_set": [7]}}, # out of range + {"rule": {"rule_type": "monthly_day"}}, # missing day_of_month + {"rule": {"rule_type": "daily", "day_of_month": 5}}, # forbidden field + {"rule": {"rule_type": "daily", "start_date": "2026-13-01"}}, + {"rule": None}, # an explicit null rule is a supplied invalid value + ], +) +async def test_create_rejections_are_422_and_persist_nothing( + admin_questdefs: SimpleNamespace, override: dict[str, object] +) -> None: + """Rejected bodies are 422 and never create a definition.""" + client = admin_questdefs.client + ids = await _create_children(client, ("Ada", "Bo")) + body: dict[str, object] = { + "title": "Tidy the den", + "rule": _weekly_rule_body(), + "assignee_child_ids": ids, + "windows": ["morning", ["evening", "19:30"]], + } + body.update(override) + + response = await client.post( + "/api/v1/admin/quest-definitions", + json=body, + headers=_admin_headers(), + ) + assert response.status_code == 422, override + assert await _definitions_by_id(admin_questdefs.database) == {} + + +# --- edit ------------------------------------------------------------------- + + +async def test_edit_persists_supplied_fields_and_leaves_omitted_untouched( + admin_questdefs: SimpleNamespace, +) -> None: + """PATCH persists exactly the supplied fields (exclude_unset).""" + client = admin_questdefs.client + ada, bo = await _create_children(client, ("Ada", "Bo")) + created = await _create_definition(client, [ada, bo]) + assert created.status_code == 201 + definition_id = created.json()["id"] + dao = _dao_rules.QuestDefinitionsDao(admin_questdefs.database) + original_rule_id = (await dao.get(definition_id)).schedule_rule_id + + # Only title and windows: description, icon, rule and assignees + # must survive; the supplied windows REPLACE the whole set. + edited = await client.patch( + f"/api/v1/admin/quest-definitions/{definition_id}", + json={"title": "Clear the den", "windows": ["afternoon"]}, + headers=_admin_headers(), + ) + assert edited.status_code == 200 + assert edited.json()["title"] == "Clear the den" + assert {w["window"]: w["due_time"] for w in edited.json()["windows"]} == { + "afternoon": None + } + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.title == "Clear the den" + assert {(w.window, w.due_time) for w in stored.windows} == { + ("afternoon", None) + } + # Omitted fields were never passed to the core: unchanged. + assert stored.definition.description == ( + "Put everything back where it belongs." + ) + assert stored.definition.icon == "toy-box" + assert stored.rule.to_dict() == _weekly_rule_dict() + # Assignment is not settable through the edit route: untouched. + assert sorted(child.id for child in stored.assignees) == [ada, bo] + + # A second edit touching only the rule leaves the metadata and the + # windows alone, and updates the rule row IN PLACE (same rule id). + rescheduled = await client.patch( + f"/api/v1/admin/quest-definitions/{definition_id}", + json={"rule": {"rule_type": "daily", "start_date": "2026-04-01"}}, + headers=_admin_headers(), + ) + assert rescheduled.status_code == 200 + assert rescheduled.json()["rule"]["rule_type"] == "daily" + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.title == "Clear the den" + assert {(w.window, w.due_time) for w in stored.windows} == { + ("afternoon", None) + } + assert stored.rule.to_dict() == { + "rule_type": "daily", + "interval": 1, + "weekday_set": None, + "day_of_month": None, + "nth_weekday": None, + "nth_weekday_weekday": None, + "month": None, + "start_date": "2026-04-01", + "end_date": None, + } + record = await dao.get(definition_id) + assert record.schedule_rule_id == original_rule_id + rule_row = await _dao_rules.ScheduleRulesDao( + admin_questdefs.database + ).get(record.schedule_rule_id) + assert rule_row.rule_type == "daily" + assert rule_row.start_date == "2026-04-01" + + +async def test_edit_can_clear_description_and_icon_explicitly( + admin_questdefs: SimpleNamespace, +) -> None: + """An explicit null description/icon clears them (the core's edit path). + + Unlike the children edit, the definition edit supports clearing: a + supplied null IS forwarded and writes NULL; everything omitted + stays untouched. + """ + client = admin_questdefs.client + ada = (await _create_children(client, ("Ada",)))[0] + created = await _create_definition(client, [ada]) + definition_id = created.json()["id"] + + response = await client.patch( + f"/api/v1/admin/quest-definitions/{definition_id}", + json={"description": None, "icon": None}, + headers=_admin_headers(), + ) + assert response.status_code == 200 + assert response.json()["description"] is None + assert response.json()["icon"] is None + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.description is None + assert stored.definition.icon is None + assert stored.definition.title == "Tidy the den" + assert stored.rule.to_dict() == _weekly_rule_dict() + + +async def test_edit_unknown_definition_is_404( + admin_questdefs: SimpleNamespace, +) -> None: + """An edit naming a definition that does not exist is 404.""" + response = await admin_questdefs.client.patch( + "/api/v1/admin/quest-definitions/999", + json={"title": "Ghost chore"}, + headers=_admin_headers(), + ) + assert response.status_code == 404 + assert response.json()["detail"] == "Quest definition not found" + + +async def test_edit_rejections_are_422_and_change_nothing( + admin_questdefs: SimpleNamespace, +) -> None: + """A rejected rule or window set is 422; the stored state survives. + + The core rejects BEFORE any write (and the DAO edit runs in one + transaction), so a 422 never leaves a half-applied change behind. + """ + client = admin_questdefs.client + ada = (await _create_children(client, ("Ada",)))[0] + created = await _create_definition(client, [ada]) + definition_id = created.json()["id"] + + rejected_bodies: list[dict[str, object]] = [ + {"rule": {"rule_type": "weekly"}}, # weekly without weekday_set + {"rule": None}, # an explicit null rule is rejected by the core + {"windows": ["attic"]}, # unknown window name + {"windows": None}, # an explicit null window set is rejected + {"title": ""}, # the core rejects an empty title + ] + for body in rejected_bodies: + response = await client.patch( + f"/api/v1/admin/quest-definitions/{definition_id}", + json=body, + headers=_admin_headers(), + ) + assert response.status_code == 422, body + + # Database proof: nothing changed for any rejected call. + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.title == "Tidy the den" + assert stored.definition.description == ( + "Put everything back where it belongs." + ) + assert stored.rule.to_dict() == _weekly_rule_dict() + assert {(w.window, w.due_time) for w in stored.windows} == { + ("morning", None), + ("evening", "19:30"), + } + + +# --- set_active ------------------------------------------------------------- + + +async def test_set_active_toggles_the_flag_and_the_definition_survives( + admin_questdefs: SimpleNamespace, +) -> None: + """is_active=false deactivates; the definition STILL exists intact. + + Deactivation is the only removal path: the row, rule, assignees and + windows all survive, and reactivation through the same route + restores the flag without anything having been lost. + """ + client = admin_questdefs.client + ada = (await _create_children(client, ("Ada",)))[0] + created = await _create_definition(client, [ada]) + assert created.status_code == 201 + definition_id = created.json()["id"] + + deactivated = await client.patch( + f"/api/v1/admin/quest-definitions/{definition_id}/active", + json={"is_active": False}, + headers=_admin_headers(), + ) + assert deactivated.status_code == 200 + payload = deactivated.json() + assert payload["is_active"] is False + # Everything else came back unchanged: deactivation only flips a flag. + assert payload["title"] == "Tidy the den" + assert payload["rule"] == _weekly_rule_dict() + assert [a["id"] for a in payload["assignees"]] == [ada] + assert {w["window"]: w["due_time"] for w in payload["windows"]} == { + "morning": None, + "evening": "19:30", + } + + # Database proof: the definition left the ACTIVE list but its row, + # rule, assignees and windows were NOT hard-deleted. + assert definition_id not in await _definitions_by_id( + admin_questdefs.database + ) + dao = _dao_rules.QuestDefinitionsDao(admin_questdefs.database) + record = await dao.get(definition_id) + assert record is not None + assert record.is_active is False + assert record.title == "Tidy the den" + assert await dao.list_assignees(definition_id) != [] + assert await dao.list_windows(definition_id) != [] + + # Reactivation goes through the same route: the flag is set, not + # one-way, and the surviving rule is still the one that was stored. + reactivated = await client.patch( + f"/api/v1/admin/quest-definitions/{definition_id}/active", + json={"is_active": True}, + headers=_admin_headers(), + ) + assert reactivated.status_code == 200 + assert reactivated.json()["is_active"] is True + stored = (await _definitions_by_id(admin_questdefs.database))[ + definition_id + ] + assert stored.definition.is_active is True + assert stored.rule.to_dict() == _weekly_rule_dict() + assert [child.id for child in stored.assignees] == [ada] + + +async def test_set_active_unknown_definition_is_404( + admin_questdefs: SimpleNamespace, +) -> None: + """A set_active naming a definition that does not exist is 404.""" + response = await admin_questdefs.client.patch( + "/api/v1/admin/quest-definitions/999/active", + json={"is_active": False}, + headers=_admin_headers(), + ) + assert response.status_code == 404 + assert response.json()["detail"] == "Quest definition not found" + + +async def test_set_active_malformed_body_is_422( + admin_questdefs: SimpleNamespace, +) -> None: + """A non-bool is_active is rejected by the body model (422).""" + response = await admin_questdefs.client.patch( + "/api/v1/admin/quest-definitions/1/active", + json={"is_active": "maybe"}, + headers=_admin_headers(), + ) + assert response.status_code == 422