From 90671815eef4140b0359a43f15e4bcb381dddcea Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 09:12:21 -0400 Subject: [PATCH] Add assign_child and unassign_child business layer (Feature 06, task 4) --- .../nestquest/quest_definitions.py | 74 +++++- tests/test_quest_definitions.py | 213 ++++++++++++++++++ 2 files changed, 286 insertions(+), 1 deletion(-) diff --git a/custom_components/nestquest/quest_definitions.py b/custom_components/nestquest/quest_definitions.py index b574f08..901a4d8 100644 --- a/custom_components/nestquest/quest_definitions.py +++ b/custom_components/nestquest/quest_definitions.py @@ -4,7 +4,8 @@ Sits between the Feature 09 service gate and the typed :class:`~.dao_rules.QuestDefinitionsDao`: callers get validation and the all-or-nothing create/edit here, and never touch the DAO SQL directly. :func:`create_quest_definition` is the create path and -:func:`edit_quest_definition` the edit path; there is no permission +:func:`edit_quest_definition` the edit path; :func:`assign_child` and +:func:`unassign_child` are the assignment paths. There is no permission check here (that is Feature 09). Validation policy mirrors :mod:`.children`: @@ -185,6 +186,18 @@ def _validate_definition_id(value: object) -> int: return value +def _validate_child_id(value: object) -> int: + """Reject non-int child ids (bools included) before any lookup. + + SQLite binds Python bools as integers, so ``True`` would silently + address child 1 and a float would round — malformed service input + must never mutate the wrong profile. + """ + if isinstance(value, bool) or not isinstance(value, int): + raise ValueError(f"child_id must be an integer, got {value!r}") + return value + + async def create_quest_definition( database: NestQuestDatabase, title: str, @@ -321,3 +334,62 @@ async def edit_quest_definition( assignees=snapshot.assignees, windows=snapshot.windows, ) + + +async def assign_child( + database: NestQuestDatabase, + definition_id: int, + child_id: int, +) -> list[ChildRecord]: + """Assign ``child_id`` to the definition (idempotent). + + Validates that both ids are plain ints (bools and floats rejected) + and that the child exists and is active — the DAO enforces the + active/exists check inside its transaction, and also confirms the + definition exists. Assigning an already-assigned child is a no-op + (no duplicate row). Returns the definition's assignees read back + through the DAO, a consistent post-assignment roster. Raises + ValueError on any rejected argument. Only the + ``quest_definition_assignees`` link is written; existing instances + and completion history are never touched. + """ + _validate_definition_id(definition_id) + _validate_child_id(child_id) + dao = QuestDefinitionsDao(database) + try: + await dao.add_assignee(definition_id, child_id) + except ValueError as error: + # The DAO names only the child/definition; re-raise so the + # public contract names the field the caller actually passed. + message = str(error) + if message.startswith("child "): + raise ValueError(f"child_id: {message}") from error + if message.startswith("quest definition "): + raise ValueError(f"definition_id: {message}") from error + raise + return await dao.list_assignees(definition_id) + + +async def unassign_child( + database: NestQuestDatabase, + definition_id: int, + child_id: int, +) -> bool: + """Unassign ``child_id`` from the definition; True when removed. + + Validates both ids are plain ints (bools and floats rejected) and + that the definition exists, then removes the (definition, child) + link. Returns True when a row was removed and False when the child + was not currently assigned. Raises ValueError on a rejected + argument. Only the ``quest_definition_assignees`` link is written; + existing instances and completion history are never touched. + """ + _validate_definition_id(definition_id) + _validate_child_id(child_id) + dao = QuestDefinitionsDao(database) + definition = await dao.get(definition_id) + if definition is None: + raise ValueError( + f"definition_id: quest definition {definition_id} does not exist" + ) + return await dao.remove_assignee(definition_id, child_id) diff --git a/tests/test_quest_definitions.py b/tests/test_quest_definitions.py index 2f91b82..c3ba262 100644 --- a/tests/test_quest_definitions.py +++ b/tests/test_quest_definitions.py @@ -16,8 +16,10 @@ from custom_components.nestquest.db import NestQuestDatabase from custom_components.nestquest.migrations import apply_migrations from custom_components.nestquest.quest_definitions import ( CreatedQuestDefinition, + assign_child, create_quest_definition, edit_quest_definition, + unassign_child, ) from custom_components.nestquest.recurrence import RuleType, ScheduleRule @@ -880,3 +882,214 @@ def test_edit_post_write_failure_rolls_back_everything(tmp_path) -> None: return None _with_db(tmp_path, "edit-post-write-rollback.db")(_body) + + +# --------------------------------------------------------------------------- +# assign_child / unassign_child: the assignment business-layer paths +# --------------------------------------------------------------------------- + + +def test_assign_child_multi_assignee_success(tmp_path) -> None: + """Assigning a second child leaves both as assignees (D-008).""" + async def _body(database, children, rules, definitions, child): + created = await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], ["morning"] + ) + bo = await children.create("Bo", NOW) + assignees = await assign_child(database, created.definition.id, bo.id) + assert sorted(c.id for c in assignees) == sorted([child.id, bo.id]) + return assignees + + _with_db(tmp_path, "assign-multi.db")(_body) + + +def test_assign_child_duplicate_is_noop(tmp_path) -> None: + """Assigning an already-assigned child adds no duplicate row.""" + async def _body(database, children, rules, definitions, child): + created = await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], ["morning"] + ) + assignees = await assign_child( + database, created.definition.id, child.id + ) + assert [c.id for c in assignees] == [child.id] + again = await assign_child( + database, created.definition.id, child.id + ) + assert [c.id for c in again] == [child.id] + return assignees + + _with_db(tmp_path, "assign-duplicate.db")(_body) + + +def test_assign_child_unknown_child_names_field(tmp_path) -> 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="child_id.*does not exist" + ): + await assign_child(database, created.definition.id, 999) + return None + + _with_db(tmp_path, "assign-unknown-child.db")(_body) + + +def test_assign_child_inactive_child_names_field(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + created = await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], ["morning"] + ) + bo = await children.create("Bo", NOW) + await children.set_active(bo.id, False) + with pytest.raises(ValueError, match="child_id.*inactive"): + await assign_child(database, created.definition.id, bo.id) + # The failed assignment leaves the roster unchanged. + assert [c.id for c in await definitions.list_assignees( + created.definition.id + )] == [child.id] + return None + + _with_db(tmp_path, "assign-inactive-child.db")(_body) + + +def test_assign_child_nonexistent_definition_names_field(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises( + ValueError, match="definition_id.*does not exist" + ): + await assign_child(database, 999, child.id) + return None + + _with_db(tmp_path, "assign-missing-definition.db")(_body) + + +@pytest.mark.parametrize("bad", [True, 1.5, "1", None]) +def test_assign_child_rejects_non_int_definition_id(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises( + ValueError, match="definition_id must be an integer" + ): + await assign_child(database, bad, child.id) + return None + + _with_db(tmp_path, "assign-bad-definition-id.db")(_body) + + +@pytest.mark.parametrize("bad", [True, 1.5, "1", None]) +def test_assign_child_rejects_non_int_child_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="child_id must be an integer"): + await assign_child(database, created.definition.id, bad) + return None + + _with_db(tmp_path, "assign-bad-child-id.db")(_body) + + +def test_unassign_child_success_returns_true(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + bo = await children.create("Bo", NOW) + created = await create_quest_definition( + database, + "Brush teeth", + _daily_rule(), + [child.id, bo.id], + ["morning"], + ) + assert await unassign_child( + database, created.definition.id, bo.id + ) is True + assert [c.id for c in await definitions.list_assignees( + created.definition.id + )] == [child.id] + return None + + _with_db(tmp_path, "unassign-success.db")(_body) + + +def test_unassign_child_not_assigned_returns_false(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + created = await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], ["morning"] + ) + assert await unassign_child( + database, created.definition.id, 999 + ) is False + return None + + _with_db(tmp_path, "unassign-not-assigned.db")(_body) + + +def test_unassign_child_nonexistent_definition_names_field(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises( + ValueError, match="definition_id.*does not exist" + ): + await unassign_child(database, 999, child.id) + return None + + _with_db(tmp_path, "unassign-missing-definition.db")(_body) + + +@pytest.mark.parametrize("bad", [True, 1.5, "1", None]) +def test_unassign_child_rejects_non_int_definition_id(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises( + ValueError, match="definition_id must be an integer" + ): + await unassign_child(database, bad, child.id) + return None + + _with_db(tmp_path, "unassign-bad-definition-id.db")(_body) + + +@pytest.mark.parametrize("bad", [True, 1.5, "1", None]) +def test_unassign_child_rejects_non_int_child_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="child_id must be an integer"): + await unassign_child(database, created.definition.id, bad) + return None + + _with_db(tmp_path, "unassign-bad-child-id.db")(_body) + + +def test_assign_and_unassign_leave_instances_and_history_alone( + tmp_path, +) -> None: + """Assign/unassign write the assignees table only. + + An already-generated instance keeps its generation-time child_id, and + no completion event appears — the assignment paths touch neither + ``quest_instances`` nor ``completion_events``. + """ + async def _body(database, children, rules, definitions, child): + created = await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], ["morning"] + ) + bo = await children.create("Bo", NOW) + await database.execute( + "INSERT INTO quest_instances (definition_id, child_id, " + "window, due_date, generated_at) VALUES (?, ?, 'morning', ?, ?)", + (created.definition.id, child.id, "2026-09-15", NOW), + ) + await assign_child(database, created.definition.id, bo.id) + await unassign_child(database, created.definition.id, child.id) + row = await database.fetch_one( + "SELECT child_id FROM quest_instances WHERE definition_id = ?", + (created.definition.id,), + ) + assert row == (child.id,) + events = await database.fetch_one( + "SELECT COUNT(*) FROM completion_events" + ) + assert events == (0,) + return None + + _with_db(tmp_path, "assign-unassign-history.db")(_body)