From c988f6508b8879c209a306a9aa8de8e13ce7598f Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 09:35:21 -0400 Subject: [PATCH 1/5] Rename task_* tables to quest_* with legacy-dev migration path (D-007) Per design/IMPLEMENTATION-ORDER.md 1.1 and decision D-007: the domain model takes the quest vocabulary pre-release. SCHEMA_V1 now creates quest_definitions/quest_instances directly; DAO classes, records and error messages follow (QuestDefinitionsDao, QuestInstancesDao). The migration runner gains migration 2, an async-callable step that detects surviving task_* tables on dev machines and ALTER TABLE RENAMES them inside the migration transaction, with FK clauses rewritten by SQLite. Migration 1's content rewrite is legal because nothing was ever shipped; dev databases stamped version 1 ride migration 2 up to 2. --- custom_components/nestquest/dao_instances.py | 56 ++-- custom_components/nestquest/dao_rules.py | 46 +-- custom_components/nestquest/migrations.py | 97 +++++- custom_components/nestquest/schema.py | 22 +- tests/test_dao_instances.py | 30 +- tests/test_dao_matrix.py | 26 +- tests/test_dao_rules.py | 50 +-- tests/test_lifecycle.py | 4 +- tests/test_migrations.py | 315 +++++++++++++++++-- tests/test_schema.py | 112 +++---- tests/test_startup_db.py | 10 +- 11 files changed, 546 insertions(+), 222 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index e4aa0b6..4c78a49 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -1,4 +1,4 @@ -"""Typed DAO layer for task_instances and the append-only completion_events. +"""Typed DAO layer for quest_instances and the append-only completion_events. All SQL for these tables lives in this module per the feature guardrails: callers get typed dataclasses back and never see raw rows @@ -14,7 +14,7 @@ separate reversal event, and instance state derives from the latest event. A package-wide guard test (bottom of the test module) keeps mutation SQL for this table out of every other module. -Upsert semantics: ``TaskInstancesDao.upsert`` is idempotent on +Upsert semantics: ``QuestInstancesDao.upsert`` is idempotent on (definition_id, due_date) — re-running materialization for a date whose instance already exists updates nothing that matters (child_id/due_time are refreshed only in the degenerate re-generation @@ -82,8 +82,8 @@ def _next_day_start(date_str: str) -> str: @dataclass(frozen=True) -class TaskInstanceRecord: - """One row of ``task_instances``.""" +class QuestInstanceRecord: + """One row of ``quest_instances``.""" id: int definition_id: int @@ -121,8 +121,8 @@ EVENT_COMPLETED = "completed" EVENT_UNCOMPLETED = "uncompleted" -def _instance_from_row(row: tuple) -> TaskInstanceRecord: - return TaskInstanceRecord( +def _instance_from_row(row: tuple) -> QuestInstanceRecord: + return QuestInstanceRecord( id=row[0], definition_id=row[1], child_id=row[2], @@ -145,8 +145,8 @@ def _event_from_row(row: tuple) -> CompletionEventRecord: ) -class TaskInstancesDao: - """Typed async access to the ``task_instances`` table.""" +class QuestInstancesDao: + """Typed async access to the ``quest_instances`` table.""" def __init__(self, database: NestQuestDatabase) -> None: self._database = database @@ -159,7 +159,7 @@ class TaskInstancesDao: generated_at: str, *, due_time: str | None = None, - ) -> TaskInstanceRecord: + ) -> QuestInstanceRecord: """Create the dated instance for (definition_id, due_date) idempotently. Running materialization twice for the same definition/date @@ -207,16 +207,16 @@ class TaskInstancesDao: f"{today!r}" ) definition = await self._database.fetch_one( - "SELECT child_id FROM task_definitions WHERE id = ?", + "SELECT child_id FROM quest_definitions WHERE id = ?", (definition_id,), ) if definition is None: raise ValueError( - f"task definition {definition_id} does not exist" + f"quest definition {definition_id} does not exist" ) if definition[0] != child_id: raise ValueError( - f"task definition {definition_id} is assigned to " + f"quest definition {definition_id} is assigned to " f"child {definition[0]}, not {child_id}; instance " "must carry the definition's assignee" ) @@ -227,7 +227,7 @@ class TaskInstancesDao: raise ValueError(f"child {child_id} does not exist") existing = await self._database.fetch_one( "SELECT 1 FROM completion_events " - "WHERE instance_id = (SELECT id FROM task_instances " + "WHERE instance_id = (SELECT id FROM quest_instances " "WHERE definition_id = ? AND due_date = ?)", (definition_id, due_date), ) @@ -238,7 +238,7 @@ class TaskInstancesDao: "event and is immutable; it cannot be regenerated" ) await self._database.execute( - "INSERT INTO task_instances (definition_id, child_id, " + "INSERT INTO quest_instances (definition_id, child_id, " "due_date, due_time, generated_at) " "VALUES (?, ?, ?, ?, ?) " "ON CONFLICT (definition_id, due_date) DO UPDATE SET " @@ -259,30 +259,30 @@ class TaskInstancesDao: async def get( self, definition_id: int, due_date: str - ) -> TaskInstanceRecord | None: + ) -> QuestInstanceRecord | None: """Return the instance for (definition_id, due_date), or None.""" row = await self._database.fetch_one( - f"SELECT {_INSTANCE_COLUMNS} FROM task_instances " + f"SELECT {_INSTANCE_COLUMNS} FROM quest_instances " "WHERE definition_id = ? AND due_date = ?", (definition_id, due_date), ) return _instance_from_row(row) if row is not None else None - async def get_by_id(self, instance_id: int) -> TaskInstanceRecord | None: + async def get_by_id(self, instance_id: int) -> QuestInstanceRecord | None: """Return the instance with ``instance_id``, or None.""" row = await self._database.fetch_one( - f"SELECT {_INSTANCE_COLUMNS} FROM task_instances WHERE id = ?", + f"SELECT {_INSTANCE_COLUMNS} FROM quest_instances WHERE id = ?", (instance_id,), ) return _instance_from_row(row) if row is not None else None async def list_by_child_and_date( self, child_id: int, due_date: str - ) -> list[TaskInstanceRecord]: + ) -> list[QuestInstanceRecord]: """Return the child's instances due exactly on ``due_date``.""" _validate_date(due_date, "due_date") rows = await self._database.fetch_all( - f"SELECT {_INSTANCE_COLUMNS} FROM task_instances " + f"SELECT {_INSTANCE_COLUMNS} FROM quest_instances " "WHERE child_id = ? AND due_date = ? ORDER BY id", (child_id, due_date), ) @@ -293,7 +293,7 @@ class TaskInstancesDao: child_id: int, range_start: str, range_end: str, - ) -> list[TaskInstanceRecord]: + ) -> list[QuestInstanceRecord]: """Return the child's instances with due_date in the closed range.""" _validate_date(range_start, "range_start") _validate_date(range_end, "range_end") @@ -303,7 +303,7 @@ class TaskInstancesDao: f"{range_end!r} < {range_start!r}" ) rows = await self._database.fetch_all( - f"SELECT {_INSTANCE_COLUMNS} FROM task_instances " + f"SELECT {_INSTANCE_COLUMNS} FROM quest_instances " "WHERE child_id = ? AND due_date >= ? AND due_date <= ? " "ORDER BY due_date, id", (child_id, range_start, range_end), @@ -338,10 +338,10 @@ class TaskInstancesDao: today = datetime.date.today().isoformat() effective_cutoff = max(cutoff_date, today) result = await self._database.execute( - "DELETE FROM task_instances WHERE definition_id = ? " + "DELETE FROM quest_instances WHERE definition_id = ? " "AND due_date >= ? AND NOT EXISTS (" " SELECT 1 FROM completion_events " - " WHERE instance_id = task_instances.id" + " WHERE instance_id = quest_instances.id" ")", (definition_id, effective_cutoff), ) @@ -418,16 +418,16 @@ class CompletionEventsDao: async with _connection_lock(self._database): async with self._database.transaction(): instance = await self._database.fetch_one( - "SELECT child_id FROM task_instances WHERE id = ?", + "SELECT child_id FROM quest_instances WHERE id = ?", (instance_id,), ) if instance is None: raise ValueError( - f"task instance {instance_id} does not exist" + f"quest instance {instance_id} does not exist" ) if instance[0] != child_id: raise ValueError( - f"task instance {instance_id} belongs to child " + f"quest instance {instance_id} belongs to child " f"{instance[0]}, not {child_id}" ) child = await self._database.fetch_one( @@ -493,7 +493,7 @@ class CompletionEventsDao: ) if due_date_scope: predicate = ( - "SELECT id FROM task_instances WHERE child_id = ? " + "SELECT id FROM quest_instances WHERE child_id = ? " "AND due_date >= ? AND due_date <= ?" ) parameters: list[object] = [child_id, range_start, range_end] diff --git a/custom_components/nestquest/dao_rules.py b/custom_components/nestquest/dao_rules.py index 51ba457..494dd03 100644 --- a/custom_components/nestquest/dao_rules.py +++ b/custom_components/nestquest/dao_rules.py @@ -1,6 +1,6 @@ -"""Typed DAO layer for the schedule_rules and task_definitions tables. +"""Typed DAO layer for the schedule_rules and quest_definitions tables. -All SQL for ``schedule_rules`` and ``task_definitions`` lives in this +All SQL for ``schedule_rules`` and ``quest_definitions`` lives in this module per the feature guardrails: callers get typed dataclasses back and never see raw rows or SQL. Every method is async and runs through :class:`~.db.NestQuestDatabase`, so each statement executes on the HA @@ -9,16 +9,16 @@ executor and the event loop never blocks. Guardrail mapping: - ``schedule_rules`` deletion is delete-if-unreferenced: a rule still - referenced by any task definition is rejected (the done-condition), + referenced by any quest definition is rejected (the done-condition), not cascaded. -- ``task_definitions`` has no delete path at all: definitions are +- ``quest_definitions`` has no delete path at all: definitions are deactivated, never hard-deleted, and history must survive. - ``set_assignee`` changes future instances only — the DAO writes the definition row and nothing else; reassignment never rewrites existing instances or completion history (Feature 06 guardrail). - Validation of rule well-formedness and child activeness lives partly in the schema CHECKs (rule-type coherence) and partly in - :meth:`TaskDefinitionsDao.create` (active child, rule existence); + :meth:`QuestDefinitionsDao.create` (active child, rule existence); no permission checks here, that is the Feature 09 gate's job. Concurrency: ``delete_rule_if_unreferenced`` performs its reference @@ -81,8 +81,8 @@ class ScheduleRuleRecord: @dataclass(frozen=True) -class TaskDefinitionRecord: - """One row of ``task_definitions``.""" +class QuestDefinitionRecord: + """One row of ``quest_definitions``.""" id: int title: str @@ -119,8 +119,8 @@ def _rule_from_row(row: tuple) -> ScheduleRuleRecord: ) -def _definition_from_row(row: tuple) -> TaskDefinitionRecord: - return TaskDefinitionRecord( +def _definition_from_row(row: tuple) -> QuestDefinitionRecord: + return QuestDefinitionRecord( id=row[0], title=row[1], description=row[2], @@ -258,7 +258,7 @@ class ScheduleRulesDao: async with _connection_lock(self._database): async with self._database.transaction(): row = await self._database.fetch_one( - "SELECT 1 FROM task_definitions " + "SELECT 1 FROM quest_definitions " "WHERE schedule_rule_id = ? LIMIT 1", (rule_id,), ) @@ -271,8 +271,8 @@ class ScheduleRulesDao: return result.rowcount > 0 -class TaskDefinitionsDao: - """Typed async access to the ``task_definitions`` table. +class QuestDefinitionsDao: + """Typed async access to the ``quest_definitions`` table. No delete method exists: definitions are deactivated via :meth:`set_active`, never hard-deleted, so completion history keeps @@ -293,7 +293,7 @@ class TaskDefinitionsDao: icon: str | None = None, due_time: str | None = None, is_active: bool = True, - ) -> TaskDefinitionRecord: + ) -> QuestDefinitionRecord: """Insert one definition and return the record as stored. Validates the assignment target as part of the same serialized @@ -312,7 +312,7 @@ class TaskDefinitionsDao: child_id, schedule_rule_id ) result = await self._database.execute( - "INSERT INTO task_definitions (title, description, " + "INSERT INTO quest_definitions (title, description, " "icon, child_id, schedule_rule_id, due_time, " "is_active, created_at) " "VALUES (?, ?, ?, ?, ?, ?, ?, ?)", @@ -349,28 +349,28 @@ class TaskDefinitionsDao: f"schedule rule {schedule_rule_id} does not exist" ) - async def get(self, definition_id: int) -> TaskDefinitionRecord | None: + async def get(self, definition_id: int) -> QuestDefinitionRecord | None: """Return the definition with ``definition_id``, or None.""" row = await self._database.fetch_one( - f"SELECT {_DEFINITION_COLUMNS} FROM task_definitions " + f"SELECT {_DEFINITION_COLUMNS} FROM quest_definitions " "WHERE id = ?", (definition_id,), ) return _definition_from_row(row) if row is not None else None - async def list_by_child(self, child_id: int) -> list[TaskDefinitionRecord]: + async def list_by_child(self, child_id: int) -> list[QuestDefinitionRecord]: """Return all definitions assigned to ``child_id``, newest first.""" rows = await self._database.fetch_all( - f"SELECT {_DEFINITION_COLUMNS} FROM task_definitions " + f"SELECT {_DEFINITION_COLUMNS} FROM quest_definitions " "WHERE child_id = ? ORDER BY id DESC", (child_id,), ) return [_definition_from_row(row) for row in rows] - async def list_active(self) -> list[TaskDefinitionRecord]: + async def list_active(self) -> list[QuestDefinitionRecord]: """Return all active definitions, oldest first (stable order).""" rows = await self._database.fetch_all( - f"SELECT {_DEFINITION_COLUMNS} FROM task_definitions " + f"SELECT {_DEFINITION_COLUMNS} FROM quest_definitions " "WHERE is_active = 1 ORDER BY id" ) return [_definition_from_row(row) for row in rows] @@ -409,7 +409,7 @@ class TaskDefinitionsDao: return 0 parameters.append(definition_id) result = await self._database.execute( - f"UPDATE task_definitions SET {', '.join(assignments)} " + f"UPDATE quest_definitions SET {', '.join(assignments)} " "WHERE id = ?", tuple(parameters), ) @@ -422,7 +422,7 @@ class TaskDefinitionsDao: existing instances and completion history untouched. """ result = await self._database.execute( - "UPDATE task_definitions SET is_active = ? WHERE id = ?", + "UPDATE quest_definitions SET is_active = ? WHERE id = ?", (int(is_active), definition_id), ) return result.rowcount @@ -442,7 +442,7 @@ class TaskDefinitionsDao: async with self._database.transaction(): await self._validate_assignable_child(child_id) result = await self._database.execute( - "UPDATE task_definitions SET child_id = ? " + "UPDATE quest_definitions SET child_id = ? " "WHERE id = ?", (child_id, definition_id), ) diff --git a/custom_components/nestquest/migrations.py b/custom_components/nestquest/migrations.py index 0cc4377..1571739 100644 --- a/custom_components/nestquest/migrations.py +++ b/custom_components/nestquest/migrations.py @@ -14,6 +14,22 @@ Version 0 means "the file exists but carries no NestQuest schema yet" the runner against version 0 applies migration 1, which applies the full v1 DDL from :mod:`.schema` and stamps version 1. +Migration 1 was rewritten pre-release (D-007): nothing was ever +shipped, so the v1 DDL now creates the ``quest_*`` tables directly +instead of the legacy ``task_*`` spellings, and no compatibility shim +exists. Dev machines that already hold a version-1 database with +``task_*`` tables are handled by migration 2, which detects the legacy +table names and ``ALTER TABLE ... RENAME TO``s them to their current +names before anything else assumes the new model. The rename step runs +inside the same transaction as its version stamp, so a crash mid-rename +rolls both back together. + +Migration entries are either an ordered sequence of SQL statements or +an async callable taking the open database. Callables exist for steps +whose SQL depends on the database's current state (the legacy rename +only fires when a legacy table is actually present) and run under the +same per-migration transaction as statement lists. + Each migration's statements and its version stamp run inside ONE transaction, and the stamp is written as DELETE + INSERT rather than UPDATE because a version-0 database has no version row to update yet. @@ -43,13 +59,18 @@ from __future__ import annotations import asyncio import logging -from collections.abc import Sequence +from collections.abc import Awaitable, Callable, Sequence +from typing import Union from .db import NestQuestDatabase from .schema import SCHEMA_V1_STATEMENTS LOGGER = logging.getLogger(__name__) +#: One migration entry: either an ordered sequence of SQL statements or +#: an async callable run under the entry's migration transaction. +MigrationStep = Union[Sequence[str], Callable[[NestQuestDatabase], Awaitable[None]]] + #: Metadata table holding the applied schema version. Prefixed with the #: integration domain so a config directory shared with other tools can #: never collide with a foreign ``schema_version`` table. @@ -73,10 +94,48 @@ VERSION_TABLE_DDL = f""" #: version ``n-1`` to version ``n``. MIGRATION_1_V1_DDL: list[str] = list(SCHEMA_V1_STATEMENTS) +#: Legacy (pre-D-007) table name -> current table name. Order matters: +#: the parent table renames first so the child's foreign key reference +#: is rewritten while its parent's new name is already in place. +LEGACY_TASK_TABLE_RENAMES: tuple[tuple[str, str], ...] = ( + ("task_definitions", "quest_definitions"), + ("task_instances", "quest_instances"), +) + + +async def _rename_legacy_task_tables(database: NestQuestDatabase) -> None: + """Rename any surviving ``task_*`` tables to their ``quest_*`` names. + + Dev-machines-only path (D-007): a database stamped version 1 by the + pre-rename runner holds ``task_definitions``/``task_instances``. + Each legacy name still present is renamed; fresh databases find + nothing and this step is a no-op. Modern SQLite rewrites foreign + key clauses in other tables to follow the rename, so the renamed + schema is indistinguishable from a freshly created one apart from + internal autoindex names. Runs inside the migration transaction, + so the renames and the version stamp commit or roll back together. + """ + for legacy_name, current_name in LEGACY_TASK_TABLE_RENAMES: + row = await database.fetch_one( + "SELECT 1 FROM sqlite_master WHERE type = 'table' " + "AND name = ?", + (legacy_name,), + ) + if row is not None: + await database.execute( + f'ALTER TABLE "{legacy_name}" RENAME TO "{current_name}"' + ) + LOGGER.info( + "Renamed legacy table %s to %s", legacy_name, current_name + ) + + #: The ordered migration list. Append-only: never edit an applied -#: entry, add the next one instead. -MIGRATIONS: Sequence[Sequence[str]] = [ +#: entry, add the next one instead. (Migration 1's content was +#: rewritten pre-release per D-007 — no version 1 database shipped.) +MIGRATIONS: Sequence[MigrationStep] = [ MIGRATION_1_V1_DDL, + _rename_legacy_task_tables, ] @@ -140,7 +199,7 @@ def _migration_lock(database: NestQuestDatabase) -> asyncio.Lock: async def apply_migrations( database: NestQuestDatabase, - migrations: Sequence[Sequence[str]] = MIGRATIONS, + migrations: Sequence[MigrationStep] = MIGRATIONS, ) -> int: """Bring the database schema up to the latest version. @@ -208,8 +267,14 @@ async def apply_migrations( f"{latest}); downgrades are not supported" ) if in_tx_version < target: - for sql in migrations[target - 1]: - await database.execute(sql) + step = migrations[target - 1] + if callable(step): + await step(database) + applied_statements = 0 + else: + for sql in step: + await database.execute(sql) + applied_statements = len(step) # INSERT keyed on the singleton row (id = 1): a # fresh file has no row yet, and migration 1 is the # only migration a version-0 database can run, so @@ -225,12 +290,18 @@ async def apply_migrations( ) applied = True if applied: - LOGGER.info( - "NestQuest schema migrated to version %d " - "(%d statement%s)", - target, - len(migrations[target - 1]), - "s" if len(migrations[target - 1]) != 1 else "", - ) + if applied_statements == 0: + LOGGER.info( + "NestQuest schema migrated to version %d", + target, + ) + else: + LOGGER.info( + "NestQuest schema migrated to version %d " + "(%d statement%s)", + target, + applied_statements, + "s" if applied_statements != 1 else "", + ) return latest \ No newline at end of file diff --git a/custom_components/nestquest/schema.py b/custom_components/nestquest/schema.py index f76cf6f..d497358 100644 --- a/custom_components/nestquest/schema.py +++ b/custom_components/nestquest/schema.py @@ -86,7 +86,7 @@ date range; a single-day override stores the same date in both columns. ``is_present`` 0/1 marks the child absent/present for the whole range; ``note`` is optional free text. -Task instances: ``task_instances`` deliberately carries NO completion +Quest instances: ``quest_instances`` deliberately carries NO completion status column. An instance's current state derives from the latest row in ``completion_events`` (Feature 08), so materialization (Feature 07) can insert instances without knowing anything about completion and the @@ -103,7 +103,7 @@ stamps one coherent time). Uniqueness: the (definition_id, due_date) pair is declared as a table-level ``UNIQUE`` constraint rather than a separate ``CREATE UNIQUE INDEX`` statement. SQLite materializes it as an implicit unique -index (``sqlite_autoindex_task_instances_1``) — the required "unique +index (``sqlite_autoindex_quest_instances_1``) — the required "unique index prevents duplicate instances" — and the constraint form cannot be silently dropped the way a standalone index can, so the duplicate door stays shut. Idempotent materialization keys on exactly this pair. No @@ -128,7 +128,7 @@ representable — missed is a derived state announced on the HA event bus (Feature 10), never an appended event. ``child_id`` is deliberately denormalized (it is also derivable via -``task_instances``): history reporting (Feature 13) filters and rates +``quest_instances``): history reporting (Feature 13) filters and rates per child directly against the log, and the log should stay self-describing without joins. @@ -219,9 +219,9 @@ SCHEMA_V1_SCHEDULE_RULES_DDL: list[str] = [ """, ] -SCHEMA_V1_TASK_DEFINITIONS_DDL: list[str] = [ +SCHEMA_V1_QUEST_DEFINITIONS_DDL: list[str] = [ """ - CREATE TABLE IF NOT EXISTS task_definitions ( + CREATE TABLE IF NOT EXISTS quest_definitions ( id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT NOT NULL, description TEXT, @@ -271,11 +271,11 @@ SCHEMA_V1_PRESENCE_OVERRIDES_DDL: list[str] = [ """, ] -SCHEMA_V1_TASK_INSTANCES_DDL: list[str] = [ +SCHEMA_V1_QUEST_INSTANCES_DDL: list[str] = [ """ - CREATE TABLE IF NOT EXISTS task_instances ( + CREATE TABLE IF NOT EXISTS quest_instances ( id INTEGER PRIMARY KEY AUTOINCREMENT, - definition_id INTEGER NOT NULL REFERENCES task_definitions(id), + definition_id INTEGER NOT NULL REFERENCES quest_definitions(id), child_id INTEGER NOT NULL REFERENCES children(id), due_date TEXT NOT NULL, due_time TEXT, @@ -289,7 +289,7 @@ SCHEMA_V1_COMPLETION_EVENTS_DDL: list[str] = [ """ CREATE TABLE IF NOT EXISTS completion_events ( id INTEGER PRIMARY KEY AUTOINCREMENT, - instance_id INTEGER NOT NULL REFERENCES task_instances(id), + instance_id INTEGER NOT NULL REFERENCES quest_instances(id), child_id INTEGER NOT NULL REFERENCES children(id), event_type TEXT NOT NULL CHECK (event_type IN ('completed', 'uncompleted')), actor_source TEXT NOT NULL CHECK (actor_source IN ('user', 'panel')), @@ -315,10 +315,10 @@ SCHEMA_V1_STATEMENTS: list[str] = [ *SCHEMA_V1_CHILDREN_DDL, *SCHEMA_V1_ADMIN_USERS_DDL, *SCHEMA_V1_SCHEDULE_RULES_DDL, - *SCHEMA_V1_TASK_DEFINITIONS_DDL, + *SCHEMA_V1_QUEST_DEFINITIONS_DDL, *SCHEMA_V1_PRESENCE_SCHEDULES_DDL, *SCHEMA_V1_PRESENCE_OVERRIDES_DDL, - *SCHEMA_V1_TASK_INSTANCES_DDL, + *SCHEMA_V1_QUEST_INSTANCES_DDL, *SCHEMA_V1_COMPLETION_EVENTS_DDL, ] diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index f0dad52..03ee03f 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -1,4 +1,4 @@ -"""Tests for dao_instances.py: task_instances + append-only completion_events.""" +"""Tests for dao_instances.py: quest_instances + append-only completion_events.""" from __future__ import annotations import asyncio @@ -11,10 +11,10 @@ from custom_components.nestquest.dao_children import ChildrenDao from custom_components.nestquest.dao_instances import ( CompletionEventRecord, CompletionEventsDao, - TaskInstanceRecord, - TaskInstancesDao, + QuestInstanceRecord, + QuestInstancesDao, ) -from custom_components.nestquest.dao_rules import ScheduleRulesDao, TaskDefinitionsDao +from custom_components.nestquest.dao_rules import ScheduleRulesDao, QuestDefinitionsDao from custom_components.nestquest.db import NestQuestDatabase from custom_components.nestquest.migrations import apply_migrations @@ -76,8 +76,8 @@ async def _prepare(path) -> tuple: await apply_migrations(database) children = ChildrenDao(database) rules = ScheduleRulesDao(database) - definitions = TaskDefinitionsDao(database) - instances = TaskInstancesDao(database) + definitions = QuestDefinitionsDao(database) + instances = QuestInstancesDao(database) events = CompletionEventsDao(database) child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) @@ -111,7 +111,7 @@ def _with_db(tmp_path, name): # --------------------------------------------------------------------------- -# task_instances: upsert idempotency +# quest_instances: upsert idempotency # --------------------------------------------------------------------------- @@ -121,13 +121,13 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: first = await instances.upsert( definition.id, child.id, D1, _now_stamp() ) - assert isinstance(first, TaskInstanceRecord) + assert isinstance(first, QuestInstanceRecord) second = await instances.upsert( definition.id, child.id, D1, _now_stamp() ) assert second.id == first.id rows = await database.fetch_one( - "SELECT COUNT(*) FROM task_instances" + "SELECT COUNT(*) FROM quest_instances" ) assert rows == (1,) return second @@ -323,8 +323,8 @@ def test_upsert_no_past_check_uses_execution_date_not_call_date( await apply_migrations(database) children = ChildrenDao(database) rules = ScheduleRulesDao(database) - definitions = TaskDefinitionsDao(database) - instances = TaskInstancesDao(database) + definitions = QuestDefinitionsDao(database) + instances = QuestInstancesDao(database) events = CompletionEventsDao(database) child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) @@ -369,7 +369,7 @@ def test_upsert_refuses_immutable_completed_instance(tmp_path) -> None: # --------------------------------------------------------------------------- -# task_instances: get / list +# quest_instances: get / list # --------------------------------------------------------------------------- @@ -453,7 +453,7 @@ def test_list_by_date_range_rejects_inverted_range(tmp_path) -> None: # --------------------------------------------------------------------------- -# task_instances: delete_future_uncompleted +# quest_instances: delete_future_uncompleted # --------------------------------------------------------------------------- @@ -1084,8 +1084,8 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: async def _prepare_in(database): children = ChildrenDao(database) rules = ScheduleRulesDao(database) - definitions = TaskDefinitionsDao(database) - instances = TaskInstancesDao(database) + definitions = QuestDefinitionsDao(database) + instances = QuestInstancesDao(database) events = CompletionEventsDao(database) child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) diff --git a/tests/test_dao_matrix.py b/tests/test_dao_matrix.py index c68b7a1..c14e65b 100644 --- a/tests/test_dao_matrix.py +++ b/tests/test_dao_matrix.py @@ -27,7 +27,7 @@ from custom_components.nestquest.dao_children import ( ) from custom_components.nestquest.dao_instances import ( CompletionEventsDao, - TaskInstancesDao, + QuestInstancesDao, ) from custom_components.nestquest.dao_presence import ( PresenceOverridesDao, @@ -35,7 +35,7 @@ from custom_components.nestquest.dao_presence import ( ) from custom_components.nestquest.dao_rules import ( ScheduleRulesDao, - TaskDefinitionsDao, + QuestDefinitionsDao, ) from custom_components.nestquest.db import NestQuestDatabase from custom_components.nestquest.migrations import apply_migrations @@ -70,10 +70,10 @@ ALL_TABLES = ( "children", "admin_users", "schedule_rules", - "task_definitions", + "quest_definitions", "presence_schedules", "presence_overrides", - "task_instances", + "quest_instances", "completion_events", ) @@ -86,10 +86,10 @@ class World: self.children = ChildrenDao(database) self.admins = AdminUsersDao(database) self.rules = ScheduleRulesDao(database) - self.definitions = TaskDefinitionsDao(database) + self.definitions = QuestDefinitionsDao(database) self.schedules = PresenceSchedulesDao(database) self.overrides = PresenceOverridesDao(database) - self.instances = TaskInstancesDao(database) + self.instances = QuestInstancesDao(database) self.events = CompletionEventsDao(database) async def seed(self) -> None: @@ -201,7 +201,7 @@ def test_matrix_schedule_rules_crud_and_constraints(tmp_path) -> None: assert _with_world(tmp_path / "m-rules.db")(_body) is True -def test_matrix_task_definitions_crud_and_constraints(tmp_path) -> None: +def test_matrix_quest_definitions_crud_and_constraints(tmp_path) -> None: async def _body(w: World): # insert (seeded) + read fetched = await w.definitions.get(w.definition.id) @@ -271,7 +271,7 @@ def test_matrix_presence_overrides_crud_and_constraints(tmp_path) -> None: assert _with_world(tmp_path / "m-overrides.db")(_body) is True -def test_matrix_task_instances_crud_and_constraints(tmp_path) -> None: +def test_matrix_quest_instances_crud_and_constraints(tmp_path) -> None: async def _body(w: World): # insert (seeded) + read fetched = await w.instances.get(w.definition.id, D1) @@ -292,20 +292,20 @@ def test_matrix_task_instances_crud_and_constraints(tmp_path) -> None: assert _with_world(tmp_path / "m-instances.db")(_body) is True -def test_matrix_task_instances_constraints(tmp_path) -> None: +def test_matrix_quest_instances_constraints(tmp_path) -> None: async def _body(w: World): # D20 has no instance yet: a raw insert succeeds, then an # identical second insert must fail on the UNIQUE constraint — # proving the schema backs the DAO's idempotency. await w.database.execute( - "INSERT INTO task_instances (definition_id, child_id, " + "INSERT INTO quest_instances (definition_id, child_id, " "due_date, due_time, generated_at) " "VALUES (?, ?, ?, NULL, ?)", (w.definition.id, w.child.id, D20, _stamp()), ) with pytest.raises(sqlite3.IntegrityError): await w.database.execute( - "INSERT INTO task_instances (definition_id, child_id, " + "INSERT INTO quest_instances (definition_id, child_id, " "due_date, due_time, generated_at) " "VALUES (?, ?, ?, NULL, ?)", (w.definition.id, w.child.id, D20, _stamp()), @@ -320,7 +320,7 @@ def test_matrix_task_instances_constraints(tmp_path) -> None: assert _with_world(tmp_path / "m-instances-constraints.db")(_body) is True -def test_matrix_task_instances_upsert_idempotency(tmp_path) -> None: +def test_matrix_quest_instances_upsert_idempotency(tmp_path) -> None: async def _body(w: World): first = await w.instances.upsert( w.definition.id, w.child.id, D2, _stamp() @@ -330,7 +330,7 @@ def test_matrix_task_instances_upsert_idempotency(tmp_path) -> None: ) assert second.id == first.id count = await w.database.fetch_one( - "SELECT COUNT(*) FROM task_instances" + "SELECT COUNT(*) FROM quest_instances" ) # Seed created one (D1) plus this one (D2) — no duplicate. assert count == (2,) diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index a27966e..4d3bf04 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -1,4 +1,4 @@ -"""Tests for dao_rules.py: typed DAO for schedule_rules and task_definitions.""" +"""Tests for dao_rules.py: typed DAO for schedule_rules and quest_definitions.""" from __future__ import annotations import asyncio @@ -10,8 +10,8 @@ from custom_components.nestquest.dao_children import ChildrenDao from custom_components.nestquest.dao_rules import ( ScheduleRuleRecord, ScheduleRulesDao, - TaskDefinitionRecord, - TaskDefinitionsDao, + QuestDefinitionRecord, + QuestDefinitionsDao, ) from custom_components.nestquest.db import NestQuestDatabase from custom_components.nestquest.migrations import apply_migrations @@ -37,7 +37,7 @@ async def _prepare(path) -> tuple: await database.open(path) await apply_migrations(database) rules = ScheduleRulesDao(database) - definitions = TaskDefinitionsDao(database) + definitions = QuestDefinitionsDao(database) children = ChildrenDao(database) child = await children.create("Ada", NOW) return database, rules, definitions, children, child @@ -322,7 +322,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( await apply_migrations(database) children = ChildrenDao(database) rules = ScheduleRulesDao(database) - definitions = TaskDefinitionsDao(database) + definitions = QuestDefinitionsDao(database) child = await children.create("Ada", NOW) rule_a = await rules.create("daily", "2026-09-14") @@ -335,7 +335,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( import custom_components.nestquest.dao_rules as dao_rules original_validate = ( - dao_rules.TaskDefinitionsDao._validate_assignable + dao_rules.QuestDefinitionsDao._validate_assignable ) release = asyncio.Event() started = asyncio.Event() @@ -353,7 +353,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( started.set() await release.wait() - dao_rules.TaskDefinitionsDao._validate_assignable = ( + dao_rules.QuestDefinitionsDao._validate_assignable = ( _pausing_validate ) try: @@ -373,7 +373,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( delete_task, create_task, return_exceptions=True ) finally: - dao_rules.TaskDefinitionsDao._validate_assignable = ( + dao_rules.QuestDefinitionsDao._validate_assignable = ( original_validate ) else: @@ -405,7 +405,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( assert rule_row is None else: # Create won: the delete was rejected, both rows exist. - assert isinstance(creation, TaskDefinitionRecord), ( + assert isinstance(creation, QuestDefinitionRecord), ( f"create must succeed, got {creation!r}" ) assert rule_row is not None @@ -425,7 +425,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( # --------------------------------------------------------------------------- -# task_definitions: create / get +# quest_definitions: create / get # --------------------------------------------------------------------------- @@ -441,7 +441,7 @@ def test_definition_create_returns_typed_record(tmp_path) -> None: icon="mdi:tooth", due_time="08:00", ) - assert isinstance(definition, TaskDefinitionRecord) + assert isinstance(definition, QuestDefinitionRecord) assert definition.title == "Brush teeth" assert definition.child_id == child.id assert definition.schedule_rule_id == rule.id @@ -497,7 +497,7 @@ def test_definition_create_unknown_rule_raises_value_error( # --------------------------------------------------------------------------- -# task_definitions: list by child / list active +# quest_definitions: list by child / list active # --------------------------------------------------------------------------- @@ -539,7 +539,7 @@ def test_definition_list_active_excludes_inactive(tmp_path) -> None: # --------------------------------------------------------------------------- -# task_definitions: update / set_active / set_assignee +# quest_definitions: update / set_active / set_assignee # --------------------------------------------------------------------------- @@ -673,7 +673,7 @@ def test_definition_no_delete_method_exists(tmp_path) -> None: methods = { name for name, _ in inspect.getmembers( - TaskDefinitionsDao, inspect.isfunction + QuestDefinitionsDao, inspect.isfunction ) } assert "delete" not in methods @@ -748,13 +748,13 @@ def test_definition_set_assignee_leaves_history_rows_alone( # Simulate an already-generated instance (materialization owns # this table, but its row shape proves reassignment isolation). await database.execute( - "INSERT INTO task_instances (definition_id, child_id, " + "INSERT INTO quest_instances (definition_id, child_id, " "due_date, generated_at) VALUES (?, ?, ?, ?)", (definition.id, child.id, "2026-09-15", NOW), ) await definitions.set_assignee(definition.id, other.id) row = await database.fetch_one( - "SELECT child_id FROM task_instances WHERE definition_id = ?", + "SELECT child_id FROM quest_instances WHERE definition_id = ?", (definition.id,), ) assert row == (child.id,) @@ -769,7 +769,7 @@ def test_definition_set_assignee_leaves_history_rows_alone( def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: - """Guardrail: schedule_rules/task_definitions SQL may appear only + """Guardrail: schedule_rules/quest_definitions SQL may appear only in the DAO modules (and the schema DDL declarations). RECURSIVE scan of package and tests, FROM/INTO/UPDATE/DELETE FROM/JOIN pattern, with exact repo-relative exclusions justified by role: @@ -801,7 +801,7 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: } sql_pattern = re.compile( r"(FROM|INTO|UPDATE|DELETE\s+FROM|JOIN)\s+[`'\"]*(\[)?" - r"(schedule_rules|task_definitions)\b", + r"(schedule_rules|quest_definitions)\b", re.IGNORECASE, ) offenders: list[str] = [] @@ -813,16 +813,16 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: if sql_pattern.search(py.read_text()): offenders.append(relative) assert offenders == [], ( - f"SQL touching schedule_rules/task_definitions leaked into: " + f"SQL touching schedule_rules/quest_definitions leaked into: " f"{offenders}" ) def test_dao_instances_test_file_uses_dao_not_raw_rules_sql() -> None: """Compensating self-scan for the test_dao_instances.py exemption: - that file may query task_instances/completion_events raw (its own + that file may query quest_instances/completion_events raw (its own guard's scope) but must go through the DAO for schedule_rules and - task_definitions, except its single sanctioned definition-create + quest_definitions, except its single sanctioned definition-create helper usage and guard spans. """ import ast @@ -853,12 +853,12 @@ def test_dao_instances_test_file_uses_dao_not_raw_rules_sql() -> None: ) sql_pattern = re.compile( r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" - r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|task_definitions)\b", + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions)\b", re.IGNORECASE, ) assert sql_pattern.search(remaining) is None, ( "test_dao_instances.py must go through the DAO, not raw SQL, " - "for schedule_rules/task_definitions" + "for schedule_rules/quest_definitions" ) @@ -866,7 +866,7 @@ def test_dao_rules_test_file_uses_dao_not_raw_table_sql() -> None: """This test module must exercise the DAO, not raw SQL, for these tables. Exceptions: the guard functions' own spans (their regexes mention the names), and the reassignment-isolation test, which - legitimately touches task_instances — a table this DAO does not + legitimately touches quest_instances — a table this DAO does not own — to prove the guardrail that reassignment leaves it alone. """ import ast @@ -898,7 +898,7 @@ def test_dao_rules_test_file_uses_dao_not_raw_table_sql() -> None: ) sql_pattern = re.compile( r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" - r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|task_definitions)\b", + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions)\b", re.IGNORECASE, ) assert sql_pattern.search(remaining) is None, ( diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 246f8b5..c3e0f17 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -247,12 +247,12 @@ async def test_setup_applies_all_v1_tables(hass, make_entry) -> None: rows = await database.fetch_all( "SELECT name FROM sqlite_master WHERE type = 'table' " "AND name IN ('children', 'admin_users', 'schedule_rules', " - "'task_definitions') ORDER BY name" + "'quest_definitions') ORDER BY name" ) assert [row[0] for row in rows] == [ "admin_users", "children", + "quest_definitions", "schedule_rules", - "task_definitions", ] await async_unload_entry(hass, entry) \ No newline at end of file diff --git a/tests/test_migrations.py b/tests/test_migrations.py index a96226f..f8ac70d 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -62,10 +62,10 @@ def _counts(database) -> dict[str, int]: "children", "admin_users", "schedule_rules", - "task_definitions", + "quest_definitions", "presence_schedules", "presence_overrides", - "task_instances", + "quest_instances", "completion_events", ) counts = {} @@ -76,16 +76,16 @@ def _counts(database) -> dict[str, int]: # --------------------------------------------------------------------------- -# Fresh file: version 0 -> version 1 +# Fresh file: version 0 -> latest version # --------------------------------------------------------------------------- -def test_fresh_file_migrates_to_version_1(tmp_path) -> None: +def test_fresh_file_migrates_to_latest_version(tmp_path) -> None: database = _open_db(tmp_path / "fresh.db") try: final = _migrate(database) - assert final == 1 - assert _version(database) == 1 + assert final == len(MIGRATIONS) + assert _version(database) == len(MIGRATIONS) finally: _run(database.close()) @@ -98,10 +98,10 @@ def test_fresh_file_migration_creates_all_v1_tables(tmp_path) -> None: "children", "admin_users", "schedule_rules", - "task_definitions", + "quest_definitions", "presence_schedules", "presence_overrides", - "task_instances", + "quest_instances", "completion_events", VERSION_TABLE, } @@ -135,9 +135,249 @@ def test_empty_version_table_reads_as_version_0(tmp_path) -> None: _run(database.close()) -def test_migration_1_is_the_full_v1_ddl() -> None: +def test_migration_list_shape() -> None: + """Migration 1 is the v1 DDL; migration 2 is the legacy rename step. + + Nothing ever shipped, so migration 1 was rewritten pre-release + (D-007) to create the quest_* tables directly; migration 2 exists + for dev databases stamped version 1 by the pre-rename runner. + """ assert MIGRATIONS[0] == SCHEMA_V1_STATEMENTS - assert len(MIGRATIONS) == 1 + assert callable(MIGRATIONS[1]) + assert len(MIGRATIONS) == 2 + + +# --------------------------------------------------------------------------- +# Legacy dev database: task_* tables stamped version 1 -> quest_* +# --------------------------------------------------------------------------- + + +_LEGACY_DDL = [ + """ + CREATE TABLE IF NOT EXISTS children ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + display_name TEXT NOT NULL, + colour TEXT, + avatar_ref TEXT, + sort_order INTEGER NOT NULL DEFAULT 0, + is_active INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL + ) + """, + """ + CREATE TABLE IF NOT EXISTS admin_users ( + ha_user_id TEXT PRIMARY KEY NOT NULL, + added_at TEXT NOT NULL + ) + """, + """ + CREATE TABLE IF NOT EXISTS schedule_rules ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + rule_type TEXT NOT NULL, + interval INTEGER NOT NULL DEFAULT 1, + weekday_set TEXT, + day_of_month INTEGER, + nth_weekday INTEGER, + month INTEGER, + start_date TEXT NOT NULL, + end_date TEXT + ) + """, + """ + CREATE TABLE IF NOT EXISTS task_definitions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + title TEXT NOT NULL, + description TEXT, + icon TEXT, + child_id INTEGER NOT NULL REFERENCES children(id), + schedule_rule_id INTEGER NOT NULL REFERENCES schedule_rules(id), + due_time TEXT, + is_active INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL + ) + """, + """ + CREATE TABLE IF NOT EXISTS presence_schedules ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + child_id INTEGER NOT NULL UNIQUE REFERENCES children(id), + cycle_length_weeks INTEGER NOT NULL, + anchor_date TEXT NOT NULL, + pattern TEXT NOT NULL + ) + """, + """ + CREATE TABLE IF NOT EXISTS presence_overrides ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + child_id INTEGER NOT NULL REFERENCES children(id), + start_date TEXT NOT NULL, + end_date TEXT NOT NULL, + is_present INTEGER NOT NULL, + note TEXT + ) + """, + """ + CREATE TABLE IF NOT EXISTS task_instances ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + definition_id INTEGER NOT NULL REFERENCES task_definitions(id), + child_id INTEGER NOT NULL REFERENCES children(id), + due_date TEXT NOT NULL, + due_time TEXT, + generated_at TEXT NOT NULL, + UNIQUE (definition_id, due_date) + ) + """, + """ + CREATE TABLE IF NOT EXISTS completion_events ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + instance_id INTEGER NOT NULL REFERENCES task_instances(id), + child_id INTEGER NOT NULL REFERENCES children(id), + event_type TEXT NOT NULL, + actor_source TEXT NOT NULL, + actor_user_id TEXT, + occurred_at TEXT NOT NULL, + was_on_time INTEGER + ) + """, +] + + +def _seed_legacy_database(database) -> None: + """Create a database indistinguishable from a pre-rename dev file. + + Applies the legacy task_* DDL, seeds one row in every renamed + table, and stamps version 1 exactly the way the old runner did. + """ + for sql in _LEGACY_DDL: + _run(database.execute(sql)) + _run(database.execute(VERSION_TABLE_DDL)) + _run( + database.execute( + f"INSERT INTO {VERSION_TABLE} (id, version) VALUES (1, 1)" + ) + ) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) " + "VALUES ('Ada', '2026-09-14T00:00:00+00:00')" + ) + ) + _run( + database.execute( + "INSERT INTO schedule_rules (rule_type, start_date) " + "VALUES ('daily', '2026-09-14')" + ) + ) + _run( + database.execute( + "INSERT INTO task_definitions (title, child_id, " + "schedule_rule_id, created_at) " + "VALUES ('Dishes', 1, 1, '2026-09-14T00:00:00+00:00')" + ) + ) + _run( + database.execute( + "INSERT INTO task_instances (definition_id, child_id, " + "due_date, generated_at) " + "VALUES (1, 1, '2026-09-15', '2026-09-14T00:00:00+00:00')" + ) + ) + + +def test_legacy_task_tables_are_renamed_and_keep_their_rows(tmp_path) -> None: + database = _open_db(tmp_path / "legacy.db") + try: + _seed_legacy_database(database) + final = _migrate(database) + assert final == len(MIGRATIONS) + assert _version(database) == len(MIGRATIONS) + tables = _tables(database) + assert "task_definitions" not in tables + assert "task_instances" not in tables + assert {"quest_definitions", "quest_instances"} <= tables + # Every seeded row survived the rename. + assert _run(database.fetch_one("SELECT COUNT(*) FROM children")) == (1,) + assert ( + _run( + database.fetch_one( + "SELECT title FROM quest_definitions WHERE id = 1" + ) + ) + == ("Dishes",) + ) + assert ( + _run( + database.fetch_one( + "SELECT due_date FROM quest_instances WHERE id = 1" + ) + ) + == ("2026-09-15",) + ) + # The untouched completion_events table still exists and is empty. + assert "completion_events" in tables + assert ( + _run(database.fetch_one("SELECT COUNT(*) FROM completion_events")) + == (0,) + ) + finally: + _run(database.close()) + + +def test_renamed_instances_foreign_key_follows_the_rename(tmp_path) -> None: + database = _open_db(tmp_path / "legacy-fk.db") + try: + _seed_legacy_database(database) + _migrate(database) + fks = _run(database.fetch_all("PRAGMA foreign_key_list(quest_instances)")) + referenced = {row[2] for row in fks} + assert "quest_definitions" in referenced + assert "task_definitions" not in referenced + # The renamed table is live: a new row validates its FKs. + _run( + database.execute( + "INSERT INTO children (display_name, created_at) " + "VALUES ('Ben', '2026-09-14T00:00:00+00:00')" + ) + ) + with pytest.raises(sqlite3.IntegrityError): + _run( + database.execute( + "INSERT INTO quest_instances (definition_id, child_id, " + "due_date, generated_at) " + "VALUES (999, 2, '2026-09-16', " + "'2026-09-14T00:00:00+00:00')" + ) + ) + finally: + _run(database.close()) + + +def test_legacy_rename_is_idempotent_across_restarts(tmp_path) -> None: + database = _open_db(tmp_path / "legacy-restart.db") + try: + _seed_legacy_database(database) + _migrate(database) + final = _migrate(database) + assert final == len(MIGRATIONS) + tables = _tables(database) + assert "quest_definitions" in tables + assert "task_definitions" not in tables + assert ( + _run(database.fetch_one("SELECT COUNT(*) FROM quest_definitions")) + == (1,) + ) + finally: + _run(database.close()) + + +def test_fresh_database_never_holds_legacy_table_names(tmp_path) -> None: + database = _open_db(tmp_path / "fresh-legacy.db") + try: + _migrate(database) + tables = _tables(database) + assert not any(name.startswith("task_") for name in tables) + assert _version(database) == len(MIGRATIONS) + finally: + _run(database.close()) # --------------------------------------------------------------------------- @@ -162,8 +402,8 @@ def test_second_run_applies_nothing_and_keeps_rows(tmp_path) -> None: ) ) final = _migrate(database) - assert final == 1 - assert _version(database) == 1 + assert final == len(MIGRATIONS) + assert _version(database) == len(MIGRATIONS) assert _counts(database)["children"] == 1 assert _counts(database)["admin_users"] == 1 finally: @@ -228,7 +468,7 @@ def test_noop_run_is_logged_at_debug_level(caplog, tmp_path) -> None: with caplog.at_level(logging.DEBUG, logger="custom_components.nestquest.migrations"): _migrate(database) assert any( - "already at version 1" in r.getMessage() + f"already at version {len(MIGRATIONS)}" in r.getMessage() for r in caplog.records ) finally: @@ -241,9 +481,17 @@ def test_noop_run_is_logged_at_debug_level(caplog, tmp_path) -> None: def _failing_migrations(prior_statements: list[str]) -> list[list[str]]: - """Two-migration list where migration 2 fails mid-application.""" + """Three-migration list where migration 3 fails mid-application. + + Migration 2 is a harmless no-op filler so the real runner — which + has already stamped version 2 by the time these tests re-run it — + still has a pending migration to fail on. + """ return [ prior_statements, + [ + "CREATE TABLE IF NOT EXISTS filler_table (id INTEGER PRIMARY KEY)" + ], [ "CREATE TABLE IF NOT EXISTS later_table (id INTEGER PRIMARY KEY)", "CREATE TABLE broken (", @@ -254,20 +502,20 @@ def _failing_migrations(prior_statements: list[str]) -> list[list[str]]: def test_failing_migration_rolls_back_to_prior_version(tmp_path) -> None: database = _open_db(tmp_path / "rollback.db") try: - # Manually simulate a database at version 1: apply migration 1's - # statements directly and stamp version 1 through the runner. + # Manually simulate a fully-migrated database: run the real + # migration list, which stamps the latest version. _migrate(database) version_row = _run( database.fetch_one(f"SELECT version FROM {VERSION_TABLE}") ) - assert version_row == (1,) + assert version_row == (len(MIGRATIONS),) with pytest.raises(sqlite3.OperationalError): _migrate(database, _failing_migrations(SCHEMA_V1_STATEMENTS)) # Version stamp unchanged, statements from the failed migration # absent, and the v1 tables untouched. - assert _version(database) == 1 + assert _version(database) == len(MIGRATIONS) assert "later_table" not in _tables(database) assert _counts(database)["children"] == 0 assert _counts(database)["completion_events"] == 0 @@ -275,21 +523,24 @@ def test_failing_migration_rolls_back_to_prior_version(tmp_path) -> None: _run(database.close()) -def test_failing_migration_on_fresh_file_leaves_version_0(tmp_path) -> None: +def test_failing_migration_on_fresh_file_leaves_last_good_version( + tmp_path, +) -> None: database = _open_db(tmp_path / "fresh-rollback.db") try: - # The fresh file still reaches migration 2 (the failing one): - # migration 1 commits, stamping version 1, before migration 2 - # fails and rolls back. The run aborts at version 1 — the last - # fully-applied migration — not at version 0. + # The fresh file reaches the failing migration (the last one): + # earlier migrations commit and stamp before it fails and rolls + # back. The run aborts at the last fully-applied migration — + # not at version 0. + broken = _failing_migrations(SCHEMA_V1_STATEMENTS) with pytest.raises(sqlite3.OperationalError): - _migrate(database, _failing_migrations(SCHEMA_V1_STATEMENTS)) - assert _version(database) == 1 - # Migration 2's tables must not survive the failed attempt. + _migrate(database, broken) + assert _version(database) == len(broken) - 1 + # The failing migration's tables must not survive the attempt. assert "later_table" not in _tables(database) # Migration 1's tables do: it committed before the failure. assert "children" in _tables(database) - assert "task_instances" in _tables(database) + assert "quest_instances" in _tables(database) finally: _run(database.close()) @@ -302,12 +553,12 @@ def test_failed_migration_can_be_retried_after_fix(tmp_path) -> None: with pytest.raises(sqlite3.OperationalError): _migrate(database, broken) # "Fix" the migration and retry: it now applies cleanly. - broken[1] = [ + broken[-1] = [ "CREATE TABLE IF NOT EXISTS later_table (id INTEGER PRIMARY KEY)" ] final = _migrate(database, broken) - assert final == 2 - assert _version(database) == 2 + assert final == len(broken) + assert _version(database) == len(broken) assert "later_table" in _tables(database) finally: _run(database.close()) @@ -575,7 +826,7 @@ async def test_setup_entry_migrates_fresh_database(hass, make_entry) -> None: entry = await _setup_entry(hass, make_entry(), hass.registry) database = entry.runtime_data.database - assert await read_schema_version(database) == 1 + assert await read_schema_version(database) == len(MIGRATIONS) # The v1 tables exist because the runner applied migration 1. names = { row[0] @@ -626,7 +877,7 @@ async def test_options_reload_reopens_and_rechecks_version( database_b = entry_reloaded.runtime_data.database assert database_b is not database_a try: - assert await read_schema_version(database_b) == 1 + assert await read_schema_version(database_b) == len(MIGRATIONS) row = await database_b.fetch_one("SELECT COUNT(*) FROM children") assert row == (1,) finally: diff --git a/tests/test_schema.py b/tests/test_schema.py index 245a87c..b95e68b 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -15,8 +15,8 @@ from custom_components.nestquest.schema import ( SCHEMA_V1_PRESENCE_SCHEDULES_DDL, SCHEMA_V1_SCHEDULE_RULES_DDL, SCHEMA_V1_STATEMENTS, - SCHEMA_V1_TASK_DEFINITIONS_DDL, - SCHEMA_V1_TASK_INSTANCES_DDL, + SCHEMA_V1_QUEST_DEFINITIONS_DDL, + SCHEMA_V1_QUEST_INSTANCES_DDL, async_apply_ddl, ) @@ -91,7 +91,7 @@ def _insert_definition(database, child_id=1, rule_id=1, **overrides): placeholders = ", ".join("?" for _ in values) result = _run( database.execute( - f"INSERT INTO task_definitions ({columns}) VALUES ({placeholders})", + f"INSERT INTO quest_definitions ({columns}) VALUES ({placeholders})", tuple(values.values()), ) ) @@ -161,10 +161,10 @@ def test_schema_v1_statements_compose_the_six_tables() -> None: *SCHEMA_V1_CHILDREN_DDL, *SCHEMA_V1_ADMIN_USERS_DDL, *SCHEMA_V1_SCHEDULE_RULES_DDL, - *SCHEMA_V1_TASK_DEFINITIONS_DDL, + *SCHEMA_V1_QUEST_DEFINITIONS_DDL, *SCHEMA_V1_PRESENCE_SCHEDULES_DDL, *SCHEMA_V1_PRESENCE_OVERRIDES_DDL, - *SCHEMA_V1_TASK_INSTANCES_DDL, + *SCHEMA_V1_QUEST_INSTANCES_DDL, *SCHEMA_V1_COMPLETION_EVENTS_DDL, ] @@ -256,17 +256,17 @@ def test_schedule_rules_table_exists_after_applying_ddl(tmp_path) -> None: _run(database.close()) -def test_task_definitions_table_exists_after_applying_ddl(tmp_path) -> None: +def test_quest_definitions_table_exists_after_applying_ddl(tmp_path) -> None: database = _open_db(tmp_path / "definitions.db") try: _apply(database) row = _run( database.fetch_one( "SELECT name FROM sqlite_master WHERE type = 'table' " - "AND name = 'task_definitions'" + "AND name = 'quest_definitions'" ) ) - assert row is not None and row[0] == "task_definitions" + assert row is not None and row[0] == "quest_definitions" finally: _run(database.close()) @@ -292,11 +292,11 @@ def test_schedule_rules_columns_types_and_constraints(tmp_path) -> None: _run(database.close()) -def test_task_definitions_columns_types_and_constraints(tmp_path) -> None: +def test_quest_definitions_columns_types_and_constraints(tmp_path) -> None: database = _open_db(tmp_path / "definitions-columns.db") try: _apply(database) - columns = _run(database.fetch_all("PRAGMA table_info(task_definitions)")) + columns = _run(database.fetch_all("PRAGMA table_info(quest_definitions)")) assert [(row[1], row[2], row[3], row[4], row[5]) for row in columns] == [ ("id", "INTEGER", 0, None, 1), ("title", "TEXT", 1, None, 0), @@ -312,11 +312,11 @@ def test_task_definitions_columns_types_and_constraints(tmp_path) -> None: _run(database.close()) -def test_task_definitions_foreign_keys_declared(tmp_path) -> None: +def test_quest_definitions_foreign_keys_declared(tmp_path) -> None: database = _open_db(tmp_path / "definitions-fks.db") try: _apply(database) - fks = _run(database.fetch_all("PRAGMA foreign_key_list(task_definitions)")) + fks = _run(database.fetch_all("PRAGMA foreign_key_list(quest_definitions)")) declared = {(row[2], row[3], row[4]) for row in fks} assert declared == { ("children", "child_id", "id"), @@ -519,7 +519,7 @@ def test_failed_statement_rolls_back_whole_application(tmp_path) -> None: # --------------------------------------------------------------------------- -# schedule_rules and task_definitions constraint enforcement +# schedule_rules and quest_definitions constraint enforcement # --------------------------------------------------------------------------- @@ -813,7 +813,7 @@ def test_schedule_rules_start_date_not_null_enforced(tmp_path) -> None: _run(database.close()) -def test_task_definitions_insert_requires_existing_child_and_rule( +def test_quest_definitions_insert_requires_existing_child_and_rule( tmp_path, ) -> None: database = _open_db(tmp_path / "definitions-fk-unknown.db") @@ -831,7 +831,7 @@ def test_task_definitions_insert_requires_existing_child_and_rule( ) row = _run( database.fetch_one( - "SELECT title, child_id, schedule_rule_id FROM task_definitions " + "SELECT title, child_id, schedule_rule_id FROM quest_definitions " "WHERE id = ?", (definition_id,), ) @@ -841,7 +841,7 @@ def test_task_definitions_insert_requires_existing_child_and_rule( _run(database.close()) -def test_task_definitions_unknown_child_id_fails(tmp_path) -> None: +def test_quest_definitions_unknown_child_id_fails(tmp_path) -> None: database = _open_db(tmp_path / "definitions-bad-child.db") try: _apply(database) @@ -852,7 +852,7 @@ def test_task_definitions_unknown_child_id_fails(tmp_path) -> None: _run(database.close()) -def test_task_definitions_unknown_schedule_rule_id_fails(tmp_path) -> None: +def test_quest_definitions_unknown_schedule_rule_id_fails(tmp_path) -> None: database = _open_db(tmp_path / "definitions-bad-rule.db") try: _apply(database) @@ -868,7 +868,7 @@ def test_task_definitions_unknown_schedule_rule_id_fails(tmp_path) -> None: _run(database.close()) -def test_task_definitions_is_active_check_enforced(tmp_path) -> None: +def test_quest_definitions_is_active_check_enforced(tmp_path) -> None: database = _open_db(tmp_path / "definitions-is-active.db") try: _apply(database) @@ -880,7 +880,7 @@ def test_task_definitions_is_active_check_enforced(tmp_path) -> None: @pytest.mark.parametrize("bad_is_active", ["abc", 1.5]) -def test_task_definitions_is_active_non_integer_fails( +def test_quest_definitions_is_active_non_integer_fails( tmp_path, bad_is_active ) -> None: database = _open_db(tmp_path / "definitions-is-active-typeof.db") @@ -901,7 +901,7 @@ def test_task_definitions_is_active_non_integer_fails( _run(database.close()) -def test_task_definitions_title_not_null_enforced(tmp_path) -> None: +def test_quest_definitions_title_not_null_enforced(tmp_path) -> None: database = _open_db(tmp_path / "definitions-title.db") try: _apply(database) @@ -912,7 +912,7 @@ def test_task_definitions_title_not_null_enforced(tmp_path) -> None: _run(database.close()) -def test_task_definitions_created_at_not_null_enforced(tmp_path) -> None: +def test_quest_definitions_created_at_not_null_enforced(tmp_path) -> None: database = _open_db(tmp_path / "definitions-created-at.db") try: _apply(database) @@ -925,7 +925,7 @@ def test_task_definitions_created_at_not_null_enforced(tmp_path) -> None: _run(database.close()) -def test_task_definitions_defaults_apply_on_insert(tmp_path) -> None: +def test_quest_definitions_defaults_apply_on_insert(tmp_path) -> None: database = _open_db(tmp_path / "definitions-defaults.db") try: _apply(database) @@ -941,7 +941,7 @@ def test_task_definitions_defaults_apply_on_insert(tmp_path) -> None: ) row = _run( database.fetch_one( - "SELECT is_active, due_time FROM task_definitions WHERE id = ?", + "SELECT is_active, due_time FROM quest_definitions WHERE id = ?", (definition_id,), ) ) @@ -969,7 +969,7 @@ def test_applying_extended_ddl_twice_keeps_new_table_rows(tmp_path) -> None: _insert_definition(database, child_id=1, rule_id=1) _apply(database) counts = [] - for table in ("children", "schedule_rules", "task_definitions"): + for table in ("children", "schedule_rules", "quest_definitions"): row = _run(database.fetch_one(f"SELECT COUNT(*) FROM {table}")) counts.append(row[0]) assert counts == [1, 1, 1] @@ -1406,7 +1406,7 @@ def test_applying_presence_ddl_twice_keeps_rows(tmp_path) -> None: # --------------------------------------------------------------------------- -# task_instances and completion_events: helpers +# quest_instances and completion_events: helpers # --------------------------------------------------------------------------- @@ -1428,7 +1428,7 @@ def _insert_instance(database, definition_id=1, **overrides): placeholders = ", ".join("?" for _ in values) result = _run( database.execute( - f"INSERT INTO task_instances ({columns}) VALUES ({placeholders})", + f"INSERT INTO quest_instances ({columns}) VALUES ({placeholders})", tuple(values.values()), ) ) @@ -1466,28 +1466,28 @@ def _insert_event(database, instance_id=1, **overrides): def _setup_definition(database) -> None: - """Create one child, one schedule rule and one task definition.""" + """Create one child, one schedule rule and one quest definition.""" _child(database) _insert_rule(database) _insert_definition(database, child_id=1, rule_id=1) # --------------------------------------------------------------------------- -# task_instances and completion_events: DDL shape +# quest_instances and completion_events: DDL shape # --------------------------------------------------------------------------- -def test_task_instances_table_exists_after_applying_ddl(tmp_path) -> None: +def test_quest_instances_table_exists_after_applying_ddl(tmp_path) -> None: database = _open_db(tmp_path / "task-instances.db") try: _apply(database) row = _run( database.fetch_one( "SELECT name FROM sqlite_master WHERE type = 'table' " - "AND name = 'task_instances'" + "AND name = 'quest_instances'" ) ) - assert row is not None and row[0] == "task_instances" + assert row is not None and row[0] == "quest_instances" finally: _run(database.close()) @@ -1507,11 +1507,11 @@ def test_completion_events_table_exists_after_applying_ddl(tmp_path) -> None: _run(database.close()) -def test_task_instances_columns_types_and_constraints(tmp_path) -> None: +def test_quest_instances_columns_types_and_constraints(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-columns.db") try: _apply(database) - columns = _run(database.fetch_all("PRAGMA table_info(task_instances)")) + columns = _run(database.fetch_all("PRAGMA table_info(quest_instances)")) # cid, name, type, notnull, dflt_value, pk assert [(row[1], row[2], row[3], row[4], row[5]) for row in columns] == [ ("id", "INTEGER", 0, None, 1), @@ -1544,14 +1544,14 @@ def test_completion_events_columns_types_and_constraints(tmp_path) -> None: _run(database.close()) -def test_task_instances_foreign_keys_declared(tmp_path) -> None: +def test_quest_instances_foreign_keys_declared(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-fks.db") try: _apply(database) - fks = _run(database.fetch_all("PRAGMA foreign_key_list(task_instances)")) + fks = _run(database.fetch_all("PRAGMA foreign_key_list(quest_instances)")) declared = {(row[2], row[3], row[4]) for row in fks} assert declared == { - ("task_definitions", "definition_id", "id"), + ("quest_definitions", "definition_id", "id"), ("children", "child_id", "id"), } finally: @@ -1565,14 +1565,14 @@ def test_completion_events_foreign_keys_declared(tmp_path) -> None: fks = _run(database.fetch_all("PRAGMA foreign_key_list(completion_events)")) declared = {(row[2], row[3], row[4]) for row in fks} assert declared == { - ("task_instances", "instance_id", "id"), + ("quest_instances", "instance_id", "id"), ("children", "child_id", "id"), } finally: _run(database.close()) -def test_task_instances_unique_index_on_definition_and_date(tmp_path) -> None: +def test_quest_instances_unique_index_on_definition_and_date(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-unique.db") try: _apply(database) @@ -1580,7 +1580,7 @@ def test_task_instances_unique_index_on_definition_and_date(tmp_path) -> None: rows = _run( database.fetch_all( "SELECT name, sql FROM sqlite_master WHERE type = 'index' " - "AND tbl_name = 'task_instances'" + "AND tbl_name = 'quest_instances'" ) ) implicit = [(name, sql) for name, sql in rows if sql is None] @@ -1599,7 +1599,7 @@ def test_task_instances_unique_index_on_definition_and_date(tmp_path) -> None: row = _run( database.fetch_one( "SELECT COUNT(DISTINCT name) FROM pragma_index_list" - "('task_instances') WHERE \"unique\" = 1" + "('quest_instances') WHERE \"unique\" = 1" ) ) assert row == (1,) @@ -1608,11 +1608,11 @@ def test_task_instances_unique_index_on_definition_and_date(tmp_path) -> None: # --------------------------------------------------------------------------- -# task_instances: constraint enforcement +# quest_instances: constraint enforcement # --------------------------------------------------------------------------- -def test_task_instances_valid_insert_round_trips(tmp_path) -> None: +def test_quest_instances_valid_insert_round_trips(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-valid.db") try: _apply(database) @@ -1621,7 +1621,7 @@ def test_task_instances_valid_insert_round_trips(tmp_path) -> None: row = _run( database.fetch_one( "SELECT definition_id, child_id, due_date, due_time, generated_at " - "FROM task_instances WHERE id = ?", + "FROM quest_instances WHERE id = ?", (instance_id,), ) ) @@ -1630,7 +1630,7 @@ def test_task_instances_valid_insert_round_trips(tmp_path) -> None: _run(database.close()) -def test_task_instances_due_time_round_trips(tmp_path) -> None: +def test_quest_instances_due_time_round_trips(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-due-time.db") try: _apply(database) @@ -1640,7 +1640,7 @@ def test_task_instances_due_time_round_trips(tmp_path) -> None: ) row = _run( database.fetch_one( - "SELECT due_time FROM task_instances WHERE id = ?", + "SELECT due_time FROM quest_instances WHERE id = ?", (instance_id,), ) ) @@ -1649,7 +1649,7 @@ def test_task_instances_due_time_round_trips(tmp_path) -> None: _run(database.close()) -def test_task_instances_duplicate_definition_date_fails(tmp_path) -> None: +def test_quest_instances_duplicate_definition_date_fails(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-duplicate.db") try: _apply(database) @@ -1661,7 +1661,7 @@ def test_task_instances_duplicate_definition_date_fails(tmp_path) -> None: _run(database.close()) -def test_task_instances_same_date_different_definitions_allowed( +def test_quest_instances_same_date_different_definitions_allowed( tmp_path, ) -> None: database = _open_db(tmp_path / "task-instances-multi-def.db") @@ -1678,26 +1678,26 @@ def test_task_instances_same_date_different_definitions_allowed( _insert_definition(database, child_id=2, rule_id=rule_id) _insert_instance(database, definition_id=1, due_date="2026-09-14") _insert_instance(database, definition_id=2, due_date="2026-09-14") - row = _run(database.fetch_one("SELECT COUNT(*) FROM task_instances")) + row = _run(database.fetch_one("SELECT COUNT(*) FROM quest_instances")) assert row == (2,) finally: _run(database.close()) -def test_task_instances_same_definition_different_dates_allowed(tmp_path) -> None: +def test_quest_instances_same_definition_different_dates_allowed(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-multi-date.db") try: _apply(database) _setup_definition(database) _insert_instance(database, definition_id=1, due_date="2026-09-14") _insert_instance(database, definition_id=1, due_date="2026-09-15") - row = _run(database.fetch_one("SELECT COUNT(*) FROM task_instances")) + row = _run(database.fetch_one("SELECT COUNT(*) FROM quest_instances")) assert row == (2,) finally: _run(database.close()) -def test_task_instances_unknown_definition_fails(tmp_path) -> None: +def test_quest_instances_unknown_definition_fails(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-bad-def.db") try: _apply(database) @@ -1708,7 +1708,7 @@ def test_task_instances_unknown_definition_fails(tmp_path) -> None: _run(database.close()) -def test_task_instances_unknown_child_fails(tmp_path) -> None: +def test_quest_instances_unknown_child_fails(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-bad-child.db") try: _apply(database) @@ -1719,7 +1719,7 @@ def test_task_instances_unknown_child_fails(tmp_path) -> None: _run(database.close()) -def test_task_instances_due_date_not_null_enforced(tmp_path) -> None: +def test_quest_instances_due_date_not_null_enforced(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-due-null.db") try: _apply(database) @@ -1730,7 +1730,7 @@ def test_task_instances_due_date_not_null_enforced(tmp_path) -> None: _run(database.close()) -def test_task_instances_generated_at_not_null_enforced(tmp_path) -> None: +def test_quest_instances_generated_at_not_null_enforced(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-generated-null.db") try: _apply(database) @@ -2026,8 +2026,8 @@ def test_applying_full_ddl_twice_keeps_rows(tmp_path) -> None: for table in ( "children", "schedule_rules", - "task_definitions", - "task_instances", + "quest_definitions", + "quest_instances", "completion_events", ): row = _run(database.fetch_one(f"SELECT COUNT(*) FROM {table}")) diff --git a/tests/test_startup_db.py b/tests/test_startup_db.py index f6620b5..da77d61 100644 --- a/tests/test_startup_db.py +++ b/tests/test_startup_db.py @@ -18,6 +18,7 @@ from custom_components.nestquest import ( ) from custom_components.nestquest.db import NestQuestDatabase from custom_components.nestquest.migrations import ( + MIGRATIONS, VERSION_TABLE, apply_migrations, ) @@ -114,12 +115,13 @@ def test_missing_file_opens_fresh(tmp_path) -> None: assert not db_path.exists() database = _run(_async_open_database(_make_hass_mock(), db_path)) try: - # The file was created fresh and migrations stamped version 1. + # The file was created fresh and migrations stamped the + # latest schema version. assert db_path.exists() row = _run( database.fetch_one(f"SELECT version FROM {VERSION_TABLE}") ) - assert row == (1,) + assert row == (len(MIGRATIONS),) finally: _run(database.close()) @@ -151,7 +153,7 @@ def test_valid_file_opens_and_migrations_noop(tmp_path) -> None: row = _run( database.fetch_one(f"SELECT version FROM {VERSION_TABLE}") ) - assert row == (1,) + assert row == (len(MIGRATIONS),) finally: _run(database.close()) # Opening a valid file must not rewrite it (the no-op path). @@ -401,7 +403,7 @@ async def test_setup_with_missing_file_creates_and_loads( row = await database.fetch_one( f"SELECT version FROM {VERSION_TABLE}" ) - assert row == (1,) + assert row == (len(MIGRATIONS),) finally: await database.close() assert await async_unload_entry(hass, entry) is True From 25f0a43e16d662dc599a6b21aeecbf165f7936d6 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 09:59:29 -0400 Subject: [PATCH 2/5] Multi-assignee definitions via quest_definition_assignees (D-008) Per IMPLEMENTATION-ORDER.md 1.3: a quest_definition_assignees table (definition_id, child_id) with a composite primary key and FKs to quest_definitions and children replaces the single child_id column on quest_definitions. - schema.py: definitions DDL loses child_id; new assignees table joins SCHEMA_V1_STATEMENTS; the definitions table body is extracted to QUEST_DEFINITIONS_TABLE_SQL so the rebuild migration cannot drift from the canonical shape. - migrations.py: callable migrations now own their transaction and stamp via stamp_schema_version(database, version), because the rebuild must bracket itself with PRAGMA foreign_keys OFF/ON. New migration 3 creates the assignees table and, when a pre-D-008 quest_definitions still carries child_id, rebuilds the table via SQLite's canonical procedure (create, copy, move assignees, drop, rename), verifies PRAGMA foreign_key_check inside the transaction, and stamps. Migration 2 converted to the new callable contract. - dao_rules.py: QuestDefinitionsDao.create drops the child_id argument and applies assignee_child_ids atomically; set_assignee is replaced by add_assignee (idempotent, active-child validated under the connection lock), remove_assignee and list_assignees; list_by_child reads through the assignees table. - dao_instances.py: upsert validates the child against quest_definition_assignees instead of the definitions column. - tests: schema shape tests for the assignees table (columns, PK, FKs, duplicate/unknown constraints); DAO tests rewritten for the new assignment API; migration tests for the v2 -> v3 rebuild (rows and instances survive, child_id gone, foreign_key_check clean). --- custom_components/nestquest/dao_instances.py | 19 +- custom_components/nestquest/dao_rules.py | 187 +++++++++----- custom_components/nestquest/migrations.py | 255 ++++++++++++++----- custom_components/nestquest/schema.py | 44 +++- tests/test_dao_instances.py | 29 ++- tests/test_dao_matrix.py | 25 +- tests/test_dao_rules.py | 208 ++++++++++----- tests/test_migrations.py | 181 ++++++++++++- tests/test_schema.py | 196 ++++++++++++-- 9 files changed, 883 insertions(+), 261 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 4c78a49..43c8997 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -19,7 +19,9 @@ Upsert semantics: ``QuestInstancesDao.upsert`` is idempotent on whose instance already exists updates nothing that matters (child_id/due_time are refreshed only in the degenerate re-generation case where they changed) and never duplicates a row. The schema's -UNIQUE(definition_id, due_date) is the last line of defense. +UNIQUE(definition_id, due_date) is the last line of defense. The +upsert refuses a child that is not an assignee of the definition +(D-008 multi-assignee model). ``delete_future_uncompleted`` implements the Feature 06/07 rule that reassignment and schedule edits regenerate future instances: it @@ -207,18 +209,23 @@ class QuestInstancesDao: f"{today!r}" ) definition = await self._database.fetch_one( - "SELECT child_id FROM quest_definitions WHERE id = ?", + "SELECT 1 FROM quest_definitions WHERE id = ?", (definition_id,), ) if definition is None: raise ValueError( f"quest definition {definition_id} does not exist" ) - if definition[0] != child_id: + assignee = await self._database.fetch_one( + "SELECT 1 FROM quest_definition_assignees " + "WHERE definition_id = ? AND child_id = ?", + (definition_id, child_id), + ) + if assignee is None: raise ValueError( - f"quest definition {definition_id} is assigned to " - f"child {definition[0]}, not {child_id}; instance " - "must carry the definition's assignee" + f"quest definition {definition_id} is not " + f"assigned to child {child_id}; instance must " + "carry an assigned child" ) child = await self._database.fetch_one( "SELECT 1 FROM children WHERE id = ?", (child_id,) diff --git a/custom_components/nestquest/dao_rules.py b/custom_components/nestquest/dao_rules.py index 494dd03..291febb 100644 --- a/custom_components/nestquest/dao_rules.py +++ b/custom_components/nestquest/dao_rules.py @@ -1,8 +1,10 @@ -"""Typed DAO layer for the schedule_rules and quest_definitions tables. +"""Typed DAO layer for the schedule_rules, quest_definitions and +quest_definition_assignees tables. -All SQL for ``schedule_rules`` and ``quest_definitions`` lives in this -module per the feature guardrails: callers get typed dataclasses back -and never see raw rows or SQL. Every method is async and runs through +All SQL for ``schedule_rules``, ``quest_definitions`` and +``quest_definition_assignees`` lives in this module per the feature +guardrails: callers get typed dataclasses back and never see raw rows +or SQL. Every method is async and runs through :class:`~.db.NestQuestDatabase`, so each statement executes on the HA executor and the event loop never blocks. @@ -13,25 +15,29 @@ Guardrail mapping: not cascaded. - ``quest_definitions`` has no delete path at all: definitions are deactivated, never hard-deleted, and history must survive. -- ``set_assignee`` changes future instances only — the DAO writes the - definition row and nothing else; reassignment never rewrites +- Assignment is multi-assignee (D-008): :meth:`add_assignee` and + :meth:`remove_assignee` change future instances only — the DAO writes + the assignee table and nothing else; assignment edits never rewrite existing instances or completion history (Feature 06 guardrail). - Validation of rule well-formedness and child activeness lives partly in the schema CHECKs (rule-type coherence) and partly in - :meth:`QuestDefinitionsDao.create` (active child, rule existence); - no permission checks here, that is the Feature 09 gate's job. + :meth:`QuestDefinitionsDao.create` (rule existence) and + :meth:`QuestDefinitionsDao.add_assignee` (active child); no + permission checks here, that is the Feature 09 gate's job. -Concurrency: ``delete_rule_if_unreferenced`` performs its reference -check and DELETE inside one transaction under the connection-scoped -lock shared with :mod:`.dao_children`, so a definition referencing the -rule cannot appear between check and delete on this connection. +Concurrency: ``delete_rule_if_unreferenced`` and the assignee +mutations perform their checks and writes inside one transaction under +the connection-scoped lock shared with :mod:`.dao_children`, so a +definition referencing the rule (or an assignee appearing) cannot slip +in between check and write on this connection. """ from __future__ import annotations import datetime from dataclasses import dataclass -from .dao_children import _connection_lock +from .dao_children import ChildRecord, _child_from_row, _connection_lock +from .dao_children import _CHILD_COLUMNS from .db import NestQuestDatabase #: Sentinel distinguishing "argument omitted" from "explicit SQL NULL" @@ -82,13 +88,12 @@ class ScheduleRuleRecord: @dataclass(frozen=True) class QuestDefinitionRecord: - """One row of ``quest_definitions``.""" + """One row of ``quest_definitions`` (without its assignees).""" id: int title: str description: str | None icon: str | None - child_id: int schedule_rule_id: int due_time: str | None is_active: bool @@ -100,7 +105,7 @@ _RULE_COLUMNS = ( "month, start_date, end_date" ) _DEFINITION_COLUMNS = ( - "id, title, description, icon, child_id, schedule_rule_id, due_time, " + "id, title, description, icon, schedule_rule_id, due_time, " "is_active, created_at" ) @@ -125,11 +130,10 @@ def _definition_from_row(row: tuple) -> QuestDefinitionRecord: title=row[1], description=row[2], icon=row[3], - child_id=row[4], - schedule_rule_id=row[5], - due_time=row[6], - is_active=bool(row[7]), - created_at=row[8], + schedule_rule_id=row[4], + due_time=row[5], + is_active=bool(row[6]), + created_at=row[7], ) @@ -272,11 +276,14 @@ class ScheduleRulesDao: class QuestDefinitionsDao: - """Typed async access to the ``quest_definitions`` table. + """Typed async access to ``quest_definitions`` and its assignees. No delete method exists: definitions are deactivated via :meth:`set_active`, never hard-deleted, so completion history keeps - its references (feature guardrail). + its references (feature guardrail). Assignment is multi-assignee + (D-008): the definition row carries no child; assignees live in + ``quest_definition_assignees`` and are managed explicitly so the + operations stay separately loggable and permission-gateable. """ def __init__(self, database: NestQuestDatabase) -> None: @@ -285,7 +292,6 @@ class QuestDefinitionsDao: async def create( self, title: str, - child_id: int, schedule_rule_id: int, created_at: str, *, @@ -293,53 +299,55 @@ class QuestDefinitionsDao: icon: str | None = None, due_time: str | None = None, is_active: bool = True, + assignee_child_ids: list[int] | None = None, ) -> QuestDefinitionRecord: """Insert one definition and return the record as stored. - Validates the assignment target as part of the same serialized - transaction as the insert: the child must exist and be active, - and the rule must exist, AT INSERT TIME — a child deactivated - or a rule deleted concurrently cannot slip between the checks - and the INSERT, which would otherwise create an invalid - assignment or surface as a raw FK IntegrityError instead of - the documented ValueError. An INACTIVE child would pass the - schema foreign keys, which is why the activeness check lives - here. + Validates the schedule rule as part of the same serialized + transaction as the insert: the rule must exist AT INSERT TIME — + a rule deleted concurrently cannot slip between the check and + the INSERT. ``assignee_child_ids`` (each child must exist and + be active) is applied in the same transaction, so a definition + can never be observed with a stale or missing assignee set; an + empty list or None creates an unassigned definition, which + callers may fill via :meth:`add_assignee`. """ async with _connection_lock(self._database): async with self._database.transaction(): - await self._validate_assignable( - child_id, schedule_rule_id - ) + await self._validate_rule_exists(schedule_rule_id) + for child_id in assignee_child_ids or []: + await self._validate_assignable_child(child_id) result = await self._database.execute( "INSERT INTO quest_definitions (title, description, " - "icon, child_id, schedule_rule_id, due_time, " - "is_active, created_at) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + "icon, schedule_rule_id, due_time, is_active, " + "created_at) VALUES (?, ?, ?, ?, ?, ?, ?)", ( title, description, icon, - child_id, schedule_rule_id, due_time, int(is_active), created_at, ), ) - definition = await self.get(result.lastrowid) + definition_id = result.lastrowid + for child_id in assignee_child_ids or []: + await self._database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (?, ?)", + (definition_id, child_id), + ) + definition = await self.get(definition_id) assert definition is not None return definition - async def _validate_assignable( - self, child_id: int, schedule_rule_id: int - ) -> None: - """Raise ValueError unless child (active) and rule both exist. + async def _validate_rule_exists(self, schedule_rule_id: int) -> None: + """Raise ValueError unless the rule exists. MUST be called inside the connection lock so the verdict cannot go stale before the caller's INSERT commits. """ - await self._validate_assignable_child(child_id) rule = await self._database.fetch_one( "SELECT 1 FROM schedule_rules WHERE id = ?", (schedule_rule_id,), @@ -359,10 +367,17 @@ class QuestDefinitionsDao: return _definition_from_row(row) if row is not None else None async def list_by_child(self, child_id: int) -> list[QuestDefinitionRecord]: - """Return all definitions assigned to ``child_id``, newest first.""" + """Return all definitions assigned to ``child_id``, newest first. + + Assignment is read through ``quest_definition_assignees`` + (D-008): a definition appears once per child regardless of how + many assignees share it. + """ rows = await self._database.fetch_all( f"SELECT {_DEFINITION_COLUMNS} FROM quest_definitions " - "WHERE child_id = ? ORDER BY id DESC", + "WHERE id IN (SELECT definition_id " + "FROM quest_definition_assignees WHERE child_id = ?) " + "ORDER BY id DESC", (child_id,), ) return [_definition_from_row(row) for row in rows] @@ -391,8 +406,9 @@ class QuestDefinitionsDao: SQL NULL, so callers can remove optional metadata (description, icon, due_time). Assignment and activation are deliberately NOT settable here: they have their own explicit - operations (:meth:`set_assignee`, :meth:`set_active`) so they - stay separately loggable and permission-gateable. + operations (:meth:`add_assignee`, :meth:`remove_assignee`, + :meth:`set_active`) so they stay separately loggable and + permission-gateable. """ assignments: list[str] = [] parameters: list[object] = [] @@ -427,32 +443,75 @@ class QuestDefinitionsDao: ) return result.rowcount - async def set_assignee( + async def add_assignee( self, definition_id: int, child_id: int - ) -> int: - """Reassign the definition to ``child_id``; rows updated (0 if absent). + ) -> None: + """Assign ``child_id`` to the definition (idempotent). - Validates the new assignee inside the same serialized - transaction as the UPDATE (existing and active AT ASSIGNMENT - TIME), so a child deactivated concurrently cannot become the - assignee. Changes future instances only: already-generated - instances and completion history are never rewritten here. + Validates the assignee inside the same serialized transaction + as the INSERT (existing and active AT ASSIGNMENT TIME), so a + child deactivated concurrently cannot become an assignee. + Assigning an already-assigned child is a no-op, mirroring the + composite primary key's storage-level guarantee. Changes + future instances only: already-generated instances and + completion history are never rewritten here. """ async with _connection_lock(self._database): async with self._database.transaction(): await self._validate_assignable_child(child_id) - result = await self._database.execute( - "UPDATE quest_definitions SET child_id = ? " - "WHERE id = ?", - (child_id, definition_id), + definition = await self._database.fetch_one( + "SELECT 1 FROM quest_definitions WHERE id = ?", + (definition_id,), ) - return result.rowcount + if definition is None: + raise ValueError( + f"quest definition {definition_id} does not exist" + ) + await self._database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (?, ?) " + "ON CONFLICT (definition_id, child_id) DO NOTHING", + (definition_id, child_id), + ) + + async def remove_assignee( + self, definition_id: int, child_id: int + ) -> bool: + """Remove the assignee link; True when a row was removed. + + Future materialization stops for this (definition, child) pair; + existing instances and completion history are never touched + here (Feature 06 guardrail). + """ + async with _connection_lock(self._database): + result = await self._database.execute( + "DELETE FROM quest_definition_assignees " + "WHERE definition_id = ? AND child_id = ?", + (definition_id, child_id), + ) + return result.rowcount > 0 + + async def list_assignees(self, definition_id: int) -> list[ChildRecord]: + """Return the definition's assignees as child records. + + Ordered by the children table's stable display order + (``sort_order``, then id) so callers render a deterministic + roster. A definition with no assignees returns []. + """ + rows = await self._database.fetch_all( + f"SELECT {_CHILD_COLUMNS} FROM children " + "WHERE id IN (SELECT child_id " + "FROM quest_definition_assignees WHERE definition_id = ?) " + "ORDER BY sort_order, id", + (definition_id,), + ) + return [_child_from_row(row) for row in rows] async def _validate_assignable_child(self, child_id: int) -> None: """Raise ValueError unless the child exists and is active. MUST be called inside the connection lock so the verdict cannot - go stale before the caller's UPDATE commits. + go stale before the caller's INSERT commits. """ child = await self._database.fetch_one( "SELECT is_active FROM children WHERE id = ?", (child_id,) diff --git a/custom_components/nestquest/migrations.py b/custom_components/nestquest/migrations.py index 1571739..0b000ef 100644 --- a/custom_components/nestquest/migrations.py +++ b/custom_components/nestquest/migrations.py @@ -20,15 +20,21 @@ instead of the legacy ``task_*`` spellings, and no compatibility shim exists. Dev machines that already hold a version-1 database with ``task_*`` tables are handled by migration 2, which detects the legacy table names and ``ALTER TABLE ... RENAME TO``s them to their current -names before anything else assumes the new model. The rename step runs -inside the same transaction as its version stamp, so a crash mid-rename -rolls both back together. +names before anything else assumes the new model. Migration 3 brings +pre-D-008 databases to the multi-assignee model: it creates +``quest_definition_assignees`` and, when ``quest_definitions`` still +carries a single ``child_id`` column, rebuilds the table without it, +copying each definition's assignee across. Each callable migration +runs inside its own transaction together with its version stamp, so a +crash mid-step rolls the statements and the stamp back together. Migration entries are either an ordered sequence of SQL statements or -an async callable taking the open database. Callables exist for steps -whose SQL depends on the database's current state (the legacy rename -only fires when a legacy table is actually present) and run under the -same per-migration transaction as statement lists. +an async callable taking ``(database, target_version)``. Callables +exist for steps whose SQL depends on the database's current state (a +legacy rename only fires when a legacy table is present; a rebuild only +when the old column shape is present) and own their transaction so they +can bracket it with connection pragmas the runner's uniform transaction +cannot express. Each migration's statements and its version stamp run inside ONE transaction, and the stamp is written as DELETE + INSERT rather than @@ -63,13 +69,21 @@ from collections.abc import Awaitable, Callable, Sequence from typing import Union from .db import NestQuestDatabase -from .schema import SCHEMA_V1_STATEMENTS +from .schema import ( + QUEST_DEFINITIONS_TABLE_SQL, + SCHEMA_V1_STATEMENTS, + SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, +) LOGGER = logging.getLogger(__name__) #: One migration entry: either an ordered sequence of SQL statements or -#: an async callable run under the entry's migration transaction. -MigrationStep = Union[Sequence[str], Callable[[NestQuestDatabase], Awaitable[None]]] +#: an async callable taking ``(database, target_version)`` that manages +#: its own transaction and stamps via :func:`stamp_schema_version`. +MigrationStep = Union[ + Sequence[str], + Callable[[NestQuestDatabase, int], Awaitable[None]], +] #: Metadata table holding the applied schema version. Prefixed with the #: integration domain so a config directory shared with other tools can @@ -94,6 +108,10 @@ VERSION_TABLE_DDL = f""" #: version ``n-1`` to version ``n``. MIGRATION_1_V1_DDL: list[str] = list(SCHEMA_V1_STATEMENTS) +#: The assignees table DDL, shared with migration 1 via the schema +#: module so a rebuilt and a freshly created table cannot drift. +_ASSIGNEES_TABLE_DDL: str = SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL[0] + #: Legacy (pre-D-007) table name -> current table name. Order matters: #: the parent table renames first so the child's foreign key reference #: is rewritten while its parent's new name is already in place. @@ -103,7 +121,34 @@ LEGACY_TASK_TABLE_RENAMES: tuple[tuple[str, str], ...] = ( ) -async def _rename_legacy_task_tables(database: NestQuestDatabase) -> None: +async def stamp_schema_version( + database: NestQuestDatabase, version: int +) -> None: + """Write the schema version stamp (inside the caller's transaction). + + INSERT keyed on the singleton row (id = 1) with an upsert: a fresh + file has no row yet, so UPDATE alone would silently match zero rows + there and leave a migrated-but-unstamped database. + """ + await database.execute( + f"INSERT INTO {VERSION_TABLE} (id, version) " + "VALUES (1, ?) " + "ON CONFLICT (id) DO UPDATE SET version = excluded.version", + (version,), + ) + + +async def _table_has_column( + database: NestQuestDatabase, table: str, column: str +) -> bool: + """Return True when ``table`` currently carries ``column``.""" + rows = await database.fetch_all(f"PRAGMA table_info({table})") + return any(row[1] == column for row in rows) + + +async def _rename_legacy_task_tables( + database: NestQuestDatabase, target_version: int +) -> None: """Rename any surviving ``task_*`` tables to their ``quest_*`` names. Dev-machines-only path (D-007): a database stamped version 1 by the @@ -112,22 +157,96 @@ async def _rename_legacy_task_tables(database: NestQuestDatabase) -> None: nothing and this step is a no-op. Modern SQLite rewrites foreign key clauses in other tables to follow the rename, so the renamed schema is indistinguishable from a freshly created one apart from - internal autoindex names. Runs inside the migration transaction, - so the renames and the version stamp commit or roll back together. + internal autoindex names. Runs in its own transaction together + with the version stamp, so a crash rolls both back. """ - for legacy_name, current_name in LEGACY_TASK_TABLE_RENAMES: - row = await database.fetch_one( - "SELECT 1 FROM sqlite_master WHERE type = 'table' " - "AND name = ?", - (legacy_name,), - ) - if row is not None: - await database.execute( - f'ALTER TABLE "{legacy_name}" RENAME TO "{current_name}"' - ) - LOGGER.info( - "Renamed legacy table %s to %s", legacy_name, current_name + async with database.transaction(): + for legacy_name, current_name in LEGACY_TASK_TABLE_RENAMES: + row = await database.fetch_one( + "SELECT 1 FROM sqlite_master WHERE type = 'table' " + "AND name = ?", + (legacy_name,), ) + if row is not None: + await database.execute( + f'ALTER TABLE "{legacy_name}" RENAME TO "{current_name}"' + ) + LOGGER.info( + "Renamed legacy table %s to %s", + legacy_name, + current_name, + ) + await stamp_schema_version(database, target_version) + + +async def _add_definition_assignees( + database: NestQuestDatabase, target_version: int +) -> None: + """Bring definitions to the multi-assignee model (D-008). + + Two shapes arrive here: + + - Fresh or renamed databases whose ``quest_definitions`` already + matches the current canonical DDL (no ``child_id`` column): only + the assignees table needs creating (``IF NOT EXISTS`` is a no-op + when migration 1 already made it). + - Databases stamped at version 2 whose ``quest_definitions`` still + carries the single ``child_id`` column: the table is rebuilt via + SQLite's canonical procedure — create the new table, copy rows, + move each definition's assignee into + ``quest_definition_assignees``, drop the old table, rename — so + ``DROP COLUMN``'s foreign-key restriction never applies and no + definition or instance row is lost. + + The copy runs with ``foreign_keys = OFF`` because dropping the old + table under enforcement would fire the implicit DELETE and violate + the ``quest_instances`` foreign key; ``PRAGMA foreign_key_check`` + runs inside the same transaction before the stamp, so a corrupt + copy aborts the whole migration instead of persisting. The pragma + is a connection-level no-op inside a transaction, hence the + off/transaction/on bracket. + """ + legacy_child_id = await _table_has_column( + database, "quest_definitions", "child_id" + ) + await database.execute("PRAGMA foreign_keys = OFF") + try: + async with database.transaction(): + await database.execute(_ASSIGNEES_TABLE_DDL) + if legacy_child_id: + await database.execute( + "CREATE TABLE quest_definitions_rebuilt " + f"{QUEST_DEFINITIONS_TABLE_SQL}" + ) + await database.execute( + "INSERT INTO quest_definitions_rebuilt " + "(id, title, description, icon, schedule_rule_id, " + "due_time, is_active, created_at) " + "SELECT id, title, description, icon, " + "schedule_rule_id, due_time, is_active, created_at " + "FROM quest_definitions ORDER BY id" + ) + await database.execute( + "INSERT OR IGNORE INTO quest_definition_assignees " + "(definition_id, child_id) " + "SELECT id, child_id FROM quest_definitions" + ) + await database.execute("DROP TABLE quest_definitions") + await database.execute( + "ALTER TABLE quest_definitions_rebuilt " + "RENAME TO quest_definitions" + ) + violations = await database.fetch_all( + "PRAGMA foreign_key_check" + ) + if violations: + raise RuntimeError( + "definition rebuild produced foreign-key " + f"violations: {violations!r}" + ) + await stamp_schema_version(database, target_version) + finally: + await database.execute("PRAGMA foreign_keys = ON") #: The ordered migration list. Append-only: never edit an applied @@ -136,6 +255,7 @@ async def _rename_legacy_task_tables(database: NestQuestDatabase) -> None: MIGRATIONS: Sequence[MigrationStep] = [ MIGRATION_1_V1_DDL, _rename_legacy_task_tables, + _add_definition_assignees, ] @@ -248,17 +368,18 @@ async def apply_migrations( for target in range(current + 1, latest + 1): applied = False - async with database.transaction(): - # Re-read the version INSIDE the write transaction: the - # outer read was advisory, and the wrapper serializes - # individual statements, not the read-decide-apply - # decision. Under the transaction's lock the version - # cannot change between the read and the stamp, and the - # loser of an interleaved race (a queued second caller - # re-running apply_migrations after the winner already - # migrated) re-reads here, sees the winner's stamp, and - # skips instead of re-applying a non-idempotent - # migration with a stale version. + step = migrations[target - 1] + if callable(step): + # Callable migrations own their transaction: some steps + # (table rebuilds) must run connection pragmas such as + # ``foreign_keys = OFF`` BEFORE opening one, which the + # runner's uniform transaction cannot express, and they + # stamp inside their own transaction so a failure rolls + # the statements and the version stamp back together. + # The version is re-read here under the migration lock: + # runners on one connection are serialized by it, so + # the version cannot change between this read and the + # callable's commit. in_tx_version = await read_schema_version(database) if in_tx_version > latest: raise RuntimeError( @@ -267,41 +388,39 @@ async def apply_migrations( f"{latest}); downgrades are not supported" ) if in_tx_version < target: - step = migrations[target - 1] - if callable(step): - await step(database) - applied_statements = 0 - else: + await step(database, target) + applied = True + else: + async with database.transaction(): + # Re-read the version INSIDE the write transaction: + # the outer read was advisory, and the wrapper + # serializes individual statements, not the + # read-decide-apply decision. Under the + # transaction's lock the version cannot change + # between the read and the stamp, and the loser of + # an interleaved race (a queued second caller + # re-running apply_migrations after the winner + # already migrated) re-reads here, sees the + # winner's stamp, and skips instead of re-applying + # a non-idempotent migration with a stale version. + in_tx_version = await read_schema_version(database) + if in_tx_version > latest: + raise RuntimeError( + f"Database schema version {in_tx_version} is " + f"newer than this integration understands " + f"(latest known: {latest}); downgrades are " + f"not supported" + ) + if in_tx_version < target: for sql in step: await database.execute(sql) + await stamp_schema_version(database, target) + applied = True applied_statements = len(step) - # INSERT keyed on the singleton row (id = 1): a - # fresh file has no row yet, and migration 1 is the - # only migration a version-0 database can run, so - # the row is seeded by the same statement batch - # when needed. UPDATE alone would silently match - # zero rows there. - await database.execute( - f"INSERT INTO {VERSION_TABLE} (id, version) " - "VALUES (1, ?) " - "ON CONFLICT (id) DO UPDATE SET version = " - "excluded.version", - (target,), - ) - applied = True if applied: - if applied_statements == 0: - LOGGER.info( - "NestQuest schema migrated to version %d", - target, - ) - else: - LOGGER.info( - "NestQuest schema migrated to version %d " - "(%d statement%s)", - target, - applied_statements, - "s" if applied_statements != 1 else "", - ) + LOGGER.info( + "NestQuest schema migrated to version %d", + target, + ) return latest \ No newline at end of file diff --git a/custom_components/nestquest/schema.py b/custom_components/nestquest/schema.py index d497358..63d2674 100644 --- a/custom_components/nestquest/schema.py +++ b/custom_components/nestquest/schema.py @@ -86,6 +86,15 @@ date range; a single-day override stores the same date in both columns. ``is_present`` 0/1 marks the child absent/present for the whole range; ``note`` is optional free text. +Multi-assignee definitions (D-008): ``quest_definitions`` carries no +``child_id`` column. Assignment lives in +``quest_definition_assignees``, one row per (definition, child) with a +composite primary key and foreign keys to the definition and the child, +so "brush teeth" is one definition covering three children. The +migration runner rebuilds a pre-D-008 table into this shape and copies +its single-assignee column across. The composite primary key makes +duplicate assignment impossible at the storage layer. + Quest instances: ``quest_instances`` deliberately carries NO completion status column. An instance's current state derives from the latest row in ``completion_events`` (Feature 08), so materialization (Feature 07) @@ -219,18 +228,34 @@ SCHEMA_V1_SCHEDULE_RULES_DDL: list[str] = [ """, ] +#: Column body of ``quest_definitions``, shared between the v1 DDL +#: below and the multi-assignee rebuild migration, which must recreate +#: the table in exactly this shape (D-008: one definition, many +#: assignees via ``quest_definition_assignees`` — no ``child_id`` +#: column on the definition itself). +QUEST_DEFINITIONS_TABLE_SQL = """( + id INTEGER PRIMARY KEY AUTOINCREMENT, + title TEXT NOT NULL, + description TEXT, + icon TEXT, + schedule_rule_id INTEGER NOT NULL REFERENCES schedule_rules(id), + due_time TEXT, + is_active INTEGER NOT NULL DEFAULT 1 CHECK (typeof(is_active) = 'integer' AND is_active IN (0, 1)), + created_at TEXT NOT NULL +)""" + SCHEMA_V1_QUEST_DEFINITIONS_DDL: list[str] = [ + f""" + CREATE TABLE IF NOT EXISTS quest_definitions {QUEST_DEFINITIONS_TABLE_SQL} + """, +] + +SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL: list[str] = [ """ - CREATE TABLE IF NOT EXISTS quest_definitions ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - title TEXT NOT NULL, - description TEXT, - icon TEXT, + CREATE TABLE IF NOT EXISTS quest_definition_assignees ( + definition_id INTEGER NOT NULL REFERENCES quest_definitions(id), child_id INTEGER NOT NULL REFERENCES children(id), - schedule_rule_id INTEGER NOT NULL REFERENCES schedule_rules(id), - due_time TEXT, - is_active INTEGER NOT NULL DEFAULT 1 CHECK (typeof(is_active) = 'integer' AND is_active IN (0, 1)), - created_at TEXT NOT NULL + PRIMARY KEY (definition_id, child_id) ) """, ] @@ -316,6 +341,7 @@ SCHEMA_V1_STATEMENTS: list[str] = [ *SCHEMA_V1_ADMIN_USERS_DDL, *SCHEMA_V1_SCHEDULE_RULES_DDL, *SCHEMA_V1_QUEST_DEFINITIONS_DDL, + *SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, *SCHEMA_V1_PRESENCE_SCHEDULES_DDL, *SCHEMA_V1_PRESENCE_OVERRIDES_DDL, *SCHEMA_V1_QUEST_INSTANCES_DDL, diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index 03ee03f..628752e 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -82,7 +82,7 @@ async def _prepare(path) -> tuple: child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) definition = await definitions.create( - "Brush teeth", child.id, rule.id, _now_stamp() + "Brush teeth", rule.id, _now_stamp(), assignee_child_ids=[child.id] ) return ( database, @@ -184,17 +184,15 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: with pytest.raises(ValueError, match="definition 999"): await instances.upsert(999, child.id, D1, _now_stamp()) # Unknown child: the definition-assignee check fires first and - # is the one that matters (an instance must carry its - # definition's assignee). To reach the child-existence check - # at all, the definition would have to claim child 999 — an - # FK-impossible state — so the child check only guards direct - # callers with a matching fake assignee; assert the guard - # order instead of forcing the FK-impossible state. + # is the one that matters (an instance must carry an assigned + # child). Under D-008 the assignee check is a membership test + # against quest_definition_assignees, so child 999 simply is + # not an assignee — the error names the pair. rule = await rules.create("daily", D1) other_definition = await definitions.create( - "X", child.id, rule.id, _now_stamp() + "X", rule.id, _now_stamp(), assignee_child_ids=[child.id] ) - with pytest.raises(ValueError, match="not 999"): + with pytest.raises(ValueError, match="not assigned to child 999"): await instances.upsert(other_definition.id, 999, D1, _now_stamp()) return None @@ -329,7 +327,10 @@ def test_upsert_no_past_check_uses_execution_date_not_call_date( child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) definition = await definitions.create( - "Brush teeth", child.id, rule.id, _now_stamp() + "Brush teeth", + rule.id, + _now_stamp(), + assignee_child_ids=[child.id], ) return await _body( database, children, rules, definitions, instances, @@ -406,7 +407,7 @@ def test_list_by_child_and_date_filters(tmp_path) -> None: other = await children.create("Bo", _now_stamp()) rule = await rules.create("daily", D1) other_definition = await definitions.create( - "Make bed", other.id, rule.id, _now_stamp() + "Make bed", rule.id, _now_stamp(), assignee_child_ids=[other.id] ) await instances.upsert(definition.id, child.id, D1, _now_stamp()) await instances.upsert(definition.id, child.id, D2, _now_stamp()) @@ -537,8 +538,8 @@ def test_delete_future_uncompleted_other_definitions_untouched( async def _body(database, children, rules, definitions, instances, events, child, definition): other = await definitions.create( - "Make bed", child.id, (await rules.create("daily", D1)).id, - _now_stamp(), + "Make bed", (await rules.create("daily", D1)).id, + _now_stamp(), assignee_child_ids=[child.id], ) await instances.upsert(definition.id, child.id, D20, _now_stamp()) await instances.upsert(other.id, child.id, D20, _now_stamp()) @@ -1090,7 +1091,7 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) definition = await definitions.create( - "Brush teeth", child.id, rule.id, _now_stamp() + "Brush teeth", rule.id, _now_stamp(), assignee_child_ids=[child.id] ) return (database, children, rules, definitions, instances, events, child, definition) diff --git a/tests/test_dao_matrix.py b/tests/test_dao_matrix.py index c14e65b..9a2b36a 100644 --- a/tests/test_dao_matrix.py +++ b/tests/test_dao_matrix.py @@ -71,6 +71,7 @@ ALL_TABLES = ( "admin_users", "schedule_rules", "quest_definitions", + "quest_definition_assignees", "presence_schedules", "presence_overrides", "quest_instances", @@ -99,7 +100,10 @@ class World: assert await self.admins.add("user-1", _stamp()) is True self.rule = await self.rules.create("daily", D1) self.definition = await self.definitions.create( - "Brush teeth", self.child.id, self.rule.id, _stamp() + "Brush teeth", + self.rule.id, + _stamp(), + assignee_child_ids=[self.child.id], ) self.schedule = await self.schedules.upsert_by_child( self.child.id, 2, D1, "0,2,4|1,3" @@ -206,6 +210,9 @@ def test_matrix_quest_definitions_crud_and_constraints(tmp_path) -> None: # insert (seeded) + read fetched = await w.definitions.get(w.definition.id) assert fetched.title == "Brush teeth" + assert [c.id for c in await w.definitions.list_assignees( + w.definition.id + )] == [w.child.id] # update await w.definitions.update(w.definition.id, title="Brush TEETH") assert (await w.definitions.get(w.definition.id)).title == ( @@ -213,11 +220,21 @@ def test_matrix_quest_definitions_crud_and_constraints(tmp_path) -> None: ) # constraint: FK to a nonexistent child rejected with ValueError with pytest.raises(ValueError, match="does not exist"): - await w.definitions.create("X", 999, w.rule.id, _stamp()) - # constraint: inactive child rejected + await w.definitions.create( + "X", w.rule.id, _stamp(), assignee_child_ids=[999] + ) + # constraint: inactive child rejected on assignment await w.children.set_active(w.child2.id, False) with pytest.raises(ValueError, match="inactive"): - await w.definitions.set_assignee(w.definition.id, w.child2.id) + await w.definitions.add_assignee(w.definition.id, w.child2.id) + # assignee removal + re-adding the active child + assert await w.definitions.remove_assignee( + w.definition.id, w.child.id + ) is True + assert await w.definitions.remove_assignee( + w.definition.id, w.child.id + ) is False + await w.definitions.add_assignee(w.definition.id, w.child.id) # set_active: deactivate, never delete assert await w.definitions.set_active(w.definition.id, False) == 1 assert (await w.definitions.get(w.definition.id)).is_active is False diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index 4d3bf04..e73af68 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -266,7 +266,7 @@ def test_rule_delete_unreferenced_succeeds(tmp_path) -> None: def test_rule_delete_referenced_is_rejected(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") - await definitions.create("Brush teeth", child.id, rule.id, NOW) + await definitions.create("Brush teeth", rule.id, NOW) assert await rules.delete_if_unreferenced(rule.id) is False fetched = await rules.get(rule.id) assert fetched is not None, ( @@ -288,7 +288,7 @@ def test_rule_delete_after_last_reference_removed_succeeds(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") definition = await definitions.create( - "Brush teeth", child.id, rule.id, NOW + "Brush teeth", rule.id, NOW ) assert await rules.delete_if_unreferenced(rule.id) is False await definitions.set_active(definition.id, False) @@ -335,12 +335,12 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( import custom_components.nestquest.dao_rules as dao_rules original_validate = ( - dao_rules.QuestDefinitionsDao._validate_assignable + dao_rules.QuestDefinitionsDao._validate_rule_exists ) release = asyncio.Event() started = asyncio.Event() - async def _pausing_validate(self, child_id, rule_id): + async def _pausing_validate(self, rule_id): # Run the real validation FIRST (this is the # time-of-check), THEN pause: the delete task gets # scheduled while the create sits between its @@ -348,19 +348,17 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( # transaction must keep the delete queued until the # insert commits, so the create must win the race # and the delete must be rejected as referenced. - await original_validate(self, child_id, rule_id) + await original_validate(self, rule_id) if rule_id == rule_a.id and not started.is_set(): started.set() await release.wait() - dao_rules.QuestDefinitionsDao._validate_assignable = ( + dao_rules.QuestDefinitionsDao._validate_rule_exists = ( _pausing_validate ) try: create_task = asyncio.ensure_future( - definitions.create( - "Brush teeth", child.id, rule_a.id, NOW - ) + definitions.create("Brush teeth", rule_a.id, NOW) ) await started.wait() delete_task = asyncio.ensure_future( @@ -382,7 +380,7 @@ def test_rule_delete_concurrent_with_definition_create_is_safe( async def _create(): return await definitions.create( - "Brush teeth", child.id, rule_a.id, NOW + "Brush teeth", rule_a.id, NOW ) delete_task = asyncio.ensure_future(_delete()) @@ -434,20 +432,24 @@ def test_definition_create_returns_typed_record(tmp_path) -> None: rule = await rules.create("weekly", "2026-09-14", weekday_set="0,2") definition = await definitions.create( "Brush teeth", - child.id, rule.id, NOW, description="Morning and night", icon="mdi:tooth", due_time="08:00", + assignee_child_ids=[child.id], ) assert isinstance(definition, QuestDefinitionRecord) assert definition.title == "Brush teeth" - assert definition.child_id == child.id assert definition.schedule_rule_id == rule.id assert definition.due_time == "08:00" assert definition.is_active is True assert definition.created_at == NOW + assert not hasattr(definition, "child_id"), ( + "D-008: assignment lives in quest_definition_assignees" + ) + assignees = await definitions.list_assignees(definition.id) + assert [c.id for c in assignees] == [child.id] return definition _with_db(tmp_path, "definition-create.db")(_body) @@ -460,26 +462,30 @@ def test_definition_get_missing_returns_none(tmp_path) -> None: assert _with_db(tmp_path, "definition-get-missing.db")(_body) is None -def test_definition_create_unknown_child_raises_value_error( +def test_definition_create_unknown_assignee_raises_value_error( tmp_path, ) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") with pytest.raises(ValueError, match="does not exist"): - await definitions.create("X", 999, rule.id, NOW) + await definitions.create( + "X", rule.id, NOW, assignee_child_ids=[999] + ) return None _with_db(tmp_path, "definition-bad-child.db")(_body) -def test_definition_create_inactive_child_raises_value_error( +def test_definition_create_inactive_assignee_raises_value_error( tmp_path, ) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") await children.set_active(child.id, False) with pytest.raises(ValueError, match="inactive"): - await definitions.create("X", child.id, rule.id, NOW) + await definitions.create( + "X", rule.id, NOW, assignee_child_ids=[child.id] + ) return None _with_db(tmp_path, "definition-inactive-child.db")(_body) @@ -490,7 +496,7 @@ def test_definition_create_unknown_rule_raises_value_error( ) -> None: async def _body(database, rules, definitions, children, child): with pytest.raises(ValueError, match="does not exist"): - await definitions.create("X", child.id, 999, NOW) + await definitions.create("X", 999, NOW) return None _with_db(tmp_path, "definition-bad-rule.db")(_body) @@ -505,9 +511,11 @@ def test_definition_list_by_child_filters_and_orders(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") other_child = await children.create("Bo", NOW) - await definitions.create("A", child.id, rule.id, NOW) - await definitions.create("B", child.id, rule.id, NOW) - await definitions.create("C", other_child.id, rule.id, NOW) + await definitions.create("A", rule.id, NOW, assignee_child_ids=[child.id]) + await definitions.create("B", rule.id, NOW, assignee_child_ids=[child.id]) + await definitions.create( + "C", rule.id, NOW, assignee_child_ids=[other_child.id] + ) mine = await definitions.list_by_child(child.id) assert [d.title for d in mine] == ["B", "A"] return mine @@ -527,8 +535,8 @@ def test_definition_list_by_child_missing_child_returns_empty( def test_definition_list_active_excludes_inactive(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") - first = await definitions.create("A", child.id, rule.id, NOW) - await definitions.create("B", child.id, rule.id, NOW) + first = await definitions.create("A", rule.id, NOW) + await definitions.create("B", rule.id, NOW) await definitions.set_active(first.id, False) active = await definitions.list_active() assert [d.title for d in active] == ["B"] @@ -547,7 +555,7 @@ def test_definition_update_fields_round_trip(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") definition = await definitions.create( - "A", child.id, rule.id, NOW, due_time="08:00" + "A", rule.id, NOW, due_time="08:00" ) assert await definitions.update( definition.id, @@ -572,9 +580,11 @@ def test_definition_update_cannot_change_assignee_or_active( async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") other = await children.create("Bo", NOW) - definition = await definitions.create("A", child.id, rule.id, NOW) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id] + ) # title= / child_id= / is_active= are NOT update() parameters; - # this asserts the API shape: reassignment and activation have + # this asserts the API shape: assignment and activation have # their own explicit methods. import inspect @@ -590,7 +600,10 @@ def test_definition_update_cannot_change_assignee_or_active( definition.id, title="A2" ) == 1 fetched = await definitions.get(definition.id) - assert fetched.child_id == child.id + assert [c.id for c in await definitions.list_assignees(definition.id)] == [ + child.id + ] + assert other.id not in [c.id for c in await definitions.list_assignees(definition.id)] assert fetched.is_active is True return fetched @@ -600,7 +613,7 @@ def test_definition_update_cannot_change_assignee_or_active( def test_definition_update_no_fields_returns_zero(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") - definition = await definitions.create("A", child.id, rule.id, NOW) + definition = await definitions.create("A", rule.id, NOW) return await definitions.update(definition.id) assert _with_db(tmp_path, "definition-update-empty.db")(_body) == 0 @@ -612,7 +625,6 @@ def test_definition_update_can_clear_optional_fields(tmp_path) -> None: rule = await rules.create("daily", "2026-09-14") definition = await definitions.create( "A", - child.id, rule.id, NOW, description="d", @@ -646,7 +658,7 @@ def test_definition_update_missing_returns_zero(tmp_path) -> None: def test_definition_set_active_round_trip(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") - definition = await definitions.create("A", child.id, rule.id, NOW) + definition = await definitions.create("A", rule.id, NOW) assert await definitions.set_active(definition.id, False) == 1 fetched = await definitions.get(definition.id) assert fetched.is_active is False @@ -683,76 +695,152 @@ def test_definition_no_delete_method_exists(tmp_path) -> None: _with_db(tmp_path, "definition-no-delete.db")(_body) -def test_definition_set_assignee_round_trip(tmp_path) -> None: +def test_definition_add_assignee_round_trip(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") other = await children.create("Bo", NOW) - definition = await definitions.create("A", child.id, rule.id, NOW) - assert await definitions.set_assignee(definition.id, other.id) == 1 - fetched = await definitions.get(definition.id) - assert fetched.child_id == other.id - return fetched + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id] + ) + await definitions.add_assignee(definition.id, other.id) + assignees = await definitions.list_assignees(definition.id) + assert sorted(c.id for c in assignees) == sorted([child.id, other.id]) + # Idempotent: re-adding an existing assignee stays a no-op. + await definitions.add_assignee(definition.id, other.id) + assignees = await definitions.list_assignees(definition.id) + assert sorted(c.id for c in assignees) == sorted([child.id, other.id]) + return assignees - _with_db(tmp_path, "definition-set-assignee.db")(_body) + _with_db(tmp_path, "definition-add-assignee.db")(_body) -def test_definition_set_assignee_inactive_child_raises(tmp_path) -> None: +def test_definition_add_assignee_inactive_child_raises(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") other = await children.create("Bo", NOW) - definition = await definitions.create("A", child.id, rule.id, NOW) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id] + ) await children.set_active(other.id, False) with pytest.raises(ValueError, match="inactive"): - await definitions.set_assignee(definition.id, other.id) - fetched = await definitions.get(definition.id) - assert fetched.child_id == child.id, ( - "failed reassignment must leave the current assignee intact" + await definitions.add_assignee(definition.id, other.id) + assignees = await definitions.list_assignees(definition.id) + assert [c.id for c in assignees] == [child.id], ( + "failed assignment must leave the current assignees intact" ) - return fetched + return assignees _with_db(tmp_path, "definition-assignee-inactive.db")(_body) -def test_definition_set_assignee_unknown_child_raises(tmp_path) -> None: +def test_definition_add_assignee_unknown_child_raises(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") - definition = await definitions.create("A", child.id, rule.id, NOW) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id] + ) with pytest.raises(ValueError, match="does not exist"): - await definitions.set_assignee(definition.id, 999) + await definitions.add_assignee(definition.id, 999) return None _with_db(tmp_path, "definition-assignee-unknown.db")(_body) -def test_definition_set_assignee_missing_definition_returns_zero( +def test_definition_add_assignee_missing_definition_raises(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + with pytest.raises(ValueError, match="does not exist"): + await definitions.add_assignee(999, child.id) + return None + + _with_db(tmp_path, "definition-assignee-missing.db")(_body) + + +def test_definition_remove_assignee_round_trip(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + other = await children.create("Bo", NOW) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id, other.id] + ) + assert await definitions.remove_assignee(definition.id, other.id) is True + assignees = await definitions.list_assignees(definition.id) + assert [c.id for c in assignees] == [child.id] + # Removing a non-assignee reports False. + assert await definitions.remove_assignee(definition.id, other.id) is False + return assignees + + _with_db(tmp_path, "definition-remove-assignee.db")(_body) + + +def test_definition_list_assignees_orders_by_child_sort_order( tmp_path, ) -> None: async def _body(database, rules, definitions, children, child): - other = await children.create("Bo", NOW) - return await definitions.set_assignee(999, other.id) + rule = await rules.create("daily", "2026-09-14") + second = await children.create("Bo", NOW) + third = await children.create("Cleo", NOW) + await children.reorder([third.id, child.id, second.id]) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[second.id, child.id, third.id] + ) + assignees = await definitions.list_assignees(definition.id) + assert [c.id for c in assignees] == [third.id, child.id, second.id] + return assignees - assert _with_db(tmp_path, "definition-assignee-missing.db")(_body) == 0 + _with_db(tmp_path, "definition-assignees-order.db")(_body) -def test_definition_set_assignee_leaves_history_rows_alone( +def test_definition_unassigned_has_no_assignees(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + definition = await definitions.create("A", rule.id, NOW) + assert await definitions.list_assignees(definition.id) == [] + return None + + _with_db(tmp_path, "definition-no-assignees.db")(_body) + + +def test_definition_shared_by_multiple_children_via_list_by_child( tmp_path, ) -> None: - """Reassignment writes the definition row only: any instances that - already exist keep their generation-time child_id (Feature 06 + """D-008: one definition covering several children appears for + every assignee through list_by_child, once each.""" + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + second = await children.create("Bo", NOW) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id, second.id] + ) + for assignee in (child, second): + mine = await definitions.list_by_child(assignee.id) + assert [d.id for d in mine] == [definition.id] + return definition + + _with_db(tmp_path, "definition-shared.db")(_body) + + +def test_definition_assignment_leaves_history_rows_alone( + tmp_path, +) -> None: + """Assignment edits write the assignees table only: any instances + that already exist keep their generation-time child_id (Feature 06 guardrail 'changes future instances only'). """ async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") other = await children.create("Bo", NOW) - definition = await definitions.create("A", child.id, rule.id, NOW) + definition = await definitions.create( + "A", rule.id, NOW, assignee_child_ids=[child.id] + ) # Simulate an already-generated instance (materialization owns - # this table, but its row shape proves reassignment isolation). + # this table, but its row shape proves assignment isolation). await database.execute( "INSERT INTO quest_instances (definition_id, child_id, " "due_date, generated_at) VALUES (?, ?, ?, ?)", (definition.id, child.id, "2026-09-15", NOW), ) - await definitions.set_assignee(definition.id, other.id) + await definitions.add_assignee(definition.id, other.id) + await definitions.remove_assignee(definition.id, child.id) row = await database.fetch_one( "SELECT child_id FROM quest_instances WHERE definition_id = ?", (definition.id,), @@ -801,7 +889,7 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: } sql_pattern = re.compile( r"(FROM|INTO|UPDATE|DELETE\s+FROM|JOIN)\s+[`'\"]*(\[)?" - r"(schedule_rules|quest_definitions)\b", + r"(schedule_rules|quest_definitions|quest_definition_assignees)\b", re.IGNORECASE, ) offenders: list[str] = [] @@ -813,7 +901,7 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: if sql_pattern.search(py.read_text()): offenders.append(relative) assert offenders == [], ( - f"SQL touching schedule_rules/quest_definitions leaked into: " + f"SQL touching schedule_rules/quest_definitions/assignees leaked: " f"{offenders}" ) @@ -853,7 +941,7 @@ def test_dao_instances_test_file_uses_dao_not_raw_rules_sql() -> None: ) sql_pattern = re.compile( r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" - r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions)\b", + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions|quest_definition_assignees)\b", re.IGNORECASE, ) assert sql_pattern.search(remaining) is None, ( @@ -898,7 +986,7 @@ def test_dao_rules_test_file_uses_dao_not_raw_table_sql() -> None: ) sql_pattern = re.compile( r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" - r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions)\b", + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions|quest_definition_assignees)\b", re.IGNORECASE, ) assert sql_pattern.search(remaining) is None, ( diff --git a/tests/test_migrations.py b/tests/test_migrations.py index f8ac70d..155d795 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -63,6 +63,7 @@ def _counts(database) -> dict[str, int]: "admin_users", "schedule_rules", "quest_definitions", + "quest_definition_assignees", "presence_schedules", "presence_overrides", "quest_instances", @@ -99,6 +100,7 @@ def test_fresh_file_migration_creates_all_v1_tables(tmp_path) -> None: "admin_users", "schedule_rules", "quest_definitions", + "quest_definition_assignees", "presence_schedules", "presence_overrides", "quest_instances", @@ -136,15 +138,18 @@ def test_empty_version_table_reads_as_version_0(tmp_path) -> None: def test_migration_list_shape() -> None: - """Migration 1 is the v1 DDL; migration 2 is the legacy rename step. + """Migration 1 is the v1 DDL; 2 and 3 are the callable steps. Nothing ever shipped, so migration 1 was rewritten pre-release (D-007) to create the quest_* tables directly; migration 2 exists - for dev databases stamped version 1 by the pre-rename runner. + for dev databases stamped version 1 by the pre-rename runner, and + migration 3 brings pre-D-008 definitions to the multi-assignee + model. """ assert MIGRATIONS[0] == SCHEMA_V1_STATEMENTS assert callable(MIGRATIONS[1]) - assert len(MIGRATIONS) == 2 + assert callable(MIGRATIONS[2]) + assert len(MIGRATIONS) == 3 # --------------------------------------------------------------------------- @@ -312,6 +317,20 @@ def test_legacy_task_tables_are_renamed_and_keep_their_rows(tmp_path) -> None: ) == ("2026-09-15",) ) + # Migration 3 moved the legacy single-assignee column into the + # assignees table: the definition kept its child, the column is + # gone. + assignees = _run( + database.fetch_one( + "SELECT child_id FROM quest_definition_assignees " + "WHERE definition_id = 1" + ) + ) + assert assignees == (1,) + columns = _run( + database.fetch_all("PRAGMA table_info(quest_definitions)") + ) + assert "child_id" not in {row[1] for row in columns} # The untouched completion_events table still exists and is empty. assert "completion_events" in tables assert ( @@ -380,6 +399,136 @@ def test_fresh_database_never_holds_legacy_table_names(tmp_path) -> None: _run(database.close()) +# --------------------------------------------------------------------------- +# Pre-D-008 database: quest_definitions with child_id stamped version 2 +# --------------------------------------------------------------------------- + +_V2_DEFINITIONS_DDL = [ + """ + CREATE TABLE IF NOT EXISTS quest_definitions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + title TEXT NOT NULL, + description TEXT, + icon TEXT, + child_id INTEGER NOT NULL REFERENCES children(id), + schedule_rule_id INTEGER NOT NULL REFERENCES schedule_rules(id), + due_time TEXT, + is_active INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL + ) + """, +] + + +def _seed_v2_multi_assignee_upgrade(database) -> None: + """Create a database shaped like a post-rename, pre-D-008 dev file. + + Fresh quest_* tables — but quest_definitions still carries the + single child_id column and no assignees table exists — stamped at + version 2, with definition and instance rows that must survive. + """ + for sql in SCHEMA_V1_STATEMENTS: + if "quest_definitions " in sql: + continue # replaced by the legacy-shape DDL below + _run(database.execute(sql)) + for sql in _V2_DEFINITIONS_DDL: + _run(database.execute(sql)) + _run(database.execute(VERSION_TABLE_DDL)) + _run( + database.execute( + f"INSERT INTO {VERSION_TABLE} (id, version) VALUES (1, 2)" + ) + ) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) " + "VALUES ('Ada', '2026-09-14T00:00:00+00:00')" + ) + ) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) " + "VALUES ('Bo', '2026-09-14T00:00:00+00:00')" + ) + ) + _run( + database.execute( + "INSERT INTO schedule_rules (rule_type, start_date) " + "VALUES ('daily', '2026-09-14')" + ) + ) + _run( + database.execute( + "INSERT INTO quest_definitions (title, child_id, " + "schedule_rule_id, created_at) " + "VALUES ('Dishes', 2, 1, '2026-09-14T00:00:00+00:00')" + ) + ) + _run( + database.execute( + "INSERT INTO quest_instances (definition_id, child_id, " + "due_date, generated_at) " + "VALUES (1, 2, '2026-09-15', '2026-09-14T00:00:00+00:00')" + ) + ) + + +def test_v2_definitions_rebuilt_to_multi_assignee(tmp_path) -> None: + database = _open_db(tmp_path / "v2-upgrade.db") + try: + _seed_v2_multi_assignee_upgrade(database) + final = _migrate(database) + assert final == len(MIGRATIONS) + assert _version(database) == len(MIGRATIONS) + # The definition row survived and lost its child_id column. + row = _run( + database.fetch_one( + "SELECT title, schedule_rule_id, is_active " + "FROM quest_definitions WHERE id = 1" + ) + ) + assert row == ("Dishes", 1, 1) + columns = _run( + database.fetch_all("PRAGMA table_info(quest_definitions)") + ) + assert "child_id" not in {c[1] for c in columns} + # The single assignee moved into the assignees table. + assignees = _run( + database.fetch_all( + "SELECT definition_id, child_id " + "FROM quest_definition_assignees ORDER BY child_id" + ) + ) + assert assignees == [(1, 2)] + # The instance survived, still referencing the definition. + instance = _run( + database.fetch_one( + "SELECT definition_id, child_id, due_date " + "FROM quest_instances WHERE id = 1" + ) + ) + assert instance == (1, 2, "2026-09-15") + # No dangling references anywhere. + assert _run(database.fetch_all("PRAGMA foreign_key_check")) == [] + # The rebuilt table is live for the new model: a definition + # insert carries no child column at all. + _run( + database.execute( + "INSERT INTO quest_definitions (title, schedule_rule_id, " + "created_at) VALUES ('Laundry', 1, " + "'2026-09-14T00:00:00+00:00')" + ) + ) + _run( + database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (2, 1)" + ) + ) + finally: + _run(database.close()) + + # --------------------------------------------------------------------------- # Idempotence: second start is a no-op # --------------------------------------------------------------------------- @@ -481,22 +630,28 @@ def test_noop_run_is_logged_at_debug_level(caplog, tmp_path) -> None: def _failing_migrations(prior_statements: list[str]) -> list[list[str]]: - """Three-migration list where migration 3 fails mid-application. + """Migration list where the LAST entry fails mid-application. - Migration 2 is a harmless no-op filler so the real runner — which - has already stamped version 2 by the time these tests re-run it — - still has a pending migration to fail on. + Sized one entry longer than the real MIGRATIONS and padded with + harmless no-op fillers, so a fully-migrated database (already at + len(MIGRATIONS)) still has exactly one pending migration — the + failing one — to attempt. """ - return [ - prior_statements, + steps: list[list[str]] = [prior_statements] + steps += [ [ - "CREATE TABLE IF NOT EXISTS filler_table (id INTEGER PRIMARY KEY)" - ], + f"CREATE TABLE IF NOT EXISTS filler_{index} " + "(id INTEGER PRIMARY KEY)" + ] + for index in range(len(MIGRATIONS) - 1) + ] + steps.append( [ "CREATE TABLE IF NOT EXISTS later_table (id INTEGER PRIMARY KEY)", "CREATE TABLE broken (", - ], - ] + ] + ) + return steps def test_failing_migration_rolls_back_to_prior_version(tmp_path) -> None: diff --git a/tests/test_schema.py b/tests/test_schema.py index b95e68b..58e65de 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -16,6 +16,7 @@ from custom_components.nestquest.schema import ( SCHEMA_V1_SCHEDULE_RULES_DDL, SCHEMA_V1_STATEMENTS, SCHEMA_V1_QUEST_DEFINITIONS_DDL, + SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, SCHEMA_V1_QUEST_INSTANCES_DDL, async_apply_ddl, ) @@ -73,15 +74,17 @@ def _insert_rule(database, **overrides): return result.lastrowid -def _insert_definition(database, child_id=1, rule_id=1, **overrides): - """Insert one valid task_definition, returning the definition id. +def _insert_definition(database, rule_id=1, assignees=(1,), **overrides): + """Insert one valid quest_definition, returning the definition id. + ``assignees`` rows are written to ``quest_definition_assignees`` + after the definition insert (the definition itself carries no + child). Pass ``assignees=None`` for an unassigned definition. Pass ``_OMIT`` as a value to leave that column out of the INSERT so its DDL DEFAULT applies. """ values: dict[str, object] = { "title": "Brush teeth", - "child_id": child_id, "schedule_rule_id": rule_id, "created_at": "2026-09-13T00:00:00+00:00", } @@ -96,6 +99,14 @@ def _insert_definition(database, child_id=1, rule_id=1, **overrides): ) ) assert result.lastrowid is not None + for child_id in assignees or (): + _run( + database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (?, ?)", + (result.lastrowid, child_id), + ) + ) return result.lastrowid @@ -156,12 +167,13 @@ def _insert_override(database, child_id, **overrides): # --------------------------------------------------------------------------- -def test_schema_v1_statements_compose_the_six_tables() -> None: +def test_schema_v1_statements_compose_the_seven_tables() -> None: assert SCHEMA_V1_STATEMENTS == [ *SCHEMA_V1_CHILDREN_DDL, *SCHEMA_V1_ADMIN_USERS_DDL, *SCHEMA_V1_SCHEDULE_RULES_DDL, *SCHEMA_V1_QUEST_DEFINITIONS_DDL, + *SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, *SCHEMA_V1_PRESENCE_SCHEDULES_DDL, *SCHEMA_V1_PRESENCE_OVERRIDES_DDL, *SCHEMA_V1_QUEST_INSTANCES_DDL, @@ -302,7 +314,6 @@ def test_quest_definitions_columns_types_and_constraints(tmp_path) -> None: ("title", "TEXT", 1, None, 0), ("description", "TEXT", 0, None, 0), ("icon", "TEXT", 0, None, 0), - ("child_id", "INTEGER", 1, None, 0), ("schedule_rule_id", "INTEGER", 1, None, 0), ("due_time", "TEXT", 0, None, 0), ("is_active", "INTEGER", 1, "1", 0), @@ -319,13 +330,66 @@ def test_quest_definitions_foreign_keys_declared(tmp_path) -> None: fks = _run(database.fetch_all("PRAGMA foreign_key_list(quest_definitions)")) declared = {(row[2], row[3], row[4]) for row in fks} assert declared == { - ("children", "child_id", "id"), ("schedule_rules", "schedule_rule_id", "id"), } finally: _run(database.close()) +def test_quest_definition_assignees_table_exists_after_applying_ddl( + tmp_path, +) -> None: + database = _open_db(tmp_path / "assignees.db") + try: + _apply(database) + row = _run( + database.fetch_one( + "SELECT name FROM sqlite_master WHERE type = 'table' " + "AND name = 'quest_definition_assignees'" + ) + ) + assert row is not None and row[0] == "quest_definition_assignees" + finally: + _run(database.close()) + + +def test_quest_definition_assignees_columns_types_and_constraints( + tmp_path, +) -> None: + database = _open_db(tmp_path / "assignees-columns.db") + try: + _apply(database) + columns = _run( + database.fetch_all("PRAGMA table_info(quest_definition_assignees)") + ) + # cid, name, type, notnull, dflt_value, pk — both columns are + # the two-part primary key, so pk is 1 and 2. + assert [(row[1], row[2], row[3], row[4], row[5]) for row in columns] == [ + ("definition_id", "INTEGER", 1, None, 1), + ("child_id", "INTEGER", 1, None, 2), + ] + finally: + _run(database.close()) + + +def test_quest_definition_assignees_foreign_keys_declared(tmp_path) -> None: + database = _open_db(tmp_path / "assignees-fks.db") + try: + _apply(database) + fks = _run( + database.fetch_all( + "PRAGMA foreign_key_list(quest_definition_assignees)" + ) + ) + declared = {(row[2], row[3], row[4]) for row in fks} + assert declared == { + ("quest_definitions", "definition_id", "id"), + ("children", "child_id", "id"), + } + finally: + _run(database.close()) + + # --------------------------------------------------------------------------- # Constraint enforcement # --------------------------------------------------------------------------- @@ -826,28 +890,114 @@ def test_quest_definitions_insert_requires_existing_child_and_rule( ) ) rule_id = _insert_rule(database) - definition_id = _insert_definition( - database, child_id=1, rule_id=rule_id - ) + definition_id = _insert_definition(database, rule_id=rule_id) row = _run( database.fetch_one( - "SELECT title, child_id, schedule_rule_id FROM quest_definitions " + "SELECT title, schedule_rule_id FROM quest_definitions " "WHERE id = ?", (definition_id,), ) ) - assert row == ("Brush teeth", 1, rule_id) + assert row == ("Brush teeth", rule_id) + assignee = _run( + database.fetch_one( + "SELECT child_id FROM quest_definition_assignees " + "WHERE definition_id = ?", + (definition_id,), + ) + ) + assert assignee == (1,) finally: _run(database.close()) -def test_quest_definitions_unknown_child_id_fails(tmp_path) -> None: - database = _open_db(tmp_path / "definitions-bad-child.db") +def test_quest_definition_assignees_unknown_child_fails(tmp_path) -> None: + database = _open_db(tmp_path / "assignees-bad-child.db") try: _apply(database) _insert_rule(database) + definition_id = _insert_definition(database, assignees=None) with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"): - _insert_definition(database, child_id=999) + _run( + database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (?, 999)", + (definition_id,), + ) + ) + finally: + _run(database.close()) + + +def test_quest_definition_assignees_unknown_definition_fails(tmp_path) -> None: + database = _open_db(tmp_path / "assignees-bad-definition.db") + try: + _apply(database) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) VALUES (?, ?)", + ("Ada", "2026-09-13T00:00:00+00:00"), + ) + ) + with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"): + _run( + database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (999, 1)" + ) + ) + finally: + _run(database.close()) + + +def test_quest_definition_assignees_duplicate_pair_fails(tmp_path) -> None: + database = _open_db(tmp_path / "assignees-duplicate.db") + try: + _apply(database) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) VALUES (?, ?)", + ("Ada", "2026-09-13T00:00:00+00:00"), + ) + ) + _insert_rule(database) + definition_id = _insert_definition(database) + with pytest.raises(sqlite3.IntegrityError): + _run( + database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (?, 1)", + (definition_id,), + ) + ) + finally: + _run(database.close()) + + +def test_quest_definition_assignees_multiple_children_allowed( + tmp_path, +) -> None: + database = _open_db(tmp_path / "assignees-multi.db") + try: + _apply(database) + for name in ("Ada", "Ben", "Cleo"): + _run( + database.execute( + "INSERT INTO children (display_name, created_at) " + "VALUES (?, ?)", + (name, "2026-09-13T00:00:00+00:00"), + ) + ) + _insert_rule(database) + definition_id = _insert_definition(database, assignees=(1, 2, 3)) + rows = _run( + database.fetch_all( + "SELECT child_id FROM quest_definition_assignees " + "WHERE definition_id = ? ORDER BY child_id", + (definition_id,), + ) + ) + assert [row[0] for row in rows] == [1, 2, 3] finally: _run(database.close()) @@ -863,7 +1013,7 @@ def test_quest_definitions_unknown_schedule_rule_id_fails(tmp_path) -> None: ) ) with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"): - _insert_definition(database, child_id=1, rule_id=999) + _insert_definition(database, rule_id=999) finally: _run(database.close()) @@ -874,7 +1024,7 @@ def test_quest_definitions_is_active_check_enforced(tmp_path) -> None: _apply(database) _insert_rule(database) with pytest.raises(sqlite3.IntegrityError, match="CHECK"): - _insert_definition(database, child_id=1, rule_id=1, is_active=2) + _insert_definition(database, rule_id=1, is_active=2) finally: _run(database.close()) @@ -895,7 +1045,7 @@ def test_quest_definitions_is_active_non_integer_fails( _insert_rule(database) with pytest.raises(sqlite3.IntegrityError, match="CHECK"): _insert_definition( - database, child_id=1, rule_id=1, is_active=bad_is_active + database, rule_id=1, is_active=bad_is_active ) finally: _run(database.close()) @@ -907,7 +1057,7 @@ def test_quest_definitions_title_not_null_enforced(tmp_path) -> None: _apply(database) _insert_rule(database) with pytest.raises(sqlite3.IntegrityError, match="NOT NULL"): - _insert_definition(database, child_id=1, rule_id=1, title=None) + _insert_definition(database, rule_id=1, title=None) finally: _run(database.close()) @@ -919,7 +1069,7 @@ def test_quest_definitions_created_at_not_null_enforced(tmp_path) -> None: _insert_rule(database) with pytest.raises(sqlite3.IntegrityError, match="NOT NULL"): _insert_definition( - database, child_id=1, rule_id=1, created_at=None + database, rule_id=1, created_at=None ) finally: _run(database.close()) @@ -937,7 +1087,7 @@ def test_quest_definitions_defaults_apply_on_insert(tmp_path) -> None: ) _insert_rule(database) definition_id = _insert_definition( - database, child_id=1, rule_id=1, is_active=_OMIT + database, rule_id=1, is_active=_OMIT ) row = _run( database.fetch_one( @@ -966,7 +1116,7 @@ def test_applying_extended_ddl_twice_keeps_new_table_rows(tmp_path) -> None: ) ) _insert_rule(database) - _insert_definition(database, child_id=1, rule_id=1) + _insert_definition(database, rule_id=1) _apply(database) counts = [] for table in ("children", "schedule_rules", "quest_definitions"): @@ -1469,7 +1619,7 @@ def _setup_definition(database) -> None: """Create one child, one schedule rule and one quest definition.""" _child(database) _insert_rule(database) - _insert_definition(database, child_id=1, rule_id=1) + _insert_definition(database, rule_id=1) # --------------------------------------------------------------------------- @@ -1675,7 +1825,7 @@ def test_quest_instances_same_date_different_definitions_allowed( ) ) rule_id = _insert_rule(database) - _insert_definition(database, child_id=2, rule_id=rule_id) + _insert_definition(database, rule_id=rule_id, assignees=(2,)) _insert_instance(database, definition_id=1, due_date="2026-09-14") _insert_instance(database, definition_id=2, due_date="2026-09-14") row = _run(database.fetch_one("SELECT COUNT(*) FROM quest_instances")) From ebc20642221c5a1017d7e6ff56fae92546b886ab Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 10:05:43 -0400 Subject: [PATCH 3/5] Quest windows via quest_definition_windows (D-008) Per IMPLEMENTATION-ORDER.md 1.4: a quest_definition_windows table (definition_id, window, due_time) declares which day windows a definition spans, one row per definition+window via the composite primary key; window is CHECK-constrained to morning/afternoon/evening. - const.py: WINDOW_MORNING/AFTERNOON/EVENING, QUEST_WINDOWS canonical order, WINDOW_CLOCK_RANGES with the documented boundaries and the boundary-to-later-window rule documented for Feature 07. - schema.py: windows DDL joins SCHEMA_V1_STATEMENTS; due_time is caller-validated strict HH:MM (DAO layer), matching the date policy. - migrations.py: migration 4 is a plain CREATE IF NOT EXISTS statement list (no rebuild needed for a new table). - dao_rules.py: QuestDefinitionsDao gains upsert_window (idempotent per window, due_time refresh in place), list_windows (const order), remove_window; _validate_time/_validate_window helpers. - tests: windows DDL shape/constraints (CHECK matrix, duplicate, unknown definition), DAO round-trips, const range shapes; leak guards extended to the new table. --- custom_components/nestquest/const.py | 25 ++++ custom_components/nestquest/dao_rules.py | 137 ++++++++++++++++++ custom_components/nestquest/migrations.py | 5 + custom_components/nestquest/schema.py | 21 +++ tests/test_const.py | 30 ++++ tests/test_dao_rules.py | 97 ++++++++++++- tests/test_migrations.py | 16 ++- tests/test_schema.py | 164 +++++++++++++++++++++- 8 files changed, 486 insertions(+), 9 deletions(-) diff --git a/custom_components/nestquest/const.py b/custom_components/nestquest/const.py index fc45d7c..b0b4daa 100644 --- a/custom_components/nestquest/const.py +++ b/custom_components/nestquest/const.py @@ -20,3 +20,28 @@ DEFAULT_DAY_ROLLOVER_TIME = "00:00" CONF_PANEL_IDLE_TIMEOUT = "panel_idle_timeout" DEFAULT_PANEL_IDLE_TIMEOUT = 300 + +# Quest windows (D-008): a definition may declare several day windows; +# each produces its own instance per day and groups the panel's Quest +# Log into three columns. The clock ranges are stored here so they stay +# configurable later. Boundaries: a window's end equals the next +# window's start; classification (Feature 07 materialisation) treats a +# due time exactly on the boundary as belonging to the LATER window, so +# 12:00 is afternoon (not morning) and 17:00 is evening (not +# afternoon). Times after 21:00 belong to no window. +WINDOW_MORNING = "morning" +WINDOW_AFTERNOON = "afternoon" +WINDOW_EVENING = "evening" + +QUEST_WINDOWS: tuple[str, ...] = ( + WINDOW_MORNING, + WINDOW_AFTERNOON, + WINDOW_EVENING, +) + +#: Inclusive "HH:MM" bounds per window, in 24-hour local time. +WINDOW_CLOCK_RANGES: dict[str, tuple[str, str]] = { + WINDOW_MORNING: ("00:00", "11:59"), + WINDOW_AFTERNOON: ("12:00", "17:00"), + WINDOW_EVENING: ("17:00", "21:00"), +} diff --git a/custom_components/nestquest/dao_rules.py b/custom_components/nestquest/dao_rules.py index 291febb..d5090ee 100644 --- a/custom_components/nestquest/dao_rules.py +++ b/custom_components/nestquest/dao_rules.py @@ -36,6 +36,7 @@ from __future__ import annotations import datetime from dataclasses import dataclass +from .const import QUEST_WINDOWS from .dao_children import ChildRecord, _child_from_row, _connection_lock from .dao_children import _CHILD_COLUMNS from .db import NestQuestDatabase @@ -71,6 +72,38 @@ def _validate_date(value: str, field: str) -> None: ) +_TIME_FORMAT = "%H:%M" + + +def _validate_time(value: str, field: str) -> None: + """Raise ValueError unless ``value`` is a strict 24-hour HH:MM. + + Same strictness policy as _validate_date: parse with the exact + format, then round-trip so '9:30' (non-padded) is rejected. Time + shapes are caller-side policy (the schema stores TEXT): windows' + optional ``due_time`` and definition ``due_time`` both use this. + """ + try: + parsed = datetime.datetime.strptime(value, _TIME_FORMAT).time() + except (TypeError, ValueError): + raise ValueError( + f"{field} must be a 24-hour HH:MM time, got {value!r}" + ) from None + if parsed.strftime(_TIME_FORMAT) != value: + raise ValueError( + f"{field} must be a strict HH:MM time, got {value!r}" + ) + + +def _validate_window(window: str) -> None: + """Raise ValueError unless ``window`` is a ``const`` window name.""" + if window not in QUEST_WINDOWS: + raise ValueError( + f"window must be one of {', '.join(QUEST_WINDOWS)}, " + f"got {window!r}" + ) + + @dataclass(frozen=True) class ScheduleRuleRecord: """One row of ``schedule_rules``.""" @@ -100,6 +133,15 @@ class QuestDefinitionRecord: created_at: str +@dataclass(frozen=True) +class QuestDefinitionWindowRecord: + """One row of ``quest_definition_windows``.""" + + definition_id: int + window: str + due_time: str | None + + _RULE_COLUMNS = ( "id, rule_type, interval, weekday_set, day_of_month, nth_weekday, " "month, start_date, end_date" @@ -108,6 +150,7 @@ _DEFINITION_COLUMNS = ( "id, title, description, icon, schedule_rule_id, due_time, " "is_active, created_at" ) +_WINDOW_COLUMNS = "definition_id, window, due_time" def _rule_from_row(row: tuple) -> ScheduleRuleRecord: @@ -137,6 +180,14 @@ def _definition_from_row(row: tuple) -> QuestDefinitionRecord: ) +def _window_from_row(row: tuple) -> QuestDefinitionWindowRecord: + return QuestDefinitionWindowRecord( + definition_id=row[0], + window=row[1], + due_time=row[2], + ) + + class ScheduleRulesDao: """Typed async access to the ``schedule_rules`` table.""" @@ -507,6 +558,92 @@ class QuestDefinitionsDao: ) return [_child_from_row(row) for row in rows] + async def upsert_window( + self, + definition_id: int, + window: str, + *, + due_time: str | None = None, + ) -> QuestDefinitionWindowRecord: + """Declare (or re-declare) ``window`` on the definition. + + The composite primary key makes the write idempotent per + (definition, window): re-declaring updates ``due_time`` in + place instead of duplicating. The window name must be one of + ``const.QUEST_WINDOWS`` (the schema CHECK backs this up) and a + provided ``due_time`` must be strict 24-hour HH:MM. The + definition must exist — validated under the connection lock so + the verdict cannot go stale before the write. + """ + _validate_window(window) + if due_time is not None: + _validate_time(due_time, "due_time") + async with _connection_lock(self._database): + async with self._database.transaction(): + definition = await self._database.fetch_one( + "SELECT 1 FROM quest_definitions WHERE id = ?", + (definition_id,), + ) + if definition is None: + raise ValueError( + f"quest definition {definition_id} does not exist" + ) + await self._database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) VALUES (?, ?, ?) " + "ON CONFLICT (definition_id, window) DO UPDATE SET " + "due_time = excluded.due_time", + (definition_id, window, due_time), + ) + row = await self._database.fetch_one( + f"SELECT {_WINDOW_COLUMNS} " + "FROM quest_definition_windows " + "WHERE definition_id = ? AND window = ?", + (definition_id, window), + ) + assert row is not None + return _window_from_row(row) + + async def list_windows( + self, definition_id: int + ) -> list[QuestDefinitionWindowRecord]: + """Return the definition's windows in ``const`` order. + + Ordered by the canonical window sequence (morning, afternoon, + evening) rather than insertion order, so callers always render + the Quest Log's three columns consistently. A definition with + no windows returns []. + """ + rows = await self._database.fetch_all( + f"SELECT {_WINDOW_COLUMNS} FROM quest_definition_windows " + "WHERE definition_id = ?", + (definition_id,), + ) + by_name = {row[1]: _window_from_row(row) for row in rows} + return [ + by_name[name] + for name in QUEST_WINDOWS + if name in by_name + ] + + async def remove_window( + self, definition_id: int, window: str + ) -> bool: + """Remove the window declaration; True when a row was removed. + + Future materialization stops producing instances for this + (definition, window); existing instances and completion history + are never touched here (Feature 06 guardrail). + """ + _validate_window(window) + async with _connection_lock(self._database): + result = await self._database.execute( + "DELETE FROM quest_definition_windows " + "WHERE definition_id = ? AND window = ?", + (definition_id, window), + ) + return result.rowcount > 0 + async def _validate_assignable_child(self, child_id: int) -> None: """Raise ValueError unless the child exists and is active. diff --git a/custom_components/nestquest/migrations.py b/custom_components/nestquest/migrations.py index 0b000ef..29d5d20 100644 --- a/custom_components/nestquest/migrations.py +++ b/custom_components/nestquest/migrations.py @@ -73,6 +73,7 @@ from .schema import ( QUEST_DEFINITIONS_TABLE_SQL, SCHEMA_V1_STATEMENTS, SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, + SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL, ) LOGGER = logging.getLogger(__name__) @@ -256,6 +257,10 @@ MIGRATIONS: Sequence[MigrationStep] = [ MIGRATION_1_V1_DDL, _rename_legacy_task_tables, _add_definition_assignees, + # Migration 4 (D-008): the windows table is a plain new table — a + # CREATE IF NOT EXISTS suffices for both fresh databases (which + # already made it in migration 1) and pre-window databases. + list(SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL), ] diff --git a/custom_components/nestquest/schema.py b/custom_components/nestquest/schema.py index 63d2674..7a73340 100644 --- a/custom_components/nestquest/schema.py +++ b/custom_components/nestquest/schema.py @@ -95,6 +95,15 @@ migration runner rebuilds a pre-D-008 table into this shape and copies its single-assignee column across. The composite primary key makes duplicate assignment impossible at the storage layer. +Quest windows (D-008): ``quest_definition_windows`` declares which day +windows a definition spans — one row per (definition, window), the +composite primary key making a window idempotent per definition. +``window`` is CHECK-constrained to the three spellings ``const`` owns; +``due_time`` optionally pins a due time inside the window and is +caller-validated (strict HH:MM) by the DAO layer, matching the date +policy above. The window clock ranges live in ``const.py``, not here: +this layer stores what the caller declares and never interprets clocks. + Quest instances: ``quest_instances`` deliberately carries NO completion status column. An instance's current state derives from the latest row in ``completion_events`` (Feature 08), so materialization (Feature 07) @@ -260,6 +269,17 @@ SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL: list[str] = [ """, ] +SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL: list[str] = [ + """ + CREATE TABLE IF NOT EXISTS quest_definition_windows ( + definition_id INTEGER NOT NULL REFERENCES quest_definitions(id), + window TEXT NOT NULL CHECK (window IN ('morning', 'afternoon', 'evening')), + due_time TEXT, + PRIMARY KEY (definition_id, window) + ) + """, +] + SCHEMA_V1_PRESENCE_SCHEDULES_DDL: list[str] = [ f""" CREATE TABLE IF NOT EXISTS presence_schedules ( @@ -342,6 +362,7 @@ SCHEMA_V1_STATEMENTS: list[str] = [ *SCHEMA_V1_SCHEDULE_RULES_DDL, *SCHEMA_V1_QUEST_DEFINITIONS_DDL, *SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, + *SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL, *SCHEMA_V1_PRESENCE_SCHEDULES_DDL, *SCHEMA_V1_PRESENCE_OVERRIDES_DDL, *SCHEMA_V1_QUEST_INSTANCES_DDL, diff --git a/tests/test_const.py b/tests/test_const.py index aea1bd9..85ff105 100644 --- a/tests/test_const.py +++ b/tests/test_const.py @@ -73,6 +73,36 @@ def test_panel_idle_timeout_config() -> None: assert const.DEFAULT_PANEL_IDLE_TIMEOUT > 0 +def test_quest_windows_canonical_order() -> None: + """QUEST_WINDOWS is exactly the three spellings in canonical order.""" + assert const.QUEST_WINDOWS == ("morning", "afternoon", "evening") + assert const.WINDOW_MORNING == "morning" + assert const.WINDOW_AFTERNOON == "afternoon" + assert const.WINDOW_EVENING == "evening" + # No duplicates, every name covered by its individual constant. + assert len(set(const.QUEST_WINDOWS)) == 3 + + +def test_window_clock_ranges_cover_every_window() -> None: + """Each window has a strict HH:MM (start, end) range. + + The ranges follow the documented boundaries (D-008): morning ends + 11:59, afternoon spans 12:00-17:00, evening 17:00-21:00, and each + window's end equals the next window's start (the boundary belongs + to the later window at classification time). + """ + assert const.WINDOW_CLOCK_RANGES == { + "morning": ("00:00", "11:59"), + "afternoon": ("12:00", "17:00"), + "evening": ("17:00", "21:00"), + } + for window in const.QUEST_WINDOWS: + start, end = const.WINDOW_CLOCK_RANGES[window] + for value in (start, end): + parsed = datetime.strptime(value, "%H:%M").time() + assert parsed.strftime("%H:%M") == value + + def test_no_hardcoded_domain_or_db_filename_in_modules() -> None: """Ensure no module other than const.py hardcodes DOMAIN string or SQLITE_DB_FILENAME.""" pkg_dir = Path(const.__file__).parent diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index e73af68..0ae00b6 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -11,6 +11,7 @@ from custom_components.nestquest.dao_rules import ( ScheduleRuleRecord, ScheduleRulesDao, QuestDefinitionRecord, + QuestDefinitionWindowRecord, QuestDefinitionsDao, ) from custom_components.nestquest.db import NestQuestDatabase @@ -790,6 +791,96 @@ def test_definition_list_assignees_orders_by_child_sort_order( _with_db(tmp_path, "definition-assignees-order.db")(_body) +def test_definition_upsert_window_round_trip(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + definition = await definitions.create("A", rule.id, NOW) + window = await definitions.upsert_window( + definition.id, "morning", due_time="09:00" + ) + assert window == QuestDefinitionWindowRecord( + definition.id, "morning", "09:00" + ) + # Upsert: re-declaring the same window updates in place, never + # duplicates (one row per definition+window). + updated = await definitions.upsert_window( + definition.id, "morning", due_time="09:30" + ) + assert updated.due_time == "09:30" + rows = await definitions.list_windows(definition.id) + assert [w.window for w in rows] == ["morning"] + # All three windows list in const order regardless of insertion. + await definitions.upsert_window(definition.id, "evening") + await definitions.upsert_window(definition.id, "afternoon") + rows = await definitions.list_windows(definition.id) + assert [w.window for w in rows] == [ + "morning", + "afternoon", + "evening", + ] + assert rows[1].due_time is None + return rows + + _with_db(tmp_path, "definition-window-roundtrip.db")(_body) + + +def test_definition_window_unknown_name_raises(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + definition = await definitions.create("A", rule.id, NOW) + with pytest.raises(ValueError, match="window must be one of"): + await definitions.upsert_window(definition.id, "noon") + with pytest.raises(ValueError, match="window must be one of"): + await definitions.remove_window(definition.id, "MORNING") + return None + + _with_db(tmp_path, "definition-window-bad-name.db")(_body) + + +def test_definition_window_bad_due_time_raises(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + definition = await definitions.create("A", rule.id, NOW) + with pytest.raises(ValueError, match="HH:MM"): + await definitions.upsert_window( + definition.id, "morning", due_time="9:30" + ) + with pytest.raises(ValueError, match="HH:MM"): + await definitions.upsert_window( + definition.id, "morning", due_time="25:00" + ) + return None + + _with_db(tmp_path, "definition-window-bad-time.db")(_body) + + +def test_definition_window_missing_definition_raises(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + with pytest.raises(ValueError, match="does not exist"): + await definitions.upsert_window(999, "morning") + return None + + _with_db(tmp_path, "definition-window-missing.db")(_body) + + +def test_definition_remove_window_round_trip(tmp_path) -> None: + async def _body(database, rules, definitions, children, child): + rule = await rules.create("daily", "2026-09-14") + definition = await definitions.create("A", rule.id, NOW) + await definitions.upsert_window(definition.id, "morning") + assert await definitions.remove_window( + definition.id, "morning" + ) is True + assert await definitions.list_windows(definition.id) == [] + # Removing an absent window reports False. + assert await definitions.remove_window( + definition.id, "morning" + ) is False + return None + + _with_db(tmp_path, "definition-window-remove.db")(_body) + + def test_definition_unassigned_has_no_assignees(tmp_path) -> None: async def _body(database, rules, definitions, children, child): rule = await rules.create("daily", "2026-09-14") @@ -889,7 +980,7 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: } sql_pattern = re.compile( r"(FROM|INTO|UPDATE|DELETE\s+FROM|JOIN)\s+[`'\"]*(\[)?" - r"(schedule_rules|quest_definitions|quest_definition_assignees)\b", + r"(schedule_rules|quest_definitions|quest_definition_assignees|quest_definition_windows)\b", re.IGNORECASE, ) offenders: list[str] = [] @@ -941,7 +1032,7 @@ def test_dao_instances_test_file_uses_dao_not_raw_rules_sql() -> None: ) sql_pattern = re.compile( r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" - r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions|quest_definition_assignees)\b", + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions|quest_definition_assignees|quest_definition_windows)\b", re.IGNORECASE, ) assert sql_pattern.search(remaining) is None, ( @@ -986,7 +1077,7 @@ def test_dao_rules_test_file_uses_dao_not_raw_table_sql() -> None: ) sql_pattern = re.compile( r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" - r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions|quest_definition_assignees)\b", + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|quest_definitions|quest_definition_assignees|quest_definition_windows)\b", re.IGNORECASE, ) assert sql_pattern.search(remaining) is None, ( diff --git a/tests/test_migrations.py b/tests/test_migrations.py index 155d795..ce81da3 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -17,7 +17,10 @@ from custom_components.nestquest.migrations import ( apply_migrations, read_schema_version, ) -from custom_components.nestquest.schema import SCHEMA_V1_STATEMENTS +from custom_components.nestquest.schema import ( + SCHEMA_V1_STATEMENTS, + SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL, +) def _run(coro): @@ -64,6 +67,7 @@ def _counts(database) -> dict[str, int]: "schedule_rules", "quest_definitions", "quest_definition_assignees", + "quest_definition_windows", "presence_schedules", "presence_overrides", "quest_instances", @@ -101,6 +105,7 @@ def test_fresh_file_migration_creates_all_v1_tables(tmp_path) -> None: "schedule_rules", "quest_definitions", "quest_definition_assignees", + "quest_definition_windows", "presence_schedules", "presence_overrides", "quest_instances", @@ -138,18 +143,19 @@ def test_empty_version_table_reads_as_version_0(tmp_path) -> None: def test_migration_list_shape() -> None: - """Migration 1 is the v1 DDL; 2 and 3 are the callable steps. + """Migration 1 is the v1 DDL; 2 and 3 are callable steps. Nothing ever shipped, so migration 1 was rewritten pre-release (D-007) to create the quest_* tables directly; migration 2 exists - for dev databases stamped version 1 by the pre-rename runner, and + for dev databases stamped version 1 by the pre-rename runner, migration 3 brings pre-D-008 definitions to the multi-assignee - model. + model, and migration 4 adds the windows table. """ assert MIGRATIONS[0] == SCHEMA_V1_STATEMENTS assert callable(MIGRATIONS[1]) assert callable(MIGRATIONS[2]) - assert len(MIGRATIONS) == 3 + assert MIGRATIONS[3] == SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL + assert len(MIGRATIONS) == 4 # --------------------------------------------------------------------------- diff --git a/tests/test_schema.py b/tests/test_schema.py index 58e65de..bd18996 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -17,6 +17,7 @@ from custom_components.nestquest.schema import ( SCHEMA_V1_STATEMENTS, SCHEMA_V1_QUEST_DEFINITIONS_DDL, SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, + SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL, SCHEMA_V1_QUEST_INSTANCES_DDL, async_apply_ddl, ) @@ -167,13 +168,14 @@ def _insert_override(database, child_id, **overrides): # --------------------------------------------------------------------------- -def test_schema_v1_statements_compose_the_seven_tables() -> None: +def test_schema_v1_statements_compose_the_eight_tables() -> None: assert SCHEMA_V1_STATEMENTS == [ *SCHEMA_V1_CHILDREN_DDL, *SCHEMA_V1_ADMIN_USERS_DDL, *SCHEMA_V1_SCHEDULE_RULES_DDL, *SCHEMA_V1_QUEST_DEFINITIONS_DDL, *SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, + *SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL, *SCHEMA_V1_PRESENCE_SCHEDULES_DDL, *SCHEMA_V1_PRESENCE_OVERRIDES_DDL, *SCHEMA_V1_QUEST_INSTANCES_DDL, @@ -974,6 +976,166 @@ def test_quest_definition_assignees_duplicate_pair_fails(tmp_path) -> None: _run(database.close()) +def test_quest_definition_windows_table_exists_after_applying_ddl( + tmp_path, +) -> None: + database = _open_db(tmp_path / "windows.db") + try: + _apply(database) + row = _run( + database.fetch_one( + "SELECT name FROM sqlite_master WHERE type = 'table' " + "AND name = 'quest_definition_windows'" + ) + ) + assert row is not None and row[0] == "quest_definition_windows" + finally: + _run(database.close()) + + +def test_quest_definition_windows_columns_types_and_constraints( + tmp_path, +) -> None: + database = _open_db(tmp_path / "windows-columns.db") + try: + _apply(database) + columns = _run( + database.fetch_all("PRAGMA table_info(quest_definition_windows)") + ) + # cid, name, type, notnull, dflt_value, pk — the composite + # primary key spans definition_id and window. + assert [(row[1], row[2], row[3], row[4], row[5]) for row in columns] == [ + ("definition_id", "INTEGER", 1, None, 1), + ("window", "TEXT", 1, None, 2), + ("due_time", "TEXT", 0, None, 0), + ] + finally: + _run(database.close()) + + +def test_quest_definition_windows_foreign_keys_declared(tmp_path) -> None: + database = _open_db(tmp_path / "windows-fks.db") + try: + _apply(database) + fks = _run( + database.fetch_all( + "PRAGMA foreign_key_list(quest_definition_windows)" + ) + ) + declared = {(row[2], row[3], row[4]) for row in fks} + assert declared == { + ("quest_definitions", "definition_id", "id"), + } + finally: + _run(database.close()) + + +def test_quest_definition_windows_valid_rows_round_trip(tmp_path) -> None: + database = _open_db(tmp_path / "windows-roundtrip.db") + try: + _apply(database) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) VALUES (?, ?)", + ("Ada", "2026-09-13T00:00:00+00:00"), + ) + ) + _insert_rule(database) + definition_id = _insert_definition(database, assignees=None) + for window, due in ( + ("morning", "09:00"), + ("afternoon", None), + ("evening", "19:30"), + ): + _run( + database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) VALUES (?, ?, ?)", + (definition_id, window, due), + ) + ) + rows = _run( + database.fetch_all( + "SELECT window, due_time FROM quest_definition_windows " + "WHERE definition_id = ? ORDER BY window", + (definition_id,), + ) + ) + assert rows == [ + ("afternoon", None), + ("evening", "19:30"), + ("morning", "09:00"), + ] + finally: + _run(database.close()) + + +@pytest.mark.parametrize( + "window", ["Midnight", "MORNING", "noon", "", "evenings"] +) +def test_quest_definition_windows_window_check_enforced( + tmp_path, window +) -> None: + database = _open_db(tmp_path / "windows-check.db") + try: + _apply(database) + _insert_rule(database) + definition_id = _insert_definition(database, assignees=None) + with pytest.raises(sqlite3.IntegrityError, match="CHECK"): + _run( + database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) VALUES (?, ?, NULL)", + (definition_id, window), + ) + ) + finally: + _run(database.close()) + + +def test_quest_definition_windows_duplicate_window_fails(tmp_path) -> None: + database = _open_db(tmp_path / "windows-duplicate.db") + try: + _apply(database) + _insert_rule(database) + definition_id = _insert_definition(database, assignees=None) + _run( + database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) " + "VALUES (?, 'morning', NULL)", + (definition_id,), + ) + ) + with pytest.raises(sqlite3.IntegrityError): + _run( + database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) " + "VALUES (?, 'morning', '09:00')", + (definition_id,), + ) + ) + finally: + _run(database.close()) + + +def test_quest_definition_windows_unknown_definition_fails(tmp_path) -> None: + database = _open_db(tmp_path / "windows-bad-definition.db") + try: + _apply(database) + with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"): + _run( + database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) " + "VALUES (999, 'morning', NULL)" + ) + ) + finally: + _run(database.close()) + + def test_quest_definition_assignees_multiple_children_allowed( tmp_path, ) -> None: From 25f1479444a5ce7edbced43ace93df5beb1de49f Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 10:17:31 -0400 Subject: [PATCH 4/5] Widen instance key to (definition_id, child_id, due_date, window) (D-008) Per IMPLEMENTATION-ORDER.md 1.2: quest_instances gains a NOT NULL window column (CHECK-constrained to the const window spellings) and the table-level UNIQUE constraint widens to the full fan-out tuple. - schema.py: instance table body extracted to QUEST_INSTANCES_TABLE_SQL shared with the rebuild migration; uniqueness docstring rewritten for the widened key. - migrations.py: migration 5 rebuilds a pre-window quest_instances via the canonical procedure; migrated rows are pinned to the documented default 'morning' window (pre-window rows were whole-day obligations; future dates are regenerated with real windows by materialization), preserving ids so completion_events keep pointing at the same instances; PRAGMA foreign_key_check verified inside the transaction. - dao_instances.py: upsert takes required keyword window and is idempotent on the widened tuple; the immutable-once-completed check and the read-back get() are keyed on the full tuple; window spelling validated via dao_rules._validate_window. - tests: schema shape (window column, widened autoindex columns), twice-daily and shared-assignee fan-out cases at both the DDL and DAO layers (incl. idempotent re-runs), cross-child same-key allowance, window CHECK/NOT NULL constraints, v2 seed frozen to the true version-2 shapes and its migration assertions widened. --- custom_components/nestquest/dao_instances.py | 86 ++++--- custom_components/nestquest/migrations.py | 85 ++++++- custom_components/nestquest/schema.py | 70 +++--- tests/test_dao_instances.py | 224 ++++++++++++------- tests/test_dao_matrix.py | 23 +- tests/test_dao_rules.py | 2 +- tests/test_migrations.py | 61 ++++- tests/test_schema.py | 101 ++++++++- 8 files changed, 479 insertions(+), 173 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 43c8997..2e9aaa3 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -15,13 +15,13 @@ event. A package-wide guard test (bottom of the test module) keeps mutation SQL for this table out of every other module. Upsert semantics: ``QuestInstancesDao.upsert`` is idempotent on -(definition_id, due_date) — re-running materialization for a date -whose instance already exists updates nothing that matters -(child_id/due_time are refreshed only in the degenerate re-generation -case where they changed) and never duplicates a row. The schema's -UNIQUE(definition_id, due_date) is the last line of defense. The -upsert refuses a child that is not an assignee of the definition -(D-008 multi-assignee model). +(definition_id, child_id, due_date, window) — re-running +materialization for a tuple whose instance already exists refreshes +only the generation-time snapshot columns and never duplicates a row. +The schema's UNIQUE(definition_id, child_id, due_date, window) is the +last line of defense. The upsert refuses a child that is not an +assignee of the definition and a window outside ``const.QUEST_WINDOWS`` +(D-008 multi-assignee, multi-window model). ``delete_future_uncompleted`` implements the Feature 06/07 rule that reassignment and schedule edits regenerate future instances: it @@ -37,7 +37,7 @@ import datetime from dataclasses import dataclass from .dao_children import _connection_lock -from .dao_rules import _validate_date +from .dao_rules import _validate_date, _validate_window from .db import NestQuestDatabase #: Timestamp policy for completion_events.occurred_at: strict UTC @@ -90,6 +90,7 @@ class QuestInstanceRecord: id: int definition_id: int child_id: int + window: str due_date: str due_time: str | None generated_at: str @@ -110,7 +111,7 @@ class CompletionEventRecord: _INSTANCE_COLUMNS = ( - "id, definition_id, child_id, due_date, due_time, generated_at" + "id, definition_id, child_id, window, due_date, due_time, generated_at" ) _EVENT_COLUMNS = ( "id, instance_id, child_id, event_type, actor_source, actor_user_id, " @@ -128,9 +129,10 @@ def _instance_from_row(row: tuple) -> QuestInstanceRecord: id=row[0], definition_id=row[1], child_id=row[2], - due_date=row[3], - due_time=row[4], - generated_at=row[5], + window=row[3], + due_date=row[4], + due_time=row[5], + generated_at=row[6], ) @@ -160,31 +162,40 @@ class QuestInstancesDao: due_date: str, generated_at: str, *, + window: str, due_time: str | None = None, ) -> QuestInstanceRecord: - """Create the dated instance for (definition_id, due_date) idempotently. + """Create the dated instance for the widened key idempotently. - Running materialization twice for the same definition/date - yields ONE row: the ON CONFLICT path refreshes the - generation-time snapshot columns (child_id, due_time) and - regenerates the stamp, but can never create a duplicate — the - schema UNIQUE(definition_id, due_date) backs this up. + The instance key is (definition_id, child_id, due_date, window) + (D-008): running materialization twice for the same tuple + yields ONE row — the ON CONFLICT path refreshes the + generation-time snapshot columns (due_time) and regenerates the + stamp, but can never create a duplicate. The schema's + UNIQUE(definition_id, child_id, due_date, window) backs this + up, so the twice-daily case (two windows, one child, one date) + yields two rows and the shared case (three assignees, one + window) yields three. Guardrails enforced here: - ``due_date`` must be TODAY or later: instances are never generated in the past (Feature 07). The caller's materialization horizon is future-dated by definition. + - ``window`` must be one of ``const.QUEST_WINDOWS``. - An existing instance WITH a completion event is IMMUTABLE: the conflict path must not rewrite its snapshot columns (Feature 07/08). The upsert refuses with ValueError in that case — silently no-oping would mask a materialization bug. + - The child must be an assignee of the definition (D-008 + multi-assignee model). The definition must exist (validated atomically under the connection lock so a concurrent definition deactivation or child deletion cannot slip between check and insert). """ _validate_date(due_date, "due_date") + _validate_window(window) # Fail fast on obviously-past dates BEFORE queuing on the # connection lock (a nice early error); the AUTHORITATIVE # check runs after the lock is acquired, because a caller can @@ -227,51 +238,56 @@ class QuestInstancesDao: f"assigned to child {child_id}; instance must " "carry an assigned child" ) - child = await self._database.fetch_one( - "SELECT 1 FROM children WHERE id = ?", (child_id,) - ) - if child is None: - raise ValueError(f"child {child_id} does not exist") existing = await self._database.fetch_one( "SELECT 1 FROM completion_events " "WHERE instance_id = (SELECT id FROM quest_instances " - "WHERE definition_id = ? AND due_date = ?)", - (definition_id, due_date), + "WHERE definition_id = ? AND child_id = ? " + "AND due_date = ? AND window = ?)", + (definition_id, child_id, due_date, window), ) if existing is not None: raise ValueError( f"instance for (definition {definition_id}, " - f"due_date {due_date!r}) already has a completion " + f"child {child_id}, due_date {due_date!r}, " + f"window {window!r}) already has a completion " "event and is immutable; it cannot be regenerated" ) await self._database.execute( "INSERT INTO quest_instances (definition_id, child_id, " - "due_date, due_time, generated_at) " - "VALUES (?, ?, ?, ?, ?) " - "ON CONFLICT (definition_id, due_date) DO UPDATE SET " - "child_id = excluded.child_id, " + "window, due_date, due_time, generated_at) " + "VALUES (?, ?, ?, ?, ?, ?) " + "ON CONFLICT (definition_id, child_id, due_date, " + "window) DO UPDATE SET " "due_time = excluded.due_time, " "generated_at = excluded.generated_at", ( definition_id, child_id, + window, due_date, due_time, generated_at, ), ) - instance = await self.get(definition_id, due_date) + instance = await self.get( + definition_id, child_id, due_date, window + ) assert instance is not None return instance async def get( - self, definition_id: int, due_date: str + self, + definition_id: int, + child_id: int, + due_date: str, + window: str, ) -> QuestInstanceRecord | None: - """Return the instance for (definition_id, due_date), or None.""" + """Return the instance for the widened key tuple, or None.""" row = await self._database.fetch_one( f"SELECT {_INSTANCE_COLUMNS} FROM quest_instances " - "WHERE definition_id = ? AND due_date = ?", - (definition_id, due_date), + "WHERE definition_id = ? AND child_id = ? AND due_date = ? " + "AND window = ?", + (definition_id, child_id, due_date, window), ) return _instance_from_row(row) if row is not None else None diff --git a/custom_components/nestquest/migrations.py b/custom_components/nestquest/migrations.py index 29d5d20..9901505 100644 --- a/custom_components/nestquest/migrations.py +++ b/custom_components/nestquest/migrations.py @@ -24,9 +24,12 @@ names before anything else assumes the new model. Migration 3 brings pre-D-008 databases to the multi-assignee model: it creates ``quest_definition_assignees`` and, when ``quest_definitions`` still carries a single ``child_id`` column, rebuilds the table without it, -copying each definition's assignee across. Each callable migration -runs inside its own transaction together with its version stamp, so a -crash mid-step rolls the statements and the stamp back together. +copying each definition's assignee across. Migration 4 adds the +windows table and migration 5 rebuilds ``quest_instances`` onto the +widened (definition_id, child_id, due_date, window) key. Each callable +migration runs inside its own transaction together with its version +stamp, so a crash mid-step rolls the statements and the stamp back +together. Migration entries are either an ordered sequence of SQL statements or an async callable taking ``(database, target_version)``. Callables @@ -71,6 +74,7 @@ from typing import Union from .db import NestQuestDatabase from .schema import ( QUEST_DEFINITIONS_TABLE_SQL, + QUEST_INSTANCES_TABLE_SQL, SCHEMA_V1_STATEMENTS, SCHEMA_V1_QUEST_DEFINITION_ASSIGNEES_DDL, SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL, @@ -147,6 +151,14 @@ async def _table_has_column( return any(row[1] == column for row in rows) +#: Window assigned to instances migrated from the pre-window instance +#: shape (D-008). Pre-window rows were generated for whole-day +#: obligations; the morning window is the documented default landing +#: spot, and future dates are regenerated with real window declarations +#: by materialization (delete_future_uncompleted + re-run). +LEGACY_MIGRATED_WINDOW = "morning" + + async def _rename_legacy_task_tables( database: NestQuestDatabase, target_version: int ) -> None: @@ -250,6 +262,70 @@ async def _add_definition_assignees( await database.execute("PRAGMA foreign_keys = ON") +async def _widen_instance_key( + database: NestQuestDatabase, target_version: int +) -> None: + """Bring ``quest_instances`` to the D-008 instance key. + + Databases whose instance table still carries the pre-window shape + (no ``window`` column, UNIQUE on (definition_id, due_date)) are + rebuilt via SQLite's canonical procedure into the widened shape — + UNIQUE (definition_id, child_id, due_date, window) — with every + migrated row landing in :data:`LEGACY_MIGRATED_WINDOW` so existing + ids (and the completion events referencing them) survive intact. + The old rows were unique per (definition, due_date), so pinning one + window cannot collide with the widened key. Fresh databases whose + table already matches the canonical shape only stamp. + + The rebuild runs with ``foreign_keys = OFF`` (dropping the old + table under enforcement would fire the implicit DELETE and violate + the ``completion_events`` foreign key) and verifies + ``PRAGMA foreign_key_check`` inside the transaction before the + stamp, so a corrupt copy aborts the whole migration. + """ + legacy_shape = not await _table_has_column( + database, "quest_instances", "window" + ) + await database.execute("PRAGMA foreign_keys = OFF") + try: + async with database.transaction(): + if legacy_shape: + await database.execute( + "CREATE TABLE quest_instances_rebuilt " + f"{QUEST_INSTANCES_TABLE_SQL}" + ) + await database.execute( + "INSERT INTO quest_instances_rebuilt " + "(id, definition_id, child_id, window, due_date, " + "due_time, generated_at) " + "SELECT id, definition_id, child_id, ?, due_date, " + "due_time, generated_at " + "FROM quest_instances ORDER BY id", + (LEGACY_MIGRATED_WINDOW,), + ) + await database.execute("DROP TABLE quest_instances") + await database.execute( + "ALTER TABLE quest_instances_rebuilt " + "RENAME TO quest_instances" + ) + violations = await database.fetch_all( + "PRAGMA foreign_key_check" + ) + if violations: + raise RuntimeError( + "instance rebuild produced foreign-key " + f"violations: {violations!r}" + ) + LOGGER.info( + "Widened quest_instances key; migrated rows pinned " + "to the %r window", + LEGACY_MIGRATED_WINDOW, + ) + await stamp_schema_version(database, target_version) + finally: + await database.execute("PRAGMA foreign_keys = ON") + + #: The ordered migration list. Append-only: never edit an applied #: entry, add the next one instead. (Migration 1's content was #: rewritten pre-release per D-007 — no version 1 database shipped.) @@ -261,6 +337,9 @@ MIGRATIONS: Sequence[MigrationStep] = [ # CREATE IF NOT EXISTS suffices for both fresh databases (which # already made it in migration 1) and pre-window databases. list(SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL), + # Migration 5 (D-008): rebuild quest_instances onto the widened + # (definition_id, child_id, due_date, window) key. + _widen_instance_key, ] diff --git a/custom_components/nestquest/schema.py b/custom_components/nestquest/schema.py index 7a73340..46447e6 100644 --- a/custom_components/nestquest/schema.py +++ b/custom_components/nestquest/schema.py @@ -109,25 +109,31 @@ status column. An instance's current state derives from the latest row in ``completion_events`` (Feature 08), so materialization (Feature 07) can insert instances without knowing anything about completion and the append-only log stays the single source of truth. ``definition_id``, -``due_date`` and ``child_id`` snapshot the materialization-time facts -("why this instance exists"): reassignment and rule edits change future -instances only and never rewrite these rows. ``due_time`` snapshots -the definition's optional due time at generation for the same reason — -the instance keeps its generation-time value even if the definition is -edited later. ``generated_at`` is a caller-set UTC ISO-8601 timestamp -per the module timestamp policy (no DB default, so a generation batch -stamps one coherent time). +``child_id``, ``due_date`` and ``window`` snapshot the +materialization-time facts ("why this instance exists"): assignment +edits and rule edits change future instances only and never rewrite +these rows. ``window`` records which declared day window produced the +instance (D-008), CHECK-constrained to the same three spellings +``const`` owns. ``due_time`` snapshots the window's optional due time +at generation for the same reason — the instance keeps its +generation-time value even if the definition is edited later. +``generated_at`` is a caller-set UTC ISO-8601 timestamp per the module +timestamp policy (no DB default, so a generation batch stamps one +coherent time). -Uniqueness: the (definition_id, due_date) pair is declared as a -table-level ``UNIQUE`` constraint rather than a separate ``CREATE -UNIQUE INDEX`` statement. SQLite materializes it as an implicit unique -index (``sqlite_autoindex_quest_instances_1``) — the required "unique -index prevents duplicate instances" — and the constraint form cannot be -silently dropped the way a standalone index can, so the duplicate door -stays shut. Idempotent materialization keys on exactly this pair. No -secondary indexes beyond it are declared yet: history and board queries -(Features 08/10/13) should justify their indexes as a later migration -once their query shapes exist. +Uniqueness: the (definition_id, child_id, due_date, window) tuple is +declared as a table-level ``UNIQUE`` constraint rather than a separate +``CREATE UNIQUE INDEX`` statement. SQLite materializes it as an +implicit unique index (``sqlite_autoindex_quest_instances_1``) — the +required "unique index prevents duplicate instances" — and the +constraint form cannot be silently dropped the way a standalone index +can, so the duplicate door stays shut. Idempotent materialization keys +on exactly this tuple: the twice-daily case (one definition, two +windows, one child, one date) yields two rows and the shared case (one +definition, three assignees, one window) yields three. No secondary +indexes beyond it are declared yet: history and board queries (Features +08/10/13) should justify their indexes as a later migration once their +query shapes exist. No ``ON DELETE`` clauses are declared on any foreign key in this module: children and definitions are deactivated, never deleted, so a @@ -316,17 +322,25 @@ SCHEMA_V1_PRESENCE_OVERRIDES_DDL: list[str] = [ """, ] +#: Column body of ``quest_instances``, shared between the v1 DDL +#: below and the instance-key rebuild migration, which must recreate +#: the table in exactly this shape (D-008: the instance key is +#: (definition_id, child_id, due_date, window) — one instance per +#: assigned child per declared window per firing date). +QUEST_INSTANCES_TABLE_SQL = """( + id INTEGER PRIMARY KEY AUTOINCREMENT, + definition_id INTEGER NOT NULL REFERENCES quest_definitions(id), + child_id INTEGER NOT NULL REFERENCES children(id), + window TEXT NOT NULL CHECK (window IN ('morning', 'afternoon', 'evening')), + due_date TEXT NOT NULL, + due_time TEXT, + generated_at TEXT NOT NULL, + UNIQUE (definition_id, child_id, due_date, window) +)""" + SCHEMA_V1_QUEST_INSTANCES_DDL: list[str] = [ - """ - CREATE TABLE IF NOT EXISTS quest_instances ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - definition_id INTEGER NOT NULL REFERENCES quest_definitions(id), - child_id INTEGER NOT NULL REFERENCES children(id), - due_date TEXT NOT NULL, - due_time TEXT, - generated_at TEXT NOT NULL, - UNIQUE (definition_id, due_date) - ) + f""" + CREATE TABLE IF NOT EXISTS quest_instances {QUEST_INSTANCES_TABLE_SQL} """, ] diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index 628752e..fe79027 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -119,12 +119,10 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): first = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") assert isinstance(first, QuestInstanceRecord) second = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") assert second.id == first.id rows = await database.fetch_one( "SELECT COUNT(*) FROM quest_instances" @@ -138,27 +136,116 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: def test_upsert_different_dates_create_distinct_rows(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - a = await instances.upsert(definition.id, child.id, D1, _now_stamp()) - b = await instances.upsert(definition.id, child.id, D2, _now_stamp()) + a = await instances.upsert(definition.id, child.id, D1, _now_stamp(), window="morning") + b = await instances.upsert(definition.id, child.id, D2, _now_stamp(), window="morning") assert a.id != b.id return (a, b) _with_db(tmp_path, "upsert-multi-date.db")(_body) +def test_upsert_twice_daily_fanout_two_windows_two_rows(tmp_path) -> None: + """D-008 twice-daily case: one definition, one child, one date, + two windows — two rows, each idempotent on its own tuple.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + morning = await instances.upsert( + definition.id, child.id, D1, _now_stamp(), window="morning" + ) + evening = await instances.upsert( + definition.id, child.id, D1, _now_stamp(), window="evening" + ) + assert morning.id != evening.id + rows = await database.fetch_all( + "SELECT window, child_id, due_date FROM quest_instances " + "WHERE definition_id = ? AND due_date = ?", + (definition.id, D1), + ) + assert sorted(rows) == [ + ("evening", child.id, D1), + ("morning", child.id, D1), + ] + # Re-running both is still idempotent. + await instances.upsert( + definition.id, child.id, D1, _now_stamp(), window="morning" + ) + await instances.upsert( + definition.id, child.id, D1, _now_stamp(), window="evening" + ) + count = await database.fetch_one( + "SELECT COUNT(*) FROM quest_instances" + ) + assert count == (2,) + return (morning, evening) + + _with_db(tmp_path, "upsert-twice-daily.db")(_body) + + +def test_upsert_shared_definition_three_assignees_three_rows( + tmp_path, +) -> None: + """D-008 shared case: one definition, three assignees, one window + — three rows, one per child.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + second = await children.create("Bo", _now_stamp()) + third = await children.create("Cleo", _now_stamp()) + await definitions.add_assignee(definition.id, second.id) + await definitions.add_assignee(definition.id, third.id) + created = [ + await instances.upsert( + definition.id, assignee.id, D1, _now_stamp(), + window="morning", + ) + for assignee in (child, second, third) + ] + assert len({row.id for row in created}) == 3 + count = await database.fetch_one( + "SELECT COUNT(*) FROM quest_instances " + "WHERE definition_id = ? AND due_date = ? AND window = ?", + (definition.id, D1, "morning"), + ) + assert count == (3,) + # Re-running the fan-out changes nothing. + for assignee in (child, second, third): + await instances.upsert( + definition.id, assignee.id, D1, _now_stamp(), + window="morning", + ) + count = await database.fetch_one( + "SELECT COUNT(*) FROM quest_instances" + ) + assert count == (3,) + return created + + _with_db(tmp_path, "upsert-shared-fanout.db")(_body) + + +def test_upsert_window_outside_const_list_raises(tmp_path) -> None: + """The DAO rejects a window spelling the schema CHECK would reject + anyway, with the typed ValueError instead of raw IntegrityError.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + with pytest.raises(ValueError, match="window must be one of"): + await instances.upsert( + definition.id, child.id, D1, _now_stamp(), window="noon" + ) + return None + + _with_db(tmp_path, "upsert-bad-window.db")(_body) + + def test_upsert_refreshes_snapshot_columns_on_conflict(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): first = await instances.upsert( - definition.id, child.id, D1, _now_stamp(), due_time="08:00" - ) + definition.id, child.id, D1, _now_stamp(), due_time="08:00", window="morning") second = await instances.upsert( definition.id, child.id, D1, f"{D1}T13:00:00+00:00", - due_time="08:30", - ) + due_time="08:30", window="morning") assert second.id == first.id assert second.due_time == "08:30" assert second.generated_at == f"{D1}T13:00:00+00:00" @@ -172,7 +259,7 @@ def test_upsert_rejects_wrong_child_for_definition(tmp_path) -> None: events, child, definition): other = await children.create("Bo", _now_stamp()) with pytest.raises(ValueError, match="assigned to"): - await instances.upsert(definition.id, other.id, D1, _now_stamp()) + await instances.upsert(definition.id, other.id, D1, _now_stamp(), window="morning") return None _with_db(tmp_path, "upsert-wrong-child.db")(_body) @@ -182,7 +269,7 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): with pytest.raises(ValueError, match="definition 999"): - await instances.upsert(999, child.id, D1, _now_stamp()) + await instances.upsert(999, child.id, D1, _now_stamp(), window="morning") # Unknown child: the definition-assignee check fires first and # is the one that matters (an instance must carry an assigned # child). Under D-008 the assignee check is a membership test @@ -193,7 +280,7 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: "X", rule.id, _now_stamp(), assignee_child_ids=[child.id] ) with pytest.raises(ValueError, match="not assigned to child 999"): - await instances.upsert(other_definition.id, 999, D1, _now_stamp()) + await instances.upsert(other_definition.id, 999, D1, _now_stamp(), window="morning") return None _with_db(tmp_path, "upsert-unknown.db")(_body) @@ -204,7 +291,7 @@ def test_upsert_malformed_due_date_raises(tmp_path) -> None: events, child, definition): for bad in ("not-a-date", "2026-9-15", ""): with pytest.raises(ValueError, match="YYYY-MM-DD"): - await instances.upsert(definition.id, child.id, bad, _now_stamp()) + await instances.upsert(definition.id, child.id, bad, _now_stamp(), window="morning") return None _with_db(tmp_path, "upsert-bad-date.db")(_body) @@ -216,11 +303,11 @@ def test_upsert_rejects_past_due_dates(tmp_path) -> None: events, child, definition): yesterday = _future_day(-1) with pytest.raises(ValueError, match="never generated in the past"): - await instances.upsert(definition.id, child.id, yesterday, _now_stamp()) + await instances.upsert(definition.id, child.id, yesterday, _now_stamp(), window="morning") # Today itself is allowed (the rollover job generates today's # instances before the day is over). today = datetime.date.today().isoformat() - record = await instances.upsert(definition.id, child.id, today, _now_stamp()) + record = await instances.upsert(definition.id, child.id, today, _now_stamp(), window="morning") assert record.due_date == today return record @@ -296,7 +383,7 @@ def test_upsert_no_past_check_uses_execution_date_not_call_date( await asyncio.wait_for(holder_inside.wait(), timeout=5) task = asyncio.ensure_future( - instances.upsert(definition.id, child.id, today, _now_stamp()) + instances.upsert(definition.id, child.id, today, _now_stamp(), window="morning") ) # A is now queued on the connection lock (it cannot pass # its in-lock check while B holds the lock). Shift the @@ -349,17 +436,15 @@ def test_upsert_refuses_immutable_completed_instance(tmp_path) -> None: events, child, definition): generated_at = _now_stamp() instance = await instances.upsert( - definition.id, child.id, D1, generated_at, due_time="08:00" - ) + definition.id, child.id, D1, generated_at, due_time="08:00", window="morning") await events.append( instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) with pytest.raises(ValueError, match="immutable"): await instances.upsert( - definition.id, child.id, D1, _now_stamp(), due_time="09:00" - ) - fetched = await instances.get(definition.id, D1) + definition.id, child.id, D1, _now_stamp(), due_time="09:00", window="morning") + fetched = await instances.get(definition.id, child.id, D1, "morning") assert fetched.due_time == "08:00", ( "failed regeneration must not touch the completed instance" ) @@ -378,12 +463,11 @@ def test_get_by_definition_and_date_round_trip(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): created = await instances.upsert( - definition.id, child.id, D1, _now_stamp(), due_time="17:30" - ) - fetched = await instances.get(definition.id, D1) + definition.id, child.id, D1, _now_stamp(), due_time="17:30", window="morning") + fetched = await instances.get(definition.id, child.id, D1, "morning") assert fetched == created assert fetched.due_time == "17:30" - assert await instances.get(definition.id, D2) is None + assert await instances.get(definition.id, child.id, D2, "morning") is None return fetched _with_db(tmp_path, "get-round-trip.db")(_body) @@ -392,7 +476,7 @@ def test_get_by_definition_and_date_round_trip(tmp_path) -> None: def test_get_by_id_round_trip(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - created = await instances.upsert(definition.id, child.id, D1, _now_stamp()) + created = await instances.upsert(definition.id, child.id, D1, _now_stamp(), window="morning") fetched = await instances.get_by_id(created.id) assert fetched == created assert await instances.get_by_id(999) is None @@ -409,11 +493,10 @@ def test_list_by_child_and_date_filters(tmp_path) -> None: other_definition = await definitions.create( "Make bed", rule.id, _now_stamp(), assignee_child_ids=[other.id] ) - await instances.upsert(definition.id, child.id, D1, _now_stamp()) - await instances.upsert(definition.id, child.id, D2, _now_stamp()) + await instances.upsert(definition.id, child.id, D1, _now_stamp(), window="morning") + await instances.upsert(definition.id, child.id, D2, _now_stamp(), window="morning") await instances.upsert( - other_definition.id, other.id, D1, _now_stamp() - ) + other_definition.id, other.id, D1, _now_stamp(), window="morning") mine = await instances.list_by_child_and_date(child.id, D1) assert len(mine) == 1 assert mine[0].child_id == child.id @@ -427,7 +510,7 @@ def test_list_by_date_range_filters_and_orders(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): for day in (D1, D3, D5, D20): - await instances.upsert(definition.id, child.id, day, _now_stamp()) + await instances.upsert(definition.id, child.id, day, _now_stamp(), window="morning") found = await instances.list_by_date_range( child.id, D2, D6 ) @@ -461,18 +544,17 @@ def test_list_by_date_range_rejects_inverted_range(tmp_path) -> None: def test_delete_future_uncompleted_removes_only_eligible(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - await instances.upsert(definition.id, child.id, D1, _now_stamp()) + await instances.upsert(definition.id, child.id, D1, _now_stamp(), window="morning") future_open = await instances.upsert( - definition.id, child.id, D5, _now_stamp() - ) - await instances.upsert(definition.id, child.id, D6, _now_stamp()) + definition.id, child.id, D5, _now_stamp(), window="morning") + await instances.upsert(definition.id, child.id, D6, _now_stamp(), window="morning") deleted = await instances.delete_future_uncompleted( definition.id, D5 ) assert deleted == 2 - assert await instances.get(definition.id, D1) is not None - assert await instances.get(definition.id, D5) is None - assert await instances.get(definition.id, D6) is None + assert await instances.get(definition.id, child.id, D1, "morning") is not None + assert await instances.get(definition.id, child.id, D5, "morning") is None + assert await instances.get(definition.id, child.id, D6, "morning") is None return deleted _with_db(tmp_path, "delete-future.db")(_body) @@ -482,11 +564,9 @@ def test_delete_future_uncompleted_spares_completed(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): completed = await instances.upsert( - definition.id, child.id, D5, _now_stamp() - ) + definition.id, child.id, D5, _now_stamp(), window="morning") open_one = await instances.upsert( - definition.id, child.id, D6, _now_stamp() - ) + definition.id, child.id, D6, _now_stamp(), window="morning") await events.append( completed.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", @@ -521,12 +601,12 @@ def test_delete_future_uncompleted_cannot_backdate_cutoff( async def _body(database, children, rules, definitions, instances, events, child, definition): today = datetime.date.today().isoformat() - await instances.upsert(definition.id, child.id, today, _now_stamp()) + await instances.upsert(definition.id, child.id, today, _now_stamp(), window="morning") deleted = await instances.delete_future_uncompleted( definition.id, "2020-01-01" ) assert deleted == 1 - assert await instances.get(definition.id, today) is None + assert await instances.get(definition.id, child.id, today, "morning") is None return deleted _with_db(tmp_path, "delete-no-backdate.db")(_body) @@ -541,13 +621,13 @@ def test_delete_future_uncompleted_other_definitions_untouched( "Make bed", (await rules.create("daily", D1)).id, _now_stamp(), assignee_child_ids=[child.id], ) - await instances.upsert(definition.id, child.id, D20, _now_stamp()) - await instances.upsert(other.id, child.id, D20, _now_stamp()) + await instances.upsert(definition.id, child.id, D20, _now_stamp(), window="morning") + await instances.upsert(other.id, child.id, D20, _now_stamp(), window="morning") deleted = await instances.delete_future_uncompleted( definition.id, D20 ) assert deleted == 1 - assert await instances.get(other.id, D20) is not None + assert await instances.get(other.id, child.id, D20, "morning") is not None return deleted _with_db(tmp_path, "delete-scoped.db")(_body) @@ -562,8 +642,7 @@ def test_append_completion_round_trips(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") event = await events.append( instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", @@ -582,8 +661,7 @@ def test_append_panel_event_round_trips(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") event = await events.append( instance.id, child.id, "completed", "panel", _now_stamp(), False, actor_user_id=None, @@ -600,8 +678,7 @@ def test_append_uncompleted_with_optional_on_time(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") reversal = await events.append( instance.id, child.id, "uncompleted", "user", _now_stamp(), None, actor_user_id="user-1", @@ -622,8 +699,7 @@ def test_append_validation_rejects_bad_shapes(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") with pytest.raises(ValueError, match="event_type"): await events.append( instance.id, child.id, "missed", "user", _now_stamp(), True, @@ -670,8 +746,7 @@ def test_append_rejects_non_utc_timestamps(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") for bad in ( "2026-09-14 12:00:00+00:00", # space separator "2026-09-14T12:00:00", # no offset @@ -695,8 +770,7 @@ def test_append_requires_was_on_time_strict_bool(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") for bad in (0.5, 1.5, 2, "true", 1): with pytest.raises(ValueError, match="must be True, False"): await events.append( @@ -717,8 +791,7 @@ def test_append_rejects_unknown_instance_or_child(tmp_path) -> None: actor_user_id="user-1", ) instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") with pytest.raises(ValueError, match="belongs to child"): await events.append( instance.id, 999, "completed", "user", _now_stamp(), True, @@ -740,8 +813,7 @@ def test_completed_uncompleted_recompleted_three_ordered_events( async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") await events.append( instance.id, child.id, "completed", "panel", _now_stamp(), True ) @@ -771,8 +843,7 @@ def test_get_latest_for_instance_none_when_untouched(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") return await events.get_latest_for_instance(instance.id) assert _with_db(tmp_path, "latest-none.db")(_body) is None @@ -782,8 +853,7 @@ def test_get_latest_returns_uncompleted_when_reversed(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") await events.append( instance.id, child.id, "completed", "panel", _now_stamp(), True ) @@ -802,14 +872,11 @@ def test_list_by_child_and_date_range_scopes_by_due_date(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): early = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") inside = await instances.upsert( - definition.id, child.id, D2, _now_stamp() - ) + definition.id, child.id, D2, _now_stamp(), window="morning") late = await instances.upsert( - definition.id, child.id, D20, _now_stamp() - ) + definition.id, child.id, D20, _now_stamp(), window="morning") await events.append( early.id, child.id, "completed", "panel", _now_stamp(), True ) @@ -847,8 +914,7 @@ def test_list_events_occurred_at_scope_variant(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): late_due = await instances.upsert( - definition.id, child.id, D20, _now_stamp() - ) + definition.id, child.id, D20, _now_stamp(), window="morning") # The event's occurred_at is today at 08:00 UTC (_now_stamp); # the instance is due D20. The due-date scope queried on D20 # still finds the event (scoping follows the instance's due @@ -1028,8 +1094,7 @@ def test_completion_events_dao_instance_has_no_mutation_capability( database.execute = _recording instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") await events.append( instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", @@ -1059,8 +1124,7 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: prepared ) instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp() - ) + definition.id, child.id, D1, _now_stamp(), window="morning") tasks = [ asyncio.ensure_future( events.append( diff --git a/tests/test_dao_matrix.py b/tests/test_dao_matrix.py index 9a2b36a..2360ffe 100644 --- a/tests/test_dao_matrix.py +++ b/tests/test_dao_matrix.py @@ -112,7 +112,8 @@ class World: self.child.id, D1, D2, True, note="test" ) self.instance = await self.instances.upsert( - self.definition.id, self.child.id, D1, _stamp() + self.definition.id, self.child.id, D1, _stamp(), + window="morning", ) @@ -291,12 +292,15 @@ def test_matrix_presence_overrides_crud_and_constraints(tmp_path) -> None: def test_matrix_quest_instances_crud_and_constraints(tmp_path) -> None: async def _body(w: World): # insert (seeded) + read - fetched = await w.instances.get(w.definition.id, D1) + fetched = await w.instances.get( + w.definition.id, w.child.id, D1, "morning" + ) assert fetched == w.instance # update path: conflict refresh on the OPEN instance — same row # id, snapshot columns (due_time) actually rewritten. refreshed = await w.instances.upsert( - w.definition.id, w.child.id, D1, _stamp(), due_time="17:00" + w.definition.id, w.child.id, D1, _stamp(), + window="morning", due_time="17:00", ) assert refreshed.id == fetched.id, ( "the conflict path must refresh the existing row in place" @@ -316,15 +320,15 @@ def test_matrix_quest_instances_constraints(tmp_path) -> None: # proving the schema backs the DAO's idempotency. await w.database.execute( "INSERT INTO quest_instances (definition_id, child_id, " - "due_date, due_time, generated_at) " - "VALUES (?, ?, ?, NULL, ?)", + "window, due_date, due_time, generated_at) " + "VALUES (?, ?, 'morning', ?, NULL, ?)", (w.definition.id, w.child.id, D20, _stamp()), ) with pytest.raises(sqlite3.IntegrityError): await w.database.execute( "INSERT INTO quest_instances (definition_id, child_id, " - "due_date, due_time, generated_at) " - "VALUES (?, ?, ?, NULL, ?)", + "window, due_date, due_time, generated_at) " + "VALUES (?, ?, 'morning', ?, NULL, ?)", (w.definition.id, w.child.id, D20, _stamp()), ) # delete future uncompleted removes D20's open instance @@ -340,10 +344,11 @@ def test_matrix_quest_instances_constraints(tmp_path) -> None: def test_matrix_quest_instances_upsert_idempotency(tmp_path) -> None: async def _body(w: World): first = await w.instances.upsert( - w.definition.id, w.child.id, D2, _stamp() + w.definition.id, w.child.id, D2, _stamp(), window="morning" ) second = await w.instances.upsert( - w.definition.id, w.child.id, D2, _stamp(), due_time="17:00" + w.definition.id, w.child.id, D2, _stamp(), + window="morning", due_time="17:00", ) assert second.id == first.id count = await w.database.fetch_one( diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index 0ae00b6..8784e73 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -927,7 +927,7 @@ def test_definition_assignment_leaves_history_rows_alone( # this table, but its row shape proves assignment isolation). await database.execute( "INSERT INTO quest_instances (definition_id, child_id, " - "due_date, generated_at) VALUES (?, ?, ?, ?)", + "window, due_date, generated_at) VALUES (?, ?, 'morning', ?, ?)", (definition.id, child.id, "2026-09-15", NOW), ) await definitions.add_assignee(definition.id, other.id) diff --git a/tests/test_migrations.py b/tests/test_migrations.py index ce81da3..8eee3ee 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -149,13 +149,16 @@ def test_migration_list_shape() -> None: (D-007) to create the quest_* tables directly; migration 2 exists for dev databases stamped version 1 by the pre-rename runner, migration 3 brings pre-D-008 definitions to the multi-assignee - model, and migration 4 adds the windows table. + model, migration 4 adds the windows table, and migration 5 rebuilds + quest_instances onto the widened (definition, child, date, window) + key. """ assert MIGRATIONS[0] == SCHEMA_V1_STATEMENTS assert callable(MIGRATIONS[1]) assert callable(MIGRATIONS[2]) assert MIGRATIONS[3] == SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL - assert len(MIGRATIONS) == 4 + assert callable(MIGRATIONS[4]) + assert len(MIGRATIONS) == 5 # --------------------------------------------------------------------------- @@ -425,6 +428,20 @@ _V2_DEFINITIONS_DDL = [ """, ] +_V2_INSTANCES_DDL = [ + """ + CREATE TABLE IF NOT EXISTS quest_instances ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + definition_id INTEGER NOT NULL REFERENCES quest_definitions(id), + child_id INTEGER NOT NULL REFERENCES children(id), + due_date TEXT NOT NULL, + due_time TEXT, + generated_at TEXT NOT NULL, + UNIQUE (definition_id, due_date) + ) + """, +] + def _seed_v2_multi_assignee_upgrade(database) -> None: """Create a database shaped like a post-rename, pre-D-008 dev file. @@ -433,11 +450,20 @@ def _seed_v2_multi_assignee_upgrade(database) -> None: single child_id column and no assignees table exists — stamped at version 2, with definition and instance rows that must survive. """ + # Freeze the version-2 shape: definitions carry child_id, the + # instance table has no window column, and the assignees/windows + # tables do not exist yet — everything else matches the current DDL. + _v2_replaced = ( + "quest_definitions ", + "quest_definition_assignees ", + "quest_definition_windows ", + "quest_instances ", + ) for sql in SCHEMA_V1_STATEMENTS: - if "quest_definitions " in sql: - continue # replaced by the legacy-shape DDL below + if any(name in sql for name in _v2_replaced): + continue _run(database.execute(sql)) - for sql in _V2_DEFINITIONS_DDL: + for sql in _V2_DEFINITIONS_DDL + _V2_INSTANCES_DDL: _run(database.execute(sql)) _run(database.execute(VERSION_TABLE_DDL)) _run( @@ -506,14 +532,33 @@ def test_v2_definitions_rebuilt_to_multi_assignee(tmp_path) -> None: ) ) assert assignees == [(1, 2)] - # The instance survived, still referencing the definition. + # The instance survived the rebuild, pinned to the migrated + # window, still referencing the definition. instance = _run( database.fetch_one( - "SELECT definition_id, child_id, due_date " + "SELECT definition_id, child_id, window, due_date " "FROM quest_instances WHERE id = 1" ) ) - assert instance == (1, 2, "2026-09-15") + assert instance == (1, 2, "morning", "2026-09-15") + # The rebuilt instance table carries the widened unique key. + index_rows = _run( + database.fetch_all( + "PRAGMA index_list(quest_instances)" + ) + ) + unique_columns = [] + for index_row in index_rows: + if index_row[2]: # unique flag + info = _run( + database.fetch_all( + f"PRAGMA index_info('{index_row[1]}')" + ) + ) + unique_columns.append([col[2] for col in info]) + assert [ + "definition_id", "child_id", "due_date", "window" + ] in unique_columns # No dangling references anywhere. assert _run(database.fetch_all("PRAGMA foreign_key_check")) == [] # The rebuilt table is live for the new model: a definition diff --git a/tests/test_schema.py b/tests/test_schema.py index bd18996..e27c774 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -1722,8 +1722,8 @@ def test_applying_presence_ddl_twice_keeps_rows(tmp_path) -> None: # --------------------------------------------------------------------------- -def _insert_instance(database, definition_id=1, **overrides): - """Insert one valid task_instance, returning the instance id. +def _insert_instance(database, definition_id=1, window="morning", **overrides): + """Insert one valid quest_instance, returning the instance id. Pass ``_OMIT`` as a value to leave that column out of the INSERT so its DDL DEFAULT applies. @@ -1731,6 +1731,7 @@ def _insert_instance(database, definition_id=1, **overrides): values: dict[str, object] = { "definition_id": definition_id, "child_id": 1, + "window": window, "due_date": "2026-09-14", "generated_at": "2026-09-14T00:00:00+00:00", } @@ -1829,6 +1830,7 @@ def test_quest_instances_columns_types_and_constraints(tmp_path) -> None: ("id", "INTEGER", 0, None, 1), ("definition_id", "INTEGER", 1, None, 0), ("child_id", "INTEGER", 1, None, 0), + ("window", "TEXT", 1, None, 0), ("due_date", "TEXT", 1, None, 0), ("due_time", "TEXT", 0, None, 0), ("generated_at", "TEXT", 1, None, 0), @@ -1884,7 +1886,7 @@ def test_completion_events_foreign_keys_declared(tmp_path) -> None: _run(database.close()) -def test_quest_instances_unique_index_on_definition_and_date(tmp_path) -> None: +def test_quest_instances_unique_index_on_widened_key(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-unique.db") try: _apply(database) @@ -1897,15 +1899,20 @@ def test_quest_instances_unique_index_on_definition_and_date(tmp_path) -> None: ) implicit = [(name, sql) for name, sql in rows if sql is None] assert len(implicit) == 1, ( - "the (definition_id, due_date) UNIQUE constraint must be backed " - "by exactly one implicit unique index" + "the (definition_id, child_id, due_date, window) UNIQUE " + "constraint must be backed by exactly one implicit unique index" ) columns = _run( database.fetch_all( f"PRAGMA index_info('{implicit[0][0]}')" ) ) - assert [row[2] for row in columns] == ["definition_id", "due_date"] + assert [row[2] for row in columns] == [ + "definition_id", + "child_id", + "due_date", + "window", + ] # The implicit index is genuinely unique: UNIQUE(...) is the only # constraint SQLite backs with a sql-less autoindex on this table. row = _run( @@ -1932,12 +1939,16 @@ def test_quest_instances_valid_insert_round_trips(tmp_path) -> None: instance_id = _insert_instance(database, definition_id=1, child_id=1) row = _run( database.fetch_one( - "SELECT definition_id, child_id, due_date, due_time, generated_at " + "SELECT definition_id, child_id, window, due_date, " + "due_time, generated_at " "FROM quest_instances WHERE id = ?", (instance_id,), ) ) - assert row == (1, 1, "2026-09-14", None, "2026-09-14T00:00:00+00:00") + assert row == ( + 1, 1, "morning", "2026-09-14", None, + "2026-09-14T00:00:00+00:00", + ) finally: _run(database.close()) @@ -1961,7 +1972,7 @@ def test_quest_instances_due_time_round_trips(tmp_path) -> None: _run(database.close()) -def test_quest_instances_duplicate_definition_date_fails(tmp_path) -> None: +def test_quest_instances_duplicate_tuple_fails(tmp_path) -> None: database = _open_db(tmp_path / "task-instances-duplicate.db") try: _apply(database) @@ -1973,6 +1984,78 @@ def test_quest_instances_duplicate_definition_date_fails(tmp_path) -> None: _run(database.close()) +def test_quest_instances_duplicate_tuple_different_child_allowed( + tmp_path, +) -> None: + """D-008: same definition, date and window for two different + assignees of the same definition are two distinct instances.""" + database = _open_db(tmp_path / "task-instances-shared.db") + try: + _apply(database) + _setup_definition(database) + _run( + database.execute( + "INSERT INTO children (display_name, created_at) VALUES (?, ?)", + ("Bo", "2026-09-13T00:00:00+00:00"), + ) + ) + _run( + database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (1, 2)" + ) + ) + _insert_instance(database, definition_id=1, child_id=1) + _insert_instance(database, definition_id=1, child_id=2) + row = _run(database.fetch_one("SELECT COUNT(*) FROM quest_instances")) + assert row == (2,) + finally: + _run(database.close()) + + +def test_quest_instances_duplicate_tuple_different_window_allowed( + tmp_path, +) -> None: + """D-008 twice-daily case: one definition, one child, one date, + two windows — two rows.""" + database = _open_db(tmp_path / "task-instances-twice-daily.db") + try: + _apply(database) + _setup_definition(database) + _insert_instance(database, definition_id=1, window="morning") + _insert_instance(database, definition_id=1, window="evening") + rows = _run( + database.fetch_all( + "SELECT window FROM quest_instances ORDER BY window" + ) + ) + assert [row[0] for row in rows] == ["evening", "morning"] + finally: + _run(database.close()) + + +def test_quest_instances_unknown_window_fails(tmp_path) -> None: + database = _open_db(tmp_path / "task-instances-bad-window.db") + try: + _apply(database) + _setup_definition(database) + with pytest.raises(sqlite3.IntegrityError, match="CHECK"): + _insert_instance(database, definition_id=1, window="noon") + finally: + _run(database.close()) + + +def test_quest_instances_window_not_null_enforced(tmp_path) -> None: + database = _open_db(tmp_path / "task-instances-window-null.db") + try: + _apply(database) + _setup_definition(database) + with pytest.raises(sqlite3.IntegrityError, match="NOT NULL"): + _insert_instance(database, definition_id=1, window=_OMIT) + finally: + _run(database.close()) + + def test_quest_instances_same_date_different_definitions_allowed( tmp_path, ) -> None: From fe90064b65a20af1a288fda23b2ea31ecaec44b6 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 10:25:50 -0400 Subject: [PATCH 5/5] Add actor_child_id to completion_events (D-008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per IMPLEMENTATION-ORDER.md 1.5: a nullable actor_child_id column records which panel profile was tapped; admin events stay NULL. - schema.py: completion_events table body extracted to COMPLETION_EVENTS_TABLE_SQL; the new column carries a FK to children(id) and the actor-pair coherence is extended to a second CHECK: 'user' events require actor_user_id and forbid actor_child_id, 'panel' events require actor_child_id and forbid actor_user_id. - migrations.py: migration 6 rebuilds legacy-shaped completion_events via the canonical procedure (CHECKs cannot be added to an existing table), backfilling each legacy panel row to its own child_id (pre-D-008 panel completions were always the tapped child) via INSERT..SELECT — no UPDATE or DELETE path exists, so the append-only guarantee is untouched; ids preserved; PRAGMA foreign_key_check verified in-transaction. - dao_instances.py: append() takes actor_child_id, validates both actor shapes (panel requires it, user forbids it) and the tapped child's existence under the connection lock; the record and column list grow accordingly. - tests: coherence CHECK matrix (5 bad shapes), unknown actor_child_id FK, panel/user round-trips at DDL and DAO layers, migration-6 backfill test on a frozen v2-shape seed; the completion_events leak guard gains justified INSERT exemptions for the migration layer (UPDATE/DELETE stay forbidden everywhere). --- custom_components/nestquest/dao_instances.py | 55 +++++++++---- custom_components/nestquest/migrations.py | 84 ++++++++++++++++++-- custom_components/nestquest/schema.py | 68 +++++++++------- tests/test_dao_instances.py | 46 ++++++++--- tests/test_dao_matrix.py | 6 +- tests/test_migrations.py | 62 ++++++++++++++- tests/test_schema.py | 63 ++++++++++++++- 7 files changed, 318 insertions(+), 66 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 2e9aaa3..93437f1 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -106,6 +106,7 @@ class CompletionEventRecord: event_type: str actor_source: str actor_user_id: str | None + actor_child_id: int | None occurred_at: str was_on_time: bool | None @@ -115,7 +116,7 @@ _INSTANCE_COLUMNS = ( ) _EVENT_COLUMNS = ( "id, instance_id, child_id, event_type, actor_source, actor_user_id, " - "occurred_at, was_on_time" + "actor_child_id, occurred_at, was_on_time" ) #: The only event types the append path accepts; the schema CHECK is @@ -144,8 +145,9 @@ def _event_from_row(row: tuple) -> CompletionEventRecord: event_type=row[3], actor_source=row[4], actor_user_id=row[5], - occurred_at=row[6], - was_on_time=None if row[7] is None else bool(row[7]), + actor_child_id=row[6], + occurred_at=row[7], + was_on_time=None if row[8] is None else bool(row[8]), ) @@ -396,19 +398,21 @@ class CompletionEventsDao: was_on_time: bool | None, *, actor_user_id: str | None = None, + actor_child_id: int | None = None, ) -> CompletionEventRecord: """Append one completion or un-completion event. - ``actor_source`` is 'user' (with ``actor_user_id`` set) or - 'panel' (no user id — the tapped child is ``child_id``); the - actor-pair CHECK enforces this shape at the schema level. - ``occurred_at`` must be the module's one strict UTC shape - (YYYY-MM-DDTHH:MM:SS+00:00) so lexicographic range queries - stay exact. ``was_on_time`` is required (True/False) for - completions; for un-completions it may be None or the reversed - completion's flag (Feature 08 still deciding). The instance - and child must exist — validated atomically under the - connection lock. + ``actor_source`` is 'user' (an admin action: ``actor_user_id`` + set, ``actor_child_id`` NULL) or 'panel' (a panel tap: no user + id, and ``actor_child_id`` carrying the tapped child profile, + D-008); the actor-pair CHECKs enforce this shape at the schema + level. ``occurred_at`` must be the module's one strict UTC + shape (YYYY-MM-DDTHH:MM:SS+00:00) so lexicographic range + queries stay exact. ``was_on_time`` is required (True/False) + for completions; for un-completions it may be None or the + reversed completion's flag (Feature 08 still deciding). The + instance, child and — for panel events — the tapped child must + exist — validated atomically under the connection lock. """ if event_type not in (EVENT_COMPLETED, EVENT_UNCOMPLETED): raise ValueError( @@ -428,6 +432,15 @@ class CompletionEventsDao: raise ValueError( "actor_source 'panel' must not carry actor_user_id" ) + if actor_source == "panel" and actor_child_id is None: + raise ValueError( + "actor_source 'panel' requires actor_child_id " + "(the tapped child profile)" + ) + if actor_source == "user" and actor_child_id is not None: + raise ValueError( + "actor_source 'user' must not carry actor_child_id" + ) if was_on_time is not None and type(was_on_time) is not bool: raise ValueError( f"was_on_time must be True, False or None, got " @@ -458,16 +471,28 @@ class CompletionEventsDao: ) if child is None: raise ValueError(f"child {child_id} does not exist") + if actor_child_id is not None: + tapped = await self._database.fetch_one( + "SELECT 1 FROM children WHERE id = ?", + (actor_child_id,), + ) + if tapped is None: + raise ValueError( + f"actor_child_id {actor_child_id} does not " + "reference an existing child" + ) result = await self._database.execute( "INSERT INTO completion_events (instance_id, child_id, " - "event_type, actor_source, actor_user_id, occurred_at, " - "was_on_time) VALUES (?, ?, ?, ?, ?, ?, ?)", + "event_type, actor_source, actor_user_id, " + "actor_child_id, occurred_at, was_on_time) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)", ( instance_id, child_id, event_type, actor_source, actor_user_id, + actor_child_id, occurred_at, None if was_on_time is None else int(was_on_time), ), diff --git a/custom_components/nestquest/migrations.py b/custom_components/nestquest/migrations.py index 9901505..2040366 100644 --- a/custom_components/nestquest/migrations.py +++ b/custom_components/nestquest/migrations.py @@ -25,11 +25,12 @@ pre-D-008 databases to the multi-assignee model: it creates ``quest_definition_assignees`` and, when ``quest_definitions`` still carries a single ``child_id`` column, rebuilds the table without it, copying each definition's assignee across. Migration 4 adds the -windows table and migration 5 rebuilds ``quest_instances`` onto the -widened (definition_id, child_id, due_date, window) key. Each callable -migration runs inside its own transaction together with its version -stamp, so a crash mid-step rolls the statements and the stamp back -together. +windows table, migration 5 rebuilds ``quest_instances`` onto the +widened (definition_id, child_id, due_date, window) key, and migration +6 rebuilds ``completion_events`` with the D-008 ``actor_child_id`` +column. Each callable migration runs inside its own transaction +together with its version stamp, so a crash mid-step rolls the +statements and the stamp back together. Migration entries are either an ordered sequence of SQL statements or an async callable taking ``(database, target_version)``. Callables @@ -73,6 +74,7 @@ from typing import Union from .db import NestQuestDatabase from .schema import ( + COMPLETION_EVENTS_TABLE_SQL, QUEST_DEFINITIONS_TABLE_SQL, QUEST_INSTANCES_TABLE_SQL, SCHEMA_V1_STATEMENTS, @@ -326,6 +328,74 @@ async def _widen_instance_key( await database.execute("PRAGMA foreign_keys = ON") +async def _add_actor_child_id( + database: NestQuestDatabase, target_version: int +) -> None: + """Bring ``completion_events`` to the D-008 actor-child shape. + + The ``actor_child_id`` column records which panel profile was + tapped; it cannot be added with ``ALTER TABLE ADD COLUMN`` alone + because the canonical shape also carries an actor-pair coherence + CHECK (panel events require it, user events forbid it), and CHECK + constraints cannot be added to an existing table. Databases with + the legacy shape are therefore rebuilt via SQLite's canonical + procedure, backfilling each legacy panel row's ``actor_child_id`` + to ``child_id`` — pre-D-008 panel completions were always recorded + against the tapped child themselves — while admin rows stay NULL. + Row ids are preserved so ``quest_instances`` references and the + append-only history stay intact. Fresh databases already match + the canonical shape and only stamp. + + The rebuild runs with ``foreign_keys = OFF`` and verifies + ``PRAGMA foreign_key_check`` inside the transaction before the + stamp. The copy is an INSERT..SELECT, not an UPDATE: the + completion_events table keeps its append-only guarantee. + """ + legacy_shape = not await _table_has_column( + database, "completion_events", "actor_child_id" + ) + await database.execute("PRAGMA foreign_keys = OFF") + try: + async with database.transaction(): + if legacy_shape: + await database.execute( + "CREATE TABLE completion_events_rebuilt " + f"{COMPLETION_EVENTS_TABLE_SQL}" + ) + await database.execute( + "INSERT INTO completion_events_rebuilt " + "(id, instance_id, child_id, event_type, actor_source, " + "actor_user_id, actor_child_id, occurred_at, " + "was_on_time) " + "SELECT id, instance_id, child_id, event_type, " + "actor_source, actor_user_id, " + "CASE WHEN actor_source = 'panel' THEN child_id " + "ELSE NULL END, " + "occurred_at, was_on_time " + "FROM completion_events ORDER BY id" + ) + await database.execute("DROP TABLE completion_events") + await database.execute( + "ALTER TABLE completion_events_rebuilt " + "RENAME TO completion_events" + ) + violations = await database.fetch_all( + "PRAGMA foreign_key_check" + ) + if violations: + raise RuntimeError( + "completion_events rebuild produced foreign-key " + f"violations: {violations!r}" + ) + LOGGER.info( + "Added actor_child_id to completion_events; legacy " + "panel rows backfilled to their own child_id" + ) + await stamp_schema_version(database, target_version) + finally: + await database.execute("PRAGMA foreign_keys = ON") + + #: The ordered migration list. Append-only: never edit an applied #: entry, add the next one instead. (Migration 1's content was #: rewritten pre-release per D-007 — no version 1 database shipped.) @@ -340,6 +410,10 @@ MIGRATIONS: Sequence[MigrationStep] = [ # Migration 5 (D-008): rebuild quest_instances onto the widened # (definition_id, child_id, due_date, window) key. _widen_instance_key, + # Migration 6 (D-008): rebuild completion_events with the + # actor_child_id column and its actor-pair coherence CHECK, + # backfilling legacy panel rows to their own child_id. + _add_actor_child_id, ] diff --git a/custom_components/nestquest/schema.py b/custom_components/nestquest/schema.py index 46447e6..ab1fb35 100644 --- a/custom_components/nestquest/schema.py +++ b/custom_components/nestquest/schema.py @@ -159,11 +159,12 @@ self-describing without joins. Actor policy: ``actor_source`` distinguishes an authenticated HA user (``'user'``, with ``actor_user_id`` holding the HA user id) from a panel tap with no user context (``'panel'``, with ``actor_user_id`` -NULL — the tapped child profile is already ``child_id``). A coherence -CHECK makes the pair shape non-negotiable: ``'user'`` requires -``actor_user_id``, ``'panel'`` forbids it. Other actor sources are -rejected; if an unattended system actor is ever needed, that is a -schema migration, not a silent widening. +NULL — the tapped child profile is ``actor_child_id``, D-008). Two +coherence CHECKs make both shapes non-negotiable: ``'user'`` requires +``actor_user_id`` and forbids ``actor_child_id``; ``'panel'`` requires +``actor_child_id`` (the tapped profile) and forbids ``actor_user_id``. +Other actor sources are rejected; if an unattended system actor is +ever needed, that is a schema migration, not a silent widening. On-time policy: ``was_on_time`` is required (integer 0 or 1) for ``completed`` events — at completion time the due date is known and the @@ -344,29 +345,42 @@ SCHEMA_V1_QUEST_INSTANCES_DDL: list[str] = [ """, ] -SCHEMA_V1_COMPLETION_EVENTS_DDL: list[str] = [ - """ - CREATE TABLE IF NOT EXISTS completion_events ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - instance_id INTEGER NOT NULL REFERENCES quest_instances(id), - child_id INTEGER NOT NULL REFERENCES children(id), - event_type TEXT NOT NULL CHECK (event_type IN ('completed', 'uncompleted')), - actor_source TEXT NOT NULL CHECK (actor_source IN ('user', 'panel')), - actor_user_id TEXT, - occurred_at TEXT NOT NULL, - was_on_time INTEGER CHECK ( - was_on_time IS NULL - OR (typeof(was_on_time) = 'integer' AND was_on_time IN (0, 1)) - ), - CHECK ( - (actor_source = 'user' AND actor_user_id IS NOT NULL) - OR (actor_source = 'panel' AND actor_user_id IS NULL) - ), - CHECK ( - (event_type = 'completed' AND was_on_time IS NOT NULL) - OR (event_type = 'uncompleted') - ) +#: Column body of ``completion_events``, shared between the v1 DDL +#: below and the actor-child rebuild migration, which must recreate the +#: table in exactly this shape (D-008: ``actor_child_id`` records which +#: panel profile was tapped — set for panel events, NULL for admin +#: ones; legacy panel rows backfill to ``child_id``, the child they +#: always were). +COMPLETION_EVENTS_TABLE_SQL = """( + id INTEGER PRIMARY KEY AUTOINCREMENT, + instance_id INTEGER NOT NULL REFERENCES quest_instances(id), + child_id INTEGER NOT NULL REFERENCES children(id), + event_type TEXT NOT NULL CHECK (event_type IN ('completed', 'uncompleted')), + actor_source TEXT NOT NULL CHECK (actor_source IN ('user', 'panel')), + actor_user_id TEXT, + actor_child_id INTEGER REFERENCES children(id), + occurred_at TEXT NOT NULL, + was_on_time INTEGER CHECK ( + was_on_time IS NULL + OR (typeof(was_on_time) = 'integer' AND was_on_time IN (0, 1)) + ), + CHECK ( + (actor_source = 'user' AND actor_user_id IS NOT NULL) + OR (actor_source = 'panel' AND actor_user_id IS NULL) + ), + CHECK ( + (actor_source = 'user' AND actor_child_id IS NULL) + OR (actor_source = 'panel' AND actor_child_id IS NOT NULL) + ), + CHECK ( + (event_type = 'completed' AND was_on_time IS NOT NULL) + OR (event_type = 'uncompleted') ) +)""" + +SCHEMA_V1_COMPLETION_EVENTS_DDL: list[str] = [ + f""" + CREATE TABLE IF NOT EXISTS completion_events {COMPLETION_EVENTS_TABLE_SQL} """, ] diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index fe79027..5894b38 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -664,6 +664,7 @@ def test_append_panel_event_round_trips(tmp_path) -> None: definition.id, child.id, D1, _now_stamp(), window="morning") event = await events.append( instance.id, child.id, "completed", "panel", _now_stamp(), False, + actor_child_id=child.id, actor_user_id=None, ) assert event.actor_source == "panel" @@ -718,17 +719,31 @@ def test_append_validation_rejects_bad_shapes(tmp_path) -> None: with pytest.raises(ValueError, match="must not carry"): await events.append( instance.id, child.id, "completed", "panel", _now_stamp(), True, + actor_child_id=child.id, actor_user_id="user-1", ) + with pytest.raises(ValueError, match="requires actor_child_id"): + await events.append( + instance.id, child.id, "completed", "panel", + _now_stamp(), True, + ) + with pytest.raises(ValueError, match="must not carry actor_child_id"): + await events.append( + instance.id, child.id, "completed", "user", _now_stamp(), True, + actor_user_id="user-1", + actor_child_id=child.id, + ) with pytest.raises(ValueError, match="requires was_on_time"): await events.append( - instance.id, child.id, "completed", "panel", _now_stamp(), None + instance.id, child.id, "completed", "panel", _now_stamp(), None, + actor_child_id=child.id ) # was_on_time must be a real bool: int-convertible junk that # would silently coerce (0.5 -> False) is rejected outright. with pytest.raises(ValueError, match="must be True, False or None"): await events.append( - instance.id, child.id, "completed", "panel", _now_stamp(), 0.5 + instance.id, child.id, "completed", "panel", + _now_stamp(), 0.5, actor_child_id=child.id, ) with pytest.raises(ValueError, match="must be True, False or None"): await events.append( @@ -758,7 +773,7 @@ def test_append_rejects_non_utc_timestamps(tmp_path) -> None: with pytest.raises(ValueError, match="UTC"): await events.append( instance.id, child.id, "completed", "panel", bad, - True, + True, actor_child_id=child.id, ) return None @@ -815,14 +830,16 @@ def test_completed_uncompleted_recompleted_three_ordered_events( instance = await instances.upsert( definition.id, child.id, D1, _now_stamp(), window="morning") await events.append( - instance.id, child.id, "completed", "panel", _now_stamp(), True + instance.id, child.id, "completed", "panel", _now_stamp(), True, + actor_child_id=child.id ) await events.append( instance.id, child.id, "uncompleted", "user", _now_stamp(), True, actor_user_id="user-1", ) await events.append( - instance.id, child.id, "completed", "panel", _now_stamp(), False + instance.id, child.id, "completed", "panel", _now_stamp(), False, + actor_child_id=child.id ) history = await events.list_by_instance(instance.id) assert [e.event_type for e in history] == [ @@ -855,7 +872,8 @@ def test_get_latest_returns_uncompleted_when_reversed(tmp_path) -> None: instance = await instances.upsert( definition.id, child.id, D1, _now_stamp(), window="morning") await events.append( - instance.id, child.id, "completed", "panel", _now_stamp(), True + instance.id, child.id, "completed", "panel", _now_stamp(), True, + actor_child_id=child.id ) await events.append( instance.id, child.id, "uncompleted", "user", _now_stamp(), True, @@ -878,17 +896,20 @@ def test_list_by_child_and_date_range_scopes_by_due_date(tmp_path) -> None: late = await instances.upsert( definition.id, child.id, D20, _now_stamp(), window="morning") await events.append( - early.id, child.id, "completed", "panel", _now_stamp(), True + early.id, child.id, "completed", "panel", _now_stamp(), True, + actor_child_id=child.id ) await events.append( - inside.id, child.id, "completed", "panel", _now_stamp(), False + inside.id, child.id, "completed", "panel", _now_stamp(), False, + actor_child_id=child.id ) await events.append( inside.id, child.id, "uncompleted", "user", _now_stamp(), True, actor_user_id="user-1", ) await events.append( - late.id, child.id, "completed", "panel", _now_stamp(), True + late.id, child.id, "completed", "panel", _now_stamp(), True, + actor_child_id=child.id ) found = await events.list_by_child_and_date_range( child.id, D2, D2 @@ -922,7 +943,7 @@ def test_list_events_occurred_at_scope_variant(tmp_path) -> None: # queried on today's date finds it too. event = await events.append( late_due.id, child.id, "completed", "panel", - _now_stamp(), False, + _now_stamp(), False, actor_child_id=child.id, ) by_due = await events.list_by_child_and_date_range( child.id, D20, D20 @@ -1010,7 +1031,12 @@ def test_no_mutation_sql_for_completion_events_anywhere() -> None: # UPDATE/DELETE/non-DAO-INSERT paths hit the pattern. insert_exempt = { "custom_components/nestquest/dao_instances.py", + "custom_components/nestquest/migrations.py", # the rebuild + # migrations re-create the table via INSERT..SELECT (append- + # order preserved); UPDATE/DELETE stays forbidden everywhere "tests/test_schema.py", + "tests/test_migrations.py", # seeds frozen legacy shapes to + # exercise the rebuild migrations "tests/test_dao_instances.py", # this guard file; scanned by # its own guard's regex spans below instead } diff --git a/tests/test_dao_matrix.py b/tests/test_dao_matrix.py index 2360ffe..a2ab3ba 100644 --- a/tests/test_dao_matrix.py +++ b/tests/test_dao_matrix.py @@ -372,14 +372,16 @@ def test_matrix_completion_events_append_read_and_no_mutation( async def _body(w: World): # insert: append a completion then a reversal, then re-complete await w.events.append( - w.instance.id, w.child.id, "completed", "panel", _stamp(), True + w.instance.id, w.child.id, "completed", "panel", _stamp(), True, + actor_child_id=w.child.id, ) await w.events.append( w.instance.id, w.child.id, "uncompleted", "user", _stamp(), None, actor_user_id="user-1", ) await w.events.append( - w.instance.id, w.child.id, "completed", "panel", _stamp(), False + w.instance.id, w.child.id, "completed", "panel", _stamp(), False, + actor_child_id=w.child.id, ) # read: full ordered history + latest history = await w.events.list_by_instance(w.instance.id) diff --git a/tests/test_migrations.py b/tests/test_migrations.py index 8eee3ee..7cf0624 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -149,16 +149,17 @@ def test_migration_list_shape() -> None: (D-007) to create the quest_* tables directly; migration 2 exists for dev databases stamped version 1 by the pre-rename runner, migration 3 brings pre-D-008 definitions to the multi-assignee - model, migration 4 adds the windows table, and migration 5 rebuilds + model, migration 4 adds the windows table, migration 5 rebuilds quest_instances onto the widened (definition, child, date, window) - key. + key, and migration 6 adds actor_child_id to completion_events. """ assert MIGRATIONS[0] == SCHEMA_V1_STATEMENTS assert callable(MIGRATIONS[1]) assert callable(MIGRATIONS[2]) assert MIGRATIONS[3] == SCHEMA_V1_QUEST_DEFINITION_WINDOWS_DDL assert callable(MIGRATIONS[4]) - assert len(MIGRATIONS) == 5 + assert callable(MIGRATIONS[5]) + assert len(MIGRATIONS) == 6 # --------------------------------------------------------------------------- @@ -442,6 +443,21 @@ _V2_INSTANCES_DDL = [ """, ] +_V2_EVENTS_DDL = [ + """ + CREATE TABLE IF NOT EXISTS completion_events ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + instance_id INTEGER NOT NULL REFERENCES quest_instances(id), + child_id INTEGER NOT NULL REFERENCES children(id), + event_type TEXT NOT NULL, + actor_source TEXT NOT NULL, + actor_user_id TEXT, + occurred_at TEXT NOT NULL, + was_on_time INTEGER + ) + """, +] + def _seed_v2_multi_assignee_upgrade(database) -> None: """Create a database shaped like a post-rename, pre-D-008 dev file. @@ -458,12 +474,13 @@ def _seed_v2_multi_assignee_upgrade(database) -> None: "quest_definition_assignees ", "quest_definition_windows ", "quest_instances ", + "completion_events ", ) for sql in SCHEMA_V1_STATEMENTS: if any(name in sql for name in _v2_replaced): continue _run(database.execute(sql)) - for sql in _V2_DEFINITIONS_DDL + _V2_INSTANCES_DDL: + for sql in _V2_DEFINITIONS_DDL + _V2_INSTANCES_DDL + _V2_EVENTS_DDL: _run(database.execute(sql)) _run(database.execute(VERSION_TABLE_DDL)) _run( @@ -503,6 +520,26 @@ def _seed_v2_multi_assignee_upgrade(database) -> None: "VALUES (1, 2, '2026-09-15', '2026-09-14T00:00:00+00:00')" ) ) + # One legacy panel event and one legacy admin event: the panel one + # must backfill actor_child_id to its own child; the admin one must + # stay NULL. + _run( + database.execute( + "INSERT INTO completion_events (instance_id, child_id, " + "event_type, actor_source, occurred_at, was_on_time) " + "VALUES (1, 2, 'completed', 'panel', " + "'2026-09-15T10:00:00+00:00', 1)" + ) + ) + _run( + database.execute( + "INSERT INTO completion_events (instance_id, child_id, " + "event_type, actor_source, actor_user_id, occurred_at, " + "was_on_time) " + "VALUES (1, 2, 'uncompleted', 'user', 'parent-1', " + "'2026-09-15T11:00:00+00:00', 1)" + ) + ) def test_v2_definitions_rebuilt_to_multi_assignee(tmp_path) -> None: @@ -559,6 +596,23 @@ def test_v2_definitions_rebuilt_to_multi_assignee(tmp_path) -> None: assert [ "definition_id", "child_id", "due_date", "window" ] in unique_columns + # The completion events survived the actor_child_id rebuild: + # the panel row backfilled to its own child, the admin row + # stayed NULL, and ids are preserved. + events = _run( + database.fetch_all( + "SELECT id, event_type, actor_source, actor_child_id " + "FROM completion_events ORDER BY id" + ) + ) + assert events == [ + (1, "completed", "panel", 2), + (2, "uncompleted", "user", None), + ] + columns = _run( + database.fetch_all("PRAGMA table_info(completion_events)") + ) + assert "actor_child_id" in {c[1] for c in columns} # No dangling references anywhere. assert _run(database.fetch_all("PRAGMA foreign_key_check")) == [] # The rebuilt table is live for the new model: a definition diff --git a/tests/test_schema.py b/tests/test_schema.py index e27c774..f869176 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -1851,6 +1851,7 @@ def test_completion_events_columns_types_and_constraints(tmp_path) -> None: ("event_type", "TEXT", 1, None, 0), ("actor_source", "TEXT", 1, None, 0), ("actor_user_id", "TEXT", 0, None, 0), + ("actor_child_id", "INTEGER", 0, None, 0), ("occurred_at", "TEXT", 1, None, 0), ("was_on_time", "INTEGER", 0, None, 0), ] @@ -1881,6 +1882,7 @@ def test_completion_events_foreign_keys_declared(tmp_path) -> None: assert declared == { ("quest_instances", "instance_id", "id"), ("children", "child_id", "id"), + ("children", "actor_child_id", "id"), } finally: _run(database.close()) @@ -2182,16 +2184,71 @@ def test_completion_events_valid_panel_completion_round_trips(tmp_path) -> None: instance_id=instance_id, actor_source="panel", actor_user_id=None, + actor_child_id=1, was_on_time=0, ) row = _run( database.fetch_one( - "SELECT actor_source, actor_user_id, was_on_time " - "FROM completion_events WHERE id = ?", + "SELECT actor_source, actor_user_id, actor_child_id, " + "was_on_time FROM completion_events WHERE id = ?", (event_id,), ) ) - assert row == ("panel", None, 0) + assert row == ("panel", None, 1, 0) + finally: + _run(database.close()) + + +@pytest.mark.parametrize( + ("actor_source", "actor_user_id", "actor_child_id"), + [ + ("user", "user-1", 1), # admin event must not carry a profile + ("panel", None, None), # panel event requires the profile + ("panel", "user-1", 1), # panel must not carry a user id either + ("panel", "user-1", None), + ("user", None, None), + ], +) +def test_completion_events_actor_child_coherence_enforced( + tmp_path, actor_source, actor_user_id, actor_child_id +) -> None: + """D-008 actor-pair shape: 'user' events carry actor_user_id and no + actor_child_id; 'panel' events carry actor_child_id and no + actor_user_id. The CHECKs make both halves non-negotiable.""" + database = _open_db(tmp_path / "completion-events-coherence.db") + try: + _apply(database) + _setup_definition(database) + instance_id = _insert_instance(database, definition_id=1) + with pytest.raises(sqlite3.IntegrityError, match="CHECK"): + _insert_event( + database, + instance_id=instance_id, + event_type="completed", + actor_source=actor_source, + actor_user_id=actor_user_id, + actor_child_id=actor_child_id, + was_on_time=1, + ) + finally: + _run(database.close()) + + +def test_completion_events_actor_child_unknown_child_fails(tmp_path) -> None: + database = _open_db(tmp_path / "completion-events-bad-actor-child.db") + try: + _apply(database) + _setup_definition(database) + instance_id = _insert_instance(database, definition_id=1) + with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"): + _insert_event( + database, + instance_id=instance_id, + actor_source="panel", + actor_user_id=None, + actor_child_id=999, + was_on_time=1, + ) finally: _run(database.close())