diff --git a/api/routes_admin.py b/api/routes_admin.py index adff79d..1c4de6b 100644 --- a/api/routes_admin.py +++ b/api/routes_admin.py @@ -252,7 +252,13 @@ import datetime from typing import Annotated, Literal, NoReturn from fastapi import APIRouter, Depends, HTTPException, Request, Response -from pydantic import BaseModel, StrictBool, field_validator, model_validator +from pydantic import ( + BaseModel, + StrictBool, + StrictInt, + field_validator, + model_validator, +) from api import transitions as api_transitions from api.auth import require_admin @@ -457,11 +463,13 @@ class AdminQuestDefinitionCreateRequest(BaseModel): date the child is absent). ``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. + Assignee ids are strict JSON integers: a boolean, float or numeric + string is 422, never coerced to a child id. """ title: str rule: AdminRuleRequest - assignee_child_ids: list[int] + assignee_child_ids: list[StrictInt] windows: list[str | tuple[str, str | None]] description: str | None = None icon: str | None = None @@ -483,6 +491,7 @@ class AdminQuestDefinitionEditRequest(BaseModel): REPLACES the whole assignee set (non-empty, every id an existing active child — create's contract; a rejection changes nothing). An omitted (or ``null``) ``skip_on_away`` keeps the stored value. + Assignee ids are strict JSON integers, as on create. """ title: str | None = None @@ -491,7 +500,7 @@ class AdminQuestDefinitionEditRequest(BaseModel): rule: AdminRuleRequest | None = None windows: list[str | tuple[str, str | None]] | None = None skip_on_away: StrictBool | None = None - assignee_child_ids: list[int] | None = None + assignee_child_ids: list[StrictInt] | None = None class AdminQuestDefinitionActiveRequest(BaseModel): diff --git a/tests/test_api_admin_questdef_assignees.py b/tests/test_api_admin_questdef_assignees.py index fc42d0c..40c4b86 100644 --- a/tests/test_api_admin_questdef_assignees.py +++ b/tests/test_api_admin_questdef_assignees.py @@ -10,6 +10,8 @@ assignee set inside the core edit's ONE transaction - an empty list, a duplicate, an unknown child or an inactive child is 422 and NOTHING is written — the stored assignees (and any other field supplied in the same body) are unchanged; +- a JSON boolean, float or numeric string id is 422 on create and edit + (never coerced to an int) and nothing is written; - an omitted ``assignee_child_ids`` leaves the assignees unchanged; - an assignment change regenerates the definition's future instances through the existing core path; @@ -39,6 +41,9 @@ from tests.admin_jwt_harness import ( _dao_rules = importlib.import_module("nestquest_core.dao_rules") _dao_instances = importlib.import_module("nestquest_core.dao_instances") _materialize = importlib.import_module("nestquest_core.materialize") +_quest_definitions = importlib.import_module( + "nestquest_core.quest_definitions" +) #: The panel service token the harness app is configured with. HARNESS_PANEL_TOKEN = "admin-test-panel-token" @@ -245,6 +250,66 @@ async def test_rejected_assignee_list_is_422_and_changes_nothing( assert (await dao.get(definition_id)).title == "Brush teeth" +#: JSON ids the lax ``int`` parser would coerce (``true`` -> 1). +_COERCIBLE_IDS = [True, 1.0, "1"] + + +@pytest.mark.parametrize("coercible", _COERCIBLE_IDS) +async def test_edit_rejects_coercible_assignee_ids_and_changes_nothing( + admin_assignees: SimpleNamespace, coercible: object +) -> None: + """A bool, float or numeric-string id edit is 422; nothing changes.""" + client = admin_assignees.client + ada, bo = await _create_children(client, ("Ada", "Bo")) + # Assign bo only, so a coerced id 1 (ada) would be a visible change. + definition_id = await _create_definition(client, [bo]) + assert ada == 1 + + response = await client.patch( + _edit_path(definition_id), + json={"title": "Renamed", "assignee_child_ids": [coercible]}, + headers=_admin_headers(), + ) + + assert response.status_code == 422 + assert await _stored_assignee_ids( + admin_assignees.database, definition_id + ) == {bo} + dao = _dao_rules.QuestDefinitionsDao(admin_assignees.database) + assert (await dao.get(definition_id)).title == "Brush teeth" + + +@pytest.mark.parametrize("coercible", _COERCIBLE_IDS) +async def test_create_rejects_coercible_assignee_ids_and_persists_nothing( + admin_assignees: SimpleNamespace, coercible: object +) -> None: + """A bool, float or numeric-string id create is 422; nothing is written.""" + client = admin_assignees.client + ada, bo = await _create_children(client, ("Ada", "Bo")) + assert ada == 1 + existing_id = await _create_definition(client, [bo]) + + response = await client.post( + "/api/v1/admin/quest-definitions", + json={ + "title": "Tidy the den", + "rule": {"rule_type": "daily"}, + "assignee_child_ids": [coercible], + "windows": ["morning"], + }, + headers=_admin_headers(), + ) + + assert response.status_code == 422 + bundles = await _quest_definitions.list_active_definitions( + admin_assignees.database + ) + assert [bundle.definition.id for bundle in bundles] == [existing_id] + assert await _stored_assignee_ids( + admin_assignees.database, existing_id + ) == {bo} + + async def test_rejected_assignees_on_unknown_definition_is_404( admin_assignees: SimpleNamespace, ) -> None: