Deactivate and reactivate a quest definition (Feature 06, task 5)
This commit is contained in:
@@ -3,10 +3,11 @@
|
|||||||
Sits between the Feature 09 service gate and the typed
|
Sits between the Feature 09 service gate and the typed
|
||||||
:class:`~.dao_rules.QuestDefinitionsDao`: callers get validation and the
|
:class:`~.dao_rules.QuestDefinitionsDao`: callers get validation and the
|
||||||
all-or-nothing create/edit here, and never touch the DAO SQL directly.
|
all-or-nothing create/edit here, and never touch the DAO SQL directly.
|
||||||
:func:`create_quest_definition` is the create path and
|
:func:`create_quest_definition` is the create path,
|
||||||
:func:`edit_quest_definition` the edit path; :func:`assign_child` and
|
:func:`edit_quest_definition` the edit path, :func:`assign_child` and
|
||||||
:func:`unassign_child` are the assignment paths. There is no permission
|
:func:`unassign_child` the assignment paths, and
|
||||||
check here (that is Feature 09).
|
:func:`set_quest_definition_active` the deactivate/reactivate path.
|
||||||
|
There is no permission check here (that is Feature 09).
|
||||||
|
|
||||||
Validation policy mirrors :mod:`.children`:
|
Validation policy mirrors :mod:`.children`:
|
||||||
|
|
||||||
@@ -40,12 +41,13 @@ from __future__ import annotations
|
|||||||
import datetime
|
import datetime
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
from .dao_children import ChildRecord
|
from .dao_children import ChildRecord, _connection_lock
|
||||||
from .dao_rules import (
|
from .dao_rules import (
|
||||||
QuestDefinitionRecord,
|
QuestDefinitionRecord,
|
||||||
QuestDefinitionWindowRecord,
|
QuestDefinitionWindowRecord,
|
||||||
QuestDefinitionsDao,
|
QuestDefinitionsDao,
|
||||||
ScheduleRuleStorage,
|
ScheduleRuleStorage,
|
||||||
|
ScheduleRulesDao,
|
||||||
_UNSET,
|
_UNSET,
|
||||||
_validate_time,
|
_validate_time,
|
||||||
_validate_window,
|
_validate_window,
|
||||||
@@ -59,7 +61,7 @@ from .recurrence import ScheduleRule
|
|||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class CreatedQuestDefinition:
|
class CreatedQuestDefinition:
|
||||||
"""The persisted result of a create or edit.
|
"""The persisted result of a create, edit or activation change.
|
||||||
|
|
||||||
``definition`` is the stored row; ``rule`` is the schedule rule
|
``definition`` is the stored row; ``rule`` is the schedule rule
|
||||||
decoded back through the storage mapping; ``assignees`` and
|
decoded back through the storage mapping; ``assignees`` and
|
||||||
@@ -393,3 +395,60 @@ async def unassign_child(
|
|||||||
f"definition_id: quest definition {definition_id} does not exist"
|
f"definition_id: quest definition {definition_id} does not exist"
|
||||||
)
|
)
|
||||||
return await dao.remove_assignee(definition_id, child_id)
|
return await dao.remove_assignee(definition_id, child_id)
|
||||||
|
|
||||||
|
|
||||||
|
async def set_quest_definition_active(
|
||||||
|
database: NestQuestDatabase,
|
||||||
|
definition_id: int,
|
||||||
|
is_active: bool,
|
||||||
|
) -> CreatedQuestDefinition:
|
||||||
|
"""Deactivate or reactivate a definition; returns the updated view.
|
||||||
|
|
||||||
|
Flipping the flag is the only removal path (Feature 06 guardrail):
|
||||||
|
a deactivated definition stops future instance generation but its
|
||||||
|
row, schedule rule, assignees and windows all survive, and existing
|
||||||
|
instances plus completion history are never touched. Reactivation
|
||||||
|
re-enables future generation; generating or removing instances is
|
||||||
|
Feature 07 and does not happen here.
|
||||||
|
|
||||||
|
``definition_id`` must be a plain int (bools and floats rejected,
|
||||||
|
since SQLite would bind ``True`` onto definition 1) and
|
||||||
|
``is_active`` must be a real bool — the DAO's ``int()`` would
|
||||||
|
otherwise silently coerce strings and numerics ("1", 0) into a
|
||||||
|
state the caller never asked for. Raises ValueError naming the
|
||||||
|
offending field when the id is malformed, ``is_active`` is not a
|
||||||
|
bool, or the definition does not exist.
|
||||||
|
|
||||||
|
Atomicity: the existence check, the UPDATE and the readback of the
|
||||||
|
definition, its rule, assignees and windows all run inside the
|
||||||
|
connection-scoped lock, so the returned bundle is a consistent
|
||||||
|
snapshot that a concurrent mutation cannot race.
|
||||||
|
"""
|
||||||
|
_validate_definition_id(definition_id)
|
||||||
|
if not isinstance(is_active, bool):
|
||||||
|
raise ValueError(f"is_active must be a real bool, got {is_active!r}")
|
||||||
|
async with _connection_lock(database):
|
||||||
|
dao = QuestDefinitionsDao(database)
|
||||||
|
if await dao.get(definition_id) is None:
|
||||||
|
raise ValueError(
|
||||||
|
f"definition_id: quest definition {definition_id} "
|
||||||
|
"does not exist"
|
||||||
|
)
|
||||||
|
await dao.set_active(definition_id, is_active)
|
||||||
|
updated = await dao.get(definition_id)
|
||||||
|
assert updated is not None
|
||||||
|
rule_record = await ScheduleRulesDao(database).get(
|
||||||
|
updated.schedule_rule_id
|
||||||
|
)
|
||||||
|
assert rule_record is not None
|
||||||
|
assignees = await dao.list_assignees(definition_id)
|
||||||
|
windows = await dao.list_windows(definition_id)
|
||||||
|
rule = schedule_rule_from_storage(
|
||||||
|
schedule_rule_storage_from_record(rule_record)
|
||||||
|
)
|
||||||
|
return CreatedQuestDefinition(
|
||||||
|
definition=updated,
|
||||||
|
rule=rule,
|
||||||
|
assignees=assignees,
|
||||||
|
windows=windows,
|
||||||
|
)
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ from custom_components.nestquest.quest_definitions import (
|
|||||||
assign_child,
|
assign_child,
|
||||||
create_quest_definition,
|
create_quest_definition,
|
||||||
edit_quest_definition,
|
edit_quest_definition,
|
||||||
|
set_quest_definition_active,
|
||||||
unassign_child,
|
unassign_child,
|
||||||
)
|
)
|
||||||
from custom_components.nestquest.recurrence import RuleType, ScheduleRule
|
from custom_components.nestquest.recurrence import RuleType, ScheduleRule
|
||||||
@@ -1150,3 +1151,148 @@ def test_concurrent_assign_unassign_returns_consistent_roster(
|
|||||||
return assignees
|
return assignees
|
||||||
|
|
||||||
_with_db(tmp_path, "assign-unassign-concurrent.db")(_body)
|
_with_db(tmp_path, "assign-unassign-concurrent.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# set_quest_definition_active: deactivate / reactivate
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_quest_definition_active_round_trip(tmp_path) -> None:
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
definition_id = created.definition.id
|
||||||
|
|
||||||
|
deactivated = await set_quest_definition_active(
|
||||||
|
database, definition_id, False
|
||||||
|
)
|
||||||
|
assert isinstance(deactivated, CreatedQuestDefinition)
|
||||||
|
assert deactivated.definition.id == definition_id
|
||||||
|
assert deactivated.definition.is_active is False
|
||||||
|
# The decoded rule, assignees and windows form a consistent
|
||||||
|
# snapshot of the (unchanged) definition.
|
||||||
|
assert deactivated.rule == _daily_rule()
|
||||||
|
assert [c.id for c in deactivated.assignees] == [child.id]
|
||||||
|
assert [w.window for w in deactivated.windows] == ["morning"]
|
||||||
|
assert await definitions.list_active() == []
|
||||||
|
|
||||||
|
reactivated = await set_quest_definition_active(
|
||||||
|
database, definition_id, True
|
||||||
|
)
|
||||||
|
assert reactivated.definition.is_active is True
|
||||||
|
assert [d.id for d in await definitions.list_active()] == [
|
||||||
|
definition_id
|
||||||
|
]
|
||||||
|
return reactivated
|
||||||
|
|
||||||
|
_with_db(tmp_path, "set-active-round-trip.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad", ["1", "0", 1, 0, 1.0, None, "true"])
|
||||||
|
def test_set_quest_definition_active_rejects_non_bool(tmp_path, bad) -> None:
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
with pytest.raises(
|
||||||
|
ValueError, match="is_active must be a real bool"
|
||||||
|
):
|
||||||
|
await set_quest_definition_active(
|
||||||
|
database, created.definition.id, bad
|
||||||
|
)
|
||||||
|
# The refused transition left the definition active.
|
||||||
|
stored = await definitions.get(created.definition.id)
|
||||||
|
assert stored is not None
|
||||||
|
assert stored.is_active is True
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "set-active-non-bool.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad", [True, False, 1.5, "1", None])
|
||||||
|
def test_set_quest_definition_active_rejects_non_int_definition_id(
|
||||||
|
tmp_path, bad
|
||||||
|
) -> None:
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
with pytest.raises(
|
||||||
|
ValueError, match="definition_id must be an integer"
|
||||||
|
):
|
||||||
|
await set_quest_definition_active(database, bad, False)
|
||||||
|
stored = await definitions.get(created.definition.id)
|
||||||
|
assert stored is not None
|
||||||
|
assert stored.is_active is True
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "set-active-non-int-id.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_quest_definition_active_nonexistent_definition_names_field(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
with pytest.raises(
|
||||||
|
ValueError, match="definition_id.*does not exist"
|
||||||
|
):
|
||||||
|
await set_quest_definition_active(database, 999, False)
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "set-active-missing-definition.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_quest_definition_active_never_deletes_or_touches_instances(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
"""Deactivation hides the definition but deletes nothing, and the
|
||||||
|
instance/history tables stay exactly as they were."""
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
|
||||||
|
)
|
||||||
|
definition_id = created.definition.id
|
||||||
|
rule_id = created.definition.schedule_rule_id
|
||||||
|
|
||||||
|
instance_count = await database.fetch_one(
|
||||||
|
"SELECT COUNT(*) FROM quest_instances"
|
||||||
|
)
|
||||||
|
event_count = await database.fetch_one(
|
||||||
|
"SELECT COUNT(*) FROM completion_events"
|
||||||
|
)
|
||||||
|
|
||||||
|
await set_quest_definition_active(database, definition_id, False)
|
||||||
|
|
||||||
|
# The definition row survives (deactivated, not deleted).
|
||||||
|
stored = await definitions.get(definition_id)
|
||||||
|
assert stored is not None
|
||||||
|
assert stored.is_active is False
|
||||||
|
# Its rule, assignee and window rows all survive too.
|
||||||
|
assert await rules.get(rule_id) is not None
|
||||||
|
assert [c.id for c in await definitions.list_assignees(
|
||||||
|
definition_id
|
||||||
|
)] == [child.id]
|
||||||
|
assert [w.window for w in await definitions.list_windows(
|
||||||
|
definition_id
|
||||||
|
)] == ["morning"]
|
||||||
|
# No instances or completion events were created or removed.
|
||||||
|
assert await database.fetch_one(
|
||||||
|
"SELECT COUNT(*) FROM quest_instances"
|
||||||
|
) == instance_count
|
||||||
|
assert await database.fetch_one(
|
||||||
|
"SELECT COUNT(*) FROM completion_events"
|
||||||
|
) == event_count
|
||||||
|
|
||||||
|
await set_quest_definition_active(database, definition_id, True)
|
||||||
|
assert (await definitions.get(definition_id)).is_active is True
|
||||||
|
assert await database.fetch_one(
|
||||||
|
"SELECT COUNT(*) FROM quest_instances"
|
||||||
|
) == instance_count
|
||||||
|
assert await database.fetch_one(
|
||||||
|
"SELECT COUNT(*) FROM completion_events"
|
||||||
|
) == event_count
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "set-active-never-deletes.db")(_body)
|
||||||
|
|||||||
Reference in New Issue
Block a user