Feature 22: Admin Definitions Read and skip_on_away (#193, #194) (#195)

Co-authored-by: dev_agent <17+dev_agent@code.cubecraftcreations.com>
Co-committed-by: dev_agent <17+dev_agent@code.cubecraftcreations.com>

custom_components/nestquest/core/dao_rules.py

@@ -105,6 +105,19 @@ def _validate_window(window: str) -> None:
         )
 
 
+def _validate_skip_on_away(value: object) -> int:
+    """Return ``value`` as 0/1, raising ValueError for anything else.
+
+    The fresh-schema CHECK constraint enforces this in SQLite, but a
+    migrated database gained the column through ``ADD COLUMN`` without
+    it, so every DAO write path validates here instead.  Accepts a bool
+    or the ints 0/1; floats, strings and other ints are rejected.
+    """
+    if not isinstance(value, int) or value not in (0, 1):
+        raise ValueError(f"skip_on_away must be 0 or 1, got {value!r}")
+    return int(value)
+
+
 @dataclass(frozen=True)
 class ScheduleRuleRecord:
     """One row of ``schedule_rules``."""
@@ -152,6 +165,7 @@ class QuestDefinitionRecord:
     due_time: str | None
     is_active: bool
     created_at: str
+    skip_on_away: bool
 
 
 @dataclass(frozen=True)
@@ -187,7 +201,7 @@ _RULE_COLUMNS = (
 )
 _DEFINITION_COLUMNS = (
     "id, title, description, icon, schedule_rule_id, due_time, "
-    "is_active, created_at"
+    "is_active, created_at, skip_on_away"
 )
 _WINDOW_COLUMNS = "definition_id, window, due_time"
 
@@ -216,6 +230,7 @@ def _definition_from_row(row: tuple) -> QuestDefinitionRecord:
         due_time=row[5],
         is_active=bool(row[6]),
         created_at=row[7],
+        skip_on_away=bool(row[8]),
     )
 
 
@@ -632,6 +647,7 @@ class QuestDefinitionsDao:
         due_time: str | None = None,
         is_active: bool = True,
         assignee_child_ids: list[int] | None = None,
+        skip_on_away: bool = True,
     ) -> QuestDefinitionRecord:
         """Insert one definition and return the record as stored.
 
@@ -644,6 +660,7 @@ class QuestDefinitionsDao:
         empty list or None creates an unassigned definition, which
         callers may fill via :meth:`add_assignee`.
         """
+        skip_on_away_value = _validate_skip_on_away(skip_on_away)
         async with _connection_lock(self._database):
             async with self._database.transaction():
                 await self._validate_rule_exists(schedule_rule_id)
@@ -652,7 +669,8 @@ class QuestDefinitionsDao:
                 result = await self._database.execute(
                     "INSERT INTO quest_definitions (title, description, "
                     "icon, schedule_rule_id, due_time, is_active, "
-                    "created_at) VALUES (?, ?, ?, ?, ?, ?, ?)",
+                    "created_at, skip_on_away) "
+                    "VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
                     (
                         title,
                         description,
@@ -661,6 +679,7 @@ class QuestDefinitionsDao:
                         due_time,
                         int(is_active),
                         created_at,
+                        skip_on_away_value,
                     ),
                 )
                 definition_id = result.lastrowid
@@ -684,6 +703,7 @@ class QuestDefinitionsDao:
         *,
         description: str | None = None,
         icon: str | None = None,
+        skip_on_away: bool = True,
     ) -> QuestDefinitionSnapshot:
         """Insert a rule, definition, assignees and windows atomically.
 
@@ -713,6 +733,7 @@ class QuestDefinitionsDao:
         _validate_date(schedule_rule.start_date, "start_date")
         if schedule_rule.end_date is not None:
             _validate_date(schedule_rule.end_date, "end_date")
+        skip_on_away_value = _validate_skip_on_away(skip_on_away)
         async with _connection_lock(self._database):
             async with self._database.transaction():
                 for child_id in assignee_child_ids:
@@ -721,8 +742,18 @@ class QuestDefinitionsDao:
                 result = await self._database.execute(
                     "INSERT INTO quest_definitions (title, description, "
                     "icon, schedule_rule_id, due_time, is_active, "
-                    "created_at) VALUES (?, ?, ?, ?, ?, ?, ?)",
-                    (title, description, icon, rule_id, None, 1, created_at),
+                    "created_at, skip_on_away) "
+                    "VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
+                    (
+                        title,
+                        description,
+                        icon,
+                        rule_id,
+                        None,
+                        1,
+                        created_at,
+                        skip_on_away_value,
+                    ),
                 )
                 definition_id = result.lastrowid
                 for child_id in assignee_child_ids:
@@ -764,6 +795,7 @@ class QuestDefinitionsDao:
         icon: str | None | object = _UNSET,
         rule: ScheduleRuleStorage | object = _UNSET,
         windows: list[tuple[str, str | None]] | object = _UNSET,
+        skip_on_away: bool | object = _UNSET,
     ) -> QuestDefinitionSnapshot:
         """Edit a definition's metadata, rule and windows atomically.
 
@@ -774,9 +806,10 @@ class QuestDefinitionsDao:
         ``_UNSET`` meaning "leave this field alone"; ``None`` writes SQL
         NULL so optional metadata (description, icon) can be cleared.
 
-        - ``title``/``description``/``icon`` update the definition row
-          in place; the definition-level ``due_time`` column is never
-          written (per-window due times supersede it, D-008).
+        - ``title``/``description``/``icon``/``skip_on_away`` update the
+          definition row in place; the definition-level ``due_time``
+          column is never written (per-window due times supersede it,
+          D-008).
         - ``rule`` (a pre-validated :class:`ScheduleRuleStorage`)
           overwrites the referenced ``schedule_rules`` row in place: the
           definition keeps its ``schedule_rule_id``, so no new or
@@ -792,6 +825,8 @@ class QuestDefinitionsDao:
         does not exist.  Edits change future instances only: no
         ``quest_instances`` or ``completion_events`` row is touched.
         """
+        if skip_on_away is not _UNSET:
+            skip_on_away = _validate_skip_on_away(skip_on_away)
         async with _connection_lock(self._database):
             async with self._database.transaction():
                 definition = await self.get(definition_id)
@@ -809,6 +844,9 @@ class QuestDefinitionsDao:
                     if value is not _UNSET:
                         assignments.append(f"{column} = ?")
                         parameters.append(value)
+                if skip_on_away is not _UNSET:
+                    assignments.append("skip_on_away = ?")
+                    parameters.append(skip_on_away)
                 if assignments:
                     parameters.append(definition_id)
                     await self._database.execute(
@@ -907,6 +945,13 @@ class QuestDefinitionsDao:
         )
         return [_definition_from_row(row) for row in rows]
 
+    async def list_all(self) -> list[QuestDefinitionRecord]:
+        """Return every definition, active AND inactive, oldest first."""
+        rows = await self._database.fetch_all(
+            f"SELECT {_DEFINITION_COLUMNS} FROM quest_definitions ORDER BY id"
+        )
+        return [_definition_from_row(row) for row in rows]
+
     async def _snapshots_for(
         self, definitions: list[QuestDefinitionRecord]
     ) -> list[QuestDefinitionSnapshot]:
@@ -951,6 +996,18 @@ class QuestDefinitionsDao:
                 definitions = await self.list_active()
                 return await self._snapshots_for(definitions)
 
+    async def list_snapshots_all(self) -> list[QuestDefinitionSnapshot]:
+        """Return every definition (active AND inactive) with its rule,
+        assignees and windows, all read inside one locked transaction.
+
+        Mirrors :meth:`list_snapshots_active` over :meth:`list_all`
+        (oldest first), with the same coherence guarantee.
+        """
+        async with _connection_lock(self._database):
+            async with self._database.transaction():
+                definitions = await self.list_all()
+                return await self._snapshots_for(definitions)
+
     async def read_snapshots_active(self) -> list[QuestDefinitionSnapshot]:
         """Return every active definition's snapshot WITHOUT locking.
 
@@ -990,13 +1047,16 @@ class QuestDefinitionsDao:
         description: str | None | object = _UNSET,
         icon: str | None | object = _UNSET,
         due_time: str | None | object = _UNSET,
+        skip_on_away: bool | None = None,
     ) -> int:
         """Update the given fields; returns rows updated (0 if absent).
 
         Arguments default to the module sentinel ``_UNSET`` meaning
         "leave this column alone"; passing ``None`` explicitly writes
         SQL NULL, so callers can remove optional metadata
-        (description, icon, due_time).  Assignment and activation are
+        (description, icon, due_time).  ``skip_on_away`` is NOT NULL,
+        so its ``None`` default leaves it unchanged; a supplied value
+        must be 0/1 (a bool).  Assignment and activation are
         deliberately NOT settable here: they have their own explicit
         operations (:meth:`add_assignee`, :meth:`remove_assignee`,
         :meth:`set_active`) so they stay separately loggable and
@@ -1013,6 +1073,9 @@ class QuestDefinitionsDao:
             if value is not _UNSET:
                 assignments.append(f"{column} = ?")
                 parameters.append(value)
+        if skip_on_away is not None:
+            assignments.append("skip_on_away = ?")
+            parameters.append(_validate_skip_on_away(skip_on_away))
         if not assignments:
             return 0
         parameters.append(definition_id)

custom_components/nestquest/core/materialize.py

@@ -18,7 +18,8 @@ The walk is deterministic and idempotent:
   :class:`~.presence.PresenceEngine` from that same coherent read.
 - For each date in the range, each definition whose decoded rule
   :func:`~.recurrence.occurs_on` on that date, each assignee who is
-  ``is_present`` on that date and each declared window, one instance is
+  ``is_present`` on that date (or every assignee, when the definition's
+  ``skip_on_away`` is off) and each declared window, one instance is
   written through :class:`~.dao_instances.QuestInstancesDao.upsert_if_valid`
   with a ``generated_at`` stamp shared by the whole batch (one coherent
   time per run).
@@ -208,7 +209,8 @@ async def materialize(
 
     definitions = [
         (snapshot.definition.id, _decode_rule(snapshot),
-         snapshot.assignees, snapshot.windows)
+         snapshot.assignees, snapshot.windows,
+         snapshot.definition.skip_on_away)
         for snapshot in snapshots
     ]
 
@@ -229,13 +231,15 @@ async def materialize(
     cursor = start
     while cursor <= end:
         iso = cursor.isoformat()
-        for definition_id, rule, assignees, windows in definitions:
+        for (
+            definition_id, rule, assignees, windows, skip_on_away
+        ) in definitions:
             if not occurs_on(rule, cursor):
                 continue
             for child in assignees:
                 if child_filter is not None and child.id not in child_filter:
                     continue
-                if not engine.is_present(child.id, cursor):
+                if skip_on_away and not engine.is_present(child.id, cursor):
                     continue
                 for window in windows:
                     written = await instances.upsert_if_valid(

custom_components/nestquest/core/migrations.py

@@ -30,8 +30,9 @@ widened (definition_id, child_id, due_date, window) key, and migration
 6 rebuilds ``completion_events`` with the D-008 ``actor_child_id``
 column.  Migration 7 adds the generic ``nestquest_meta_state``
 key/value table for integration-owned bookkeeping (Feature 11's
-missed-sweep watermark).  Each callable migration runs inside its own
-transaction
+missed-sweep watermark).  Migration 8 adds the ``skip_on_away``
+column to ``quest_definitions`` (Feature 22), defaulting to 1.  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.
 
@@ -400,6 +401,28 @@ async def _add_actor_child_id(
         await database.execute("PRAGMA foreign_keys = ON")
 
 
+async def _add_skip_on_away(
+    database: NestQuestDatabase, target_version: int
+) -> None:
+    """Add the ``skip_on_away`` column to ``quest_definitions``.
+
+    Strictly additive: ``ALTER TABLE ... ADD COLUMN`` with a NOT NULL
+    DEFAULT 1, so existing rows read as 1 (skip absent dates — the
+    behaviour before the flag existed).  Fresh databases already carry
+    the column from the canonical DDL, so the ALTER is guarded and the
+    step only stamps there.
+    """
+    async with database.transaction():
+        if not await _table_has_column(
+            database, "quest_definitions", "skip_on_away"
+        ):
+            await database.execute(
+                "ALTER TABLE quest_definitions ADD COLUMN "
+                "skip_on_away INTEGER NOT NULL DEFAULT 1"
+            )
+        await stamp_schema_version(database, target_version)
+
+
 #: 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.)
@@ -423,6 +446,10 @@ MIGRATIONS: Sequence[MigrationStep] = [
     # fresh databases (which already made it in migration 1) and
     # pre-v7 databases.  Strictly additive; no existing table changes.
     list(SCHEMA_V7_META_STATE_DDL),
+    # Migration 8 (Feature 22): the skip_on_away flag on
+    # quest_definitions — an additive column defaulting to 1, so every
+    # pre-existing definition keeps today's skip-when-absent behaviour.
+    _add_skip_on_away,
 ]
 
 

custom_components/nestquest/core/quest_definitions.py

@@ -191,6 +191,17 @@ def _validate_definition_id(value: object) -> int:
     return value
 
 
+def _validate_skip_on_away(value: object) -> bool:
+    """Reject a non-bool ``skip_on_away`` before any write.
+
+    The DAO's ``int()`` would otherwise silently coerce strings and
+    numerics ("0", 1) into a state the caller never asked for.
+    """
+    if not isinstance(value, bool):
+        raise ValueError(f"skip_on_away must be a real bool, got {value!r}")
+    return value
+
+
 def _validate_child_id(value: object) -> int:
     """Reject non-int child ids (bools included) before any lookup.
 
@@ -212,6 +223,7 @@ async def create_quest_definition(
     *,
     description: str | None = None,
     icon: str | None = None,
+    skip_on_away: bool = True,
 ) -> CreatedQuestDefinition:
     """Create a quest definition (rule + assignees + windows) atomically.
 
@@ -231,6 +243,7 @@ async def create_quest_definition(
     window_specs = _normalize_windows(windows)
     description_value = _validate_text(description, "description")
     icon_value = _validate_text(icon, "icon")
+    skip_on_away_value = _validate_skip_on_away(skip_on_away)
 
     storage = schedule_rule_to_storage(rule)
     dao = QuestDefinitionsDao(database)
@@ -243,6 +256,7 @@ async def create_quest_definition(
             window_specs,
             description=description_value,
             icon=icon_value,
+            skip_on_away=skip_on_away_value,
         )
     except ValueError as error:
         # The DAO's assignee check names only the child; re-raise so the
@@ -271,6 +285,7 @@ async def edit_quest_definition(
     icon: str | None | object = _UNSET,
     rule: ScheduleRule | object = _UNSET,
     windows: list[str | tuple[str, str | None]] | object = _UNSET,
+    skip_on_away: bool | None = None,
     today: datetime.date | None = None,
     horizon_days: int | None = None,
 ) -> CreatedQuestDefinition:
@@ -285,7 +300,9 @@ async def edit_quest_definition(
     ``schedule_rules`` row is updated, never orphaned or duplicated.  A
     provided ``windows`` list REPLACES the whole window set (each entry
     a ``const.QUEST_WINDOWS`` name, optionally with a strict HH:MM due
-    time), matching create's non-empty contract.  Assignment is NOT
+    time), matching create's non-empty contract.  A provided
+    ``skip_on_away`` (a real bool) replaces the stored flag; ``None``
+    leaves it unchanged.  Assignment is NOT
     settable here.  Raises ValueError when the definition does not
     exist or any argument is rejected.
 
@@ -331,6 +348,10 @@ async def edit_quest_definition(
     if windows is not _UNSET:
         window_specs = _normalize_windows(windows)
 
+    skip_on_away_value: bool | object = _UNSET
+    if skip_on_away is not None:
+        skip_on_away_value = _validate_skip_on_away(skip_on_away)
+
     dao = QuestDefinitionsDao(database)
     try:
         snapshot = await dao.edit_definition(
@@ -340,6 +361,7 @@ async def edit_quest_definition(
             icon=icon_value,
             rule=rule_storage,
             windows=window_specs,
+            skip_on_away=skip_on_away_value,
         )
     except ValueError as error:
         # The DAO's missing-definition error names only the id; re-raise
@@ -558,6 +580,21 @@ async def list_active_definitions(
     return [_bundle_snapshot(snapshot) for snapshot in snapshots]
 
 
+async def list_all_definitions(
+    database: NestQuestDatabase,
+) -> list[CreatedQuestDefinition]:
+    """Return EVERY definition, active AND inactive (rich view).
+
+    Mirrors :func:`list_active_definitions` without the activity
+    filter: ordered by rising definition id, each entry the same
+    :class:`CreatedQuestDefinition` snapshot, read by the DAO inside one
+    locked transaction.
+    """
+    dao = QuestDefinitionsDao(database)
+    snapshots = await dao.list_snapshots_all()
+    return [_bundle_snapshot(snapshot) for snapshot in snapshots]
+
+
 async def list_definitions_for_child(
     database: NestQuestDatabase,
     child_id: int,

custom_components/nestquest/core/schema.py

@@ -257,7 +257,8 @@ QUEST_DEFINITIONS_TABLE_SQL = """(
     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
+    created_at TEXT NOT NULL,
+    skip_on_away INTEGER NOT NULL DEFAULT 1 CHECK (skip_on_away IN (0, 1))
 )"""
 
 SCHEMA_V1_QUEST_DEFINITIONS_DDL: list[str] = [