Reorder children for panel display order (Feature 03, task 3)

- ChildrenDao.reorder now requires a COMPLETE permutation of the
  children table: partial lists (missing an existing child), unknown
  ids and duplicates are rejected with ValueError before any write,
  inside the existing transaction and connection lock (check and
  write atomic). Empty list is the maximally partial reorder against
  a non-empty table; against an empty table it stays a no-op. The two
  DAO tests that encoded the old permissive semantics (silent no-op
  on empty/unknown) are updated to the new contract.
- children.py gains reorder_children: the Feature 09-facing wrapper,
  which additionally rejects non-integer ids up front (bools and
  floats would silently compare against stored integer ids at the SQL
  layer).
- Business tests: round-trip (active list returns new order), partial/
  unknown/duplicate/non-int rejections with order-unchanged proofs,
  empty-list rejection, and inactive children keeping positions in a
  whole-table permutation. Suite: 866 passed.
This commit is contained in:
Joshua King
2026-09-15 16:41:14 -04:00
parent ecffa5bb0e
commit 7a585afb9e
4 changed files with 184 additions and 11 deletions
+24
View File
@@ -236,6 +236,30 @@ async def set_child_active(
return updated return updated
async def reorder_children(
database: NestQuestDatabase, ordered_ids: list[int]
) -> None:
"""Rewrite the children's panel display order in one transaction.
``ordered_ids`` must be a complete permutation of the children
table — every child id exactly once. A partial list (missing an
existing child), an unknown id, or a duplicate is rejected with
ValueError before any write, so the current order survives a
rejected call. The DAO enforces this inside its own transaction
(check and write are atomic under the connection lock); this
wrapper additionally rejects non-integer ids up front, since
``bool`` and floats would silently compare against stored integer
ids at the SQL layer.
"""
for child_id in ordered_ids:
if isinstance(child_id, bool) or not isinstance(child_id, int):
raise ValueError(
f"ordered_ids must contain only integer child ids, got "
f"{child_id!r}"
)
await ChildrenDao(database).reorder(ordered_ids)
async def list_children( async def list_children(
database: NestQuestDatabase, *, active_only: bool = False database: NestQuestDatabase, *, active_only: bool = False
) -> list[ChildRecord]: ) -> list[ChildRecord]:
+37 -7
View File
@@ -191,13 +191,20 @@ class ChildrenDao:
async def reorder(self, ordered_ids: list[int]) -> None: async def reorder(self, ordered_ids: list[int]) -> None:
"""Persist a new sort order: ``ordered_ids[i]`` gets sort_order i. """Persist a new sort order: ``ordered_ids[i]`` gets sort_order i.
The list must be a COMPLETE permutation of the children table:
every existing child id appears exactly once. Partial lists
(missing an existing child) and unknown ids (naming a child
that does not exist) are rejected with ValueError BEFORE any
write, leaving the current order untouched — a silent partial
reorder would leave unlisted children with stale positions and
hide a caller bug. Duplicate ids are rejected up front: they
cannot all hold distinct positions, and silently assigning the
last occurrence's index would hide a caller bug.
Runs inside one transaction; positions are staged at offset Runs inside one transaction; positions are staged at offset
-len (all negative, so any permutation of the same ids never -len (all negative, so any permutation of the same ids never
collides mid-reorder under the wrapper's single-connection collides mid-reorder under the wrapper's single-connection
serialization), then finalized 0..n-1. Duplicate ids are serialization), then finalized 0..n-1.
rejected up front: they cannot all hold distinct positions, and
silently assigning the last occurrence's index would hide a
caller bug.
Concurrent reorder() calls on one connection are serialized by Concurrent reorder() calls on one connection are serialized by
a connection-scoped asyncio lock (shared across every ChildrenDao a connection-scoped asyncio lock (shared across every ChildrenDao
@@ -205,10 +212,10 @@ class ChildrenDao:
transaction() rejects a second concurrent transaction with transaction() rejects a second concurrent transaction with
RuntimeError, so racing callers would crash instead of RuntimeError, so racing callers would crash instead of
queueing. The lock makes the second call wait, then run queueing. The lock makes the second call wait, then run
against the first call's final state. against the first call's final state — which also keeps the
completeness check honest: the re-read of the table happens
inside the same lock and transaction as the writes.
""" """
if not ordered_ids:
return
if len(set(ordered_ids)) != len(ordered_ids): if len(set(ordered_ids)) != len(ordered_ids):
raise ValueError( raise ValueError(
"reorder() received duplicate child ids; every id must " "reorder() received duplicate child ids; every id must "
@@ -216,6 +223,29 @@ class ChildrenDao:
) )
async with _connection_lock(self._database): async with _connection_lock(self._database):
async with self._database.transaction(): async with self._database.transaction():
rows = await self._database.fetch_all(
f"SELECT id FROM children"
)
existing = {row[0] for row in rows}
listed = set(ordered_ids)
missing = sorted(existing - listed)
unknown = sorted(listed - existing)
if missing or unknown:
details = []
if missing:
details.append(
f"missing child ids {missing} (a partial "
"reorder would leave them with stale "
"positions)"
)
if unknown:
details.append(
f"unknown child ids {unknown}"
)
raise ValueError(
"reorder() must list every child exactly once; "
+ "; ".join(details)
)
offset = len(ordered_ids) offset = len(ordered_ids)
await self._database.execute_many( await self._database.execute_many(
"UPDATE children SET sort_order = ? WHERE id = ?", "UPDATE children SET sort_order = ? WHERE id = ?",
+109
View File
@@ -12,6 +12,7 @@ from custom_components.nestquest.children import (
create_child, create_child,
edit_child, edit_child,
list_children, list_children,
reorder_children,
set_child_active, set_child_active,
) )
from custom_components.nestquest.dao_children import ChildRecord, ChildrenDao from custom_components.nestquest.dao_children import ChildRecord, ChildrenDao
@@ -441,6 +442,114 @@ def test_set_child_active_missing_raises(tmp_path) -> None:
_with_db(tmp_path, "set-active-missing.db")(_body) _with_db(tmp_path, "set-active-missing.db")(_body)
# ---------------------------------------------------------------------------
# reorder_children: complete permutation or nothing changes
# ---------------------------------------------------------------------------
def test_reorder_children_persists_display_order(tmp_path) -> None:
async def _body(database):
first = await create_child(database, "Ada", sort_order=0)
second = await create_child(database, "Bo", sort_order=1)
third = await create_child(database, "Cleo", sort_order=2)
await reorder_children(database, [third.id, first.id, second.id])
# The active list returns children in the new sort order.
listed = await list_children(database, active_only=True)
assert [c.id for c in listed] == [third.id, first.id, second.id]
assert [c.sort_order for c in listed] == [0, 1, 2]
return listed
_with_db(tmp_path, "reorder-round-trip.db")(_body)
def test_reorder_children_partial_list_rejected_order_unchanged(
tmp_path,
) -> None:
async def _body(database):
first = await create_child(database, "Ada", sort_order=0)
second = await create_child(database, "Bo", sort_order=1)
third = await create_child(database, "Cleo", sort_order=2)
with pytest.raises(ValueError, match="missing child ids"):
await reorder_children(database, [third.id, first.id])
listed = await list_children(database)
# Nothing changed: rejection happens before any write.
assert [c.id for c in listed] == [first.id, second.id, third.id]
assert [c.sort_order for c in listed] == [0, 1, 2]
return listed
_with_db(tmp_path, "reorder-partial.db")(_body)
def test_reorder_children_unknown_id_rejected_order_unchanged(
tmp_path,
) -> None:
async def _body(database):
first = await create_child(database, "Ada", sort_order=0)
second = await create_child(database, "Bo", sort_order=1)
with pytest.raises(ValueError, match="unknown child ids"):
await reorder_children(database, [second.id, first.id, 999])
listed = await list_children(database)
assert [c.id for c in listed] == [first.id, second.id]
assert [c.sort_order for c in listed] == [0, 1]
return listed
_with_db(tmp_path, "reorder-unknown.db")(_body)
def test_reorder_children_empty_list_rejected_when_children_exist(
tmp_path,
) -> None:
"""An empty list is the maximally partial reorder: rejected while
any child exists (against an empty table it is a complete
permutation and stays a harmless no-op)."""
async def _body(database):
first = await create_child(database, "Ada", sort_order=0)
with pytest.raises(ValueError, match="missing child ids"):
await reorder_children(database, [])
assert [c.sort_order for c in await list_children(database)] == [0]
return None
_with_db(tmp_path, "reorder-empty.db")(_body)
def test_reorder_children_duplicate_and_non_int_rejected(tmp_path) -> None:
async def _body(database):
first = await create_child(database, "Ada", sort_order=0)
second = await create_child(database, "Bo", sort_order=1)
with pytest.raises(ValueError, match="duplicate child ids"):
await reorder_children(database, [first.id, first.id, second.id])
with pytest.raises(ValueError, match="only integer child ids"):
await reorder_children(database, [first.id, second.id, True])
with pytest.raises(ValueError, match="only integer child ids"):
await reorder_children(database, [first.id, second.id, "3"])
listed = await list_children(database)
assert [c.sort_order for c in listed] == [0, 1]
return listed
_with_db(tmp_path, "reorder-bad-inputs.db")(_body)
def test_reorder_children_inactive_children_included(tmp_path) -> None:
"""Inactive children keep their display position: the permutation
covers the whole table, not just the active list."""
async def _body(database):
first = await create_child(database, "Ada", sort_order=0)
second = await create_child(database, "Bo", sort_order=1)
await set_child_active(database, second.id, False)
# Both ids are required even though one is inactive.
await reorder_children(database, [second.id, first.id])
listed = await list_children(database)
assert [c.id for c in listed] == [second.id, first.id]
assert [c.sort_order for c in listed] == [0, 1]
# The active list skips the inactive child but keeps position.
assert [c.id for c in await list_children(database, active_only=True)] == [
first.id
]
return listed
_with_db(tmp_path, "reorder-inactive.db")(_body)
@pytest.mark.parametrize("bad", ["1", "0", 1, 0, 1.0, None, "true"]) @pytest.mark.parametrize("bad", ["1", "0", 1, 0, 1.0, None, "true"])
def test_set_child_active_rejects_non_bool(tmp_path, bad) -> None: def test_set_child_active_rejects_non_bool(tmp_path, bad) -> None:
"""The DAO's int() would silently coerce "1"/0/1.0 into a state the """The DAO's int() would silently coerce "1"/0/1.0 into a state the
+14 -4
View File
@@ -227,24 +227,34 @@ def test_reorder_is_transactional_and_idempotent(tmp_path) -> None:
_run(database.close()) _run(database.close())
def test_reorder_empty_list_is_noop(tmp_path) -> None: def test_reorder_empty_list_is_rejected(tmp_path) -> None:
"""An empty list is the maximally partial reorder: rejected while
any child exists (Feature 03 reorder contract)."""
import pytest as pytest_module
database = _open_db(tmp_path / "reorder-empty.db") database = _open_db(tmp_path / "reorder-empty.db")
try: try:
dao, _ = _daos(database) dao, _ = _daos(database)
_make_child(dao, "Ada") _make_child(dao, "Ada")
_run(dao.reorder([])) with pytest_module.raises(ValueError, match="missing child ids"):
_run(dao.reorder([]))
children = _run(dao.list_all()) children = _run(dao.list_all())
assert children[0].sort_order == 0 assert children[0].sort_order == 0
finally: finally:
_run(database.close()) _run(database.close())
def test_reorder_unknown_id_leaves_it_untouched(tmp_path) -> None: def test_reorder_unknown_id_is_rejected(tmp_path) -> None:
"""An unknown id is rejected before any write; the order is
untouched (Feature 03 reorder contract)."""
import pytest as pytest_module
database = _open_db(tmp_path / "reorder-unknown.db") database = _open_db(tmp_path / "reorder-unknown.db")
try: try:
dao, _ = _daos(database) dao, _ = _daos(database)
a = _make_child(dao, "Ada") a = _make_child(dao, "Ada")
_run(dao.reorder([a.id, 999])) with pytest_module.raises(ValueError, match="unknown child ids"):
_run(dao.reorder([a.id, 999]))
children = _run(dao.list_all()) children = _run(dao.list_all())
assert children[0].sort_order == 0 assert children[0].sort_order == 0
finally: finally: