From 77bc7d459628cd52ea6561c197767688fbf09118 Mon Sep 17 00:00:00 2001 From: tempus2016 Date: Wed, 4 Nov 2026 08:08:32 +0000 Subject: [PATCH] fix(chores): deleting a chore clears its timers, and bulk delete clears like a single delete --- custom_components/taskmate/coord_chores.py | 16 ++- custom_components/taskmate/coord_trash.py | 4 + custom_components/taskmate/storage.py | 6 + tests/test_delete_chore_timed_sessions.py | 141 +++++++++++++++++++++ wiki/Recently-Deleted.md | 4 +- 5 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 tests/test_delete_chore_timed_sessions.py diff --git a/custom_components/taskmate/coord_chores.py b/custom_components/taskmate/coord_chores.py index 6beb1f5a..fea79fb1 100644 --- a/custom_components/taskmate/coord_chores.py +++ b/custom_components/taskmate/coord_chores.py @@ -423,16 +423,17 @@ async def async_bulk_chore_action( if action not in ("delete", "enable", "disable", "reassign"): raise ValueError(f"Unknown bulk action {action}") count = 0 - called_off = [] for cid in ids: chore = self.storage.get_chore(cid) if not chore: continue if action == "delete": - self._snapshot_chore(cid) - self.storage.remove_chore(cid) - # Its auctions go with it, live ones called off (#999). - called_off += self.remove_chore_from_auctions(cid) + # The same delete as one at a time (#771): it snapshots for + # Recently deleted and clears completions, timers, groups, + # auctions (live ones called off, #999) and the rest. Removing + # only the chore left its completions behind, so a restore + # then added them a second time. + await self.async_remove_chore(cid, refresh=False) elif action == "enable": chore.enabled = True chore.disabled_for = [] @@ -446,8 +447,6 @@ async def async_bulk_chore_action( count += 1 await self.storage.async_save() await self.async_refresh() - for auction, bidders in called_off: - await self._async_tell_bidders_called_off(auction, bidders) return count async def async_approve_chores_bulk( @@ -676,6 +675,9 @@ async def async_remove_chore(self, chore_id: str, *, keep_deleted: bool = True, self.storage.remove_swap_requests_for_chore(chore_id) # Teamwork joins (#928) belong to the occurrence of a chore that's gone. self.storage.remove_team_joins_for_chore(chore_id) + # A running or paused timer on it goes now, not at the midnight sweep. + # Not snapshotted, so a restore never brings a stale timer back (#771). + self.storage.remove_timed_sessions_for_chore(chore_id) # Its auctions (#982) go with it; live ones are called off (#999). called_off = self.remove_chore_from_auctions(chore_id) # Remove chore from children's chore_order lists diff --git a/custom_components/taskmate/coord_trash.py b/custom_components/taskmate/coord_trash.py index 03aeeef6..235378c7 100644 --- a/custom_components/taskmate/coord_trash.py +++ b/custom_components/taskmate/coord_trash.py @@ -164,6 +164,10 @@ async def _restore_chore(self, item: dict, extra: dict, children: dict) -> None: # Its calendar events were taken down with it; publish them afresh. chore.publish_calendar_published_dates = [] self.storage.add_chore(chore) + # A timer left behind by a delete made before sessions were cleared + # with the chore would reattach to it here, still "running" since the + # delete. A restored chore starts with no timer (#771). + self.storage.remove_timed_sessions_for_chore(chore.id) for comp in extra.get("completions", []): if comp.get("child_id") in children or comp.get("child_id") == "__parent__": data.setdefault("completions", []).append(comp) diff --git a/custom_components/taskmate/storage.py b/custom_components/taskmate/storage.py index cb6c77a9..03085d66 100644 --- a/custom_components/taskmate/storage.py +++ b/custom_components/taskmate/storage.py @@ -2369,6 +2369,12 @@ def remove_timed_session(self, session_id: str) -> None: """Remove a timed session.""" self._data["timed_sessions"] = [s for s in self._data.get("timed_sessions", []) if s.get("id") != session_id] + def remove_timed_sessions_for_chore(self, chore_id: str) -> None: + """Drop a deleted chore's timed sessions, running or paused (#771).""" + self._data["timed_sessions"] = [ + s for s in self._data.get("timed_sessions", []) if s.get("chore_id") != chore_id + ] + def remove_timed_sessions_for_child(self, child_id: str) -> None: """Drop a deleted child's timed sessions (#946).""" self._data["timed_sessions"] = [ diff --git a/tests/test_delete_chore_timed_sessions.py b/tests/test_delete_chore_timed_sessions.py new file mode 100644 index 00000000..eccb7bf0 --- /dev/null +++ b/tests/test_delete_chore_timed_sessions.py @@ -0,0 +1,141 @@ +"""Deleting a chore clears its timed sessions straight away. + +A running or paused timer on a deleted chore used to stay in storage until the +midnight stale-session sweep, and a bulk delete left the chore's completions +and other per-chore records behind as well — so restoring it from Recently +deleted then added its completions a second time. Every delete path now goes +through async_remove_chore, and a restore never brings a timer back. +""" + +from __future__ import annotations + +from custom_components.taskmate.models import ChoreCompletion, TimedSession + +from .test_completion_eligibility import _now, run +from .test_recently_deleted import _at, _chore, _system + + +def _timed(coord, name="Reading"): + chore = _chore(coord, name) + chore.task_type = "timed" + chore.timed_rate_minutes = 5 + chore.timed_rate_points = 2 + coord.storage.update_chore(chore) + return chore + + +def _session(storage, chore, child, state="paused"): + storage.save_timed_session( + TimedSession( + chore_id=chore.id, + child_id=child.id, + state=state, + segments=[{"start": _now().isoformat(), "end": None if state == "running" else _now().isoformat()}], + session_date=_now().date().isoformat(), + ) + ) + + +def _sessions_for(storage, chore_id): + return [s for s in storage.get_timed_sessions() if s.chore_id == chore_id] + + +class TestDeleteClearsSessions: + def test_remove_chore_drops_its_running_and_paused_sessions(self): + coord, storage, kid, other = _system() + chore = _timed(coord) + _session(storage, chore, kid, "running") + _session(storage, chore, other, "paused") + with _at(_now()): + run(coord.async_remove_chore(chore.id)) + assert _sessions_for(storage, chore.id) == [] + + def test_other_chores_sessions_are_left_alone(self): + coord, storage, kid, _other = _system() + gone, kept = _timed(coord, "Reading"), _timed(coord, "Piano") + _session(storage, gone, kid) + _session(storage, kept, kid, "running") + with _at(_now()): + run(coord.async_remove_chore(gone.id)) + assert [s.chore_id for s in storage.get_timed_sessions()] == [kept.id] + + def test_delete_service_path_drops_sessions(self): + from custom_components.taskmate.chore_services import async_delete_chore_from_call + + coord, storage, kid, _other = _system() + chore = _timed(coord) + _session(storage, chore, kid) + with _at(_now()): + run(async_delete_chore_from_call(coord, {"chore_id": chore.id, "confirm": True})) + assert _sessions_for(storage, chore.id) == [] + + def test_bulk_delete_drops_sessions(self): + coord, storage, kid, _other = _system() + a, b = _timed(coord, "A"), _timed(coord, "B") + _session(storage, a, kid) + _session(storage, b, kid, "running") + with _at(_now()): + run(coord.async_bulk_chore_action("delete", [a.id, b.id])) + assert storage.get_timed_sessions() == [] + + def test_shopping_job_retire_path_drops_sessions(self): + """keep_deleted=False (chores TaskMate retires itself) clears them too.""" + coord, storage, kid, _other = _system() + chore = _timed(coord) + _session(storage, chore, kid) + with _at(_now()): + run(coord.async_remove_chore(chore.id, keep_deleted=False)) + assert _sessions_for(storage, chore.id) == [] + assert coord.recently_deleted() == [] + + +class TestRestore: + def test_restore_does_not_bring_a_timer_back(self): + coord, storage, kid, _other = _system() + chore = _timed(coord) + _session(storage, chore, kid, "running") + with _at(_now()): + run(coord.async_remove_chore(chore.id)) + run(coord.async_restore_deleted("chore", chore.id)) + assert storage.get_chore(chore.id) is not None + assert _sessions_for(storage, chore.id) == [] + + def test_restore_drops_an_orphan_timer_left_by_an_older_delete(self): + """Before the fix a delete left the session behind; a restore must not + pick it back up as a timer that has been "running" ever since.""" + coord, storage, kid, _other = _system() + chore = _timed(coord) + with _at(_now()): + run(coord.async_remove_chore(chore.id)) + _session(storage, chore, kid, "running") # the orphan an older delete left + with _at(_now()): + run(coord.async_restore_deleted("chore", chore.id)) + assert _sessions_for(storage, chore.id) == [] + + def test_restore_after_bulk_delete_does_not_double_completions(self): + coord, storage, kid, _other = _system() + chore = _chore(coord) + storage.add_completion( + ChoreCompletion(chore_id=chore.id, child_id=kid.id, completed_at=_now(), approved=True, points_awarded=5) + ) + with _at(_now()): + run(coord.async_bulk_chore_action("delete", [chore.id])) + assert [c for c in storage.get_completions() if c.chore_id == chore.id] == [] + with _at(_now()): + run(coord.async_restore_deleted("chore", chore.id)) + assert len([c for c in storage.get_completions() if c.chore_id == chore.id]) == 1 + + def test_bulk_delete_cleans_what_a_single_delete_cleans(self): + coord, storage, kid, _other = _system() + first, chore = _chore(coord, "Make bed"), _chore(coord) + storage.set_last_completed(chore.id, kid.id, _now().isoformat()) + storage.data["task_groups"] = [ + {"id": "g1", "name": "Morning", "policy": "sticky", "chore_ids": [first.id, chore.id]} + ] + with _at(_now()): + run(coord.async_set_chore_order(kid.id, [chore.id, first.id])) + run(coord.async_bulk_chore_action("delete", [chore.id])) + assert not storage.get_last_completed(chore.id, kid.id) + assert storage.get_task_groups()[0].chore_ids == [first.id] + assert storage.get_child(kid.id).chore_order == [first.id] + assert [e["id"] for e in coord.recently_deleted()] == [chore.id] diff --git a/wiki/Recently-Deleted.md b/wiki/Recently-Deleted.md index 0bf09b49..e4eddb8b 100644 --- a/wiki/Recently-Deleted.md +++ b/wiki/Recently-Deleted.md @@ -18,7 +18,7 @@ Restore puts the item back under its **old id**, so its Home Assistant entities **Savings on a reward.** Deleting a reward refunds what the children had saved towards it. Restoring moves those points back into the jar, but only as far as each child still has them: a child who spent part of the refund gets back what they can cover. -**Not brought back:** a chore's open swap requests and teamwork joins, and its auctions (live auctions were called off and the bidders told when the chore went). Records for a child who has been deleted since are left out. +**Not brought back:** a chore's open swap requests and teamwork joins, its auctions (live auctions were called off and the bidders told when the chore went), and any timer a child had running or paused on a timed chore (it stops when the chore is deleted; seconds not yet stopped aren't credited). Records for a child who has been deleted since are left out. Restore refuses if something already uses the same id. @@ -26,7 +26,7 @@ Restore refuses if something already uses the same id. Once a minute TaskMate forgets anything deleted more than 7 days ago. That is when a deleted chore's or reward's picture is removed from disk too (unless another chore or reward still shows it). -Chores removed with the Chores page's **bulk delete** are kept here too. +Chores removed with the Chores page's **bulk delete** are kept here too, and are cleared exactly as a single delete clears them. ## Related