From d995978df8b678515239752fb96c0017a0a8dbcb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 24 Mar 2026 18:40:20 +0000 Subject: [PATCH] Fix four bugs found during code audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. sensor.py: double dict lookup on child_lookup could produce unexpected results — replaced with a clean single lookup using `in` check. 2. sensor.py: float()/int() conversions on settings values had no error handling — a corrupted setting would crash the sensor and make the whole integration unavailable. Added _safe_float()/_safe_int() helpers with fallback defaults. 3. coordinator.py: is_chore_available_for_child() caught ValueError but not TypeError when slicing current_iso — if the stored value was not a string (e.g. None), [:10] raises TypeError which was unhandled and would crash the availability check. Now catches (ValueError, TypeError). 4. coordinator.py / storage.py: async_reject_reward() was directly writing to storage._data["reward_claims"] bypassing the storage API, making it fragile to internal storage changes. Added remove_reward_claim() to TaskMateStorage and updated the coordinator to use it. https://claude.ai/code/session_01CkPmKWe5siThpfZ6GDpNQF --- custom_components/taskmate/coordinator.py | 7 ++----- custom_components/taskmate/sensor.py | 22 +++++++++++++++++++--- custom_components/taskmate/storage.py | 6 ++++++ tests/test_coordinator_rewards.py | 6 ++++++ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/custom_components/taskmate/coordinator.py b/custom_components/taskmate/coordinator.py index cd3d557..9a89e26 100644 --- a/custom_components/taskmate/coordinator.py +++ b/custom_components/taskmate/coordinator.py @@ -390,7 +390,7 @@ def is_chore_available_for_child(self, chore, child_id: str) -> bool: try: last_dt = date.fromisoformat(current_iso[:10]) - except ValueError: + except (ValueError, TypeError): return True # every_2_days with anchor — check alignment @@ -676,10 +676,7 @@ async def async_approve_reward(self, claim_id: str) -> None: async def async_reject_reward(self, claim_id: str) -> None: """Reject a reward claim — no refund needed as points were never deducted.""" - self.storage._data["reward_claims"] = [ - c for c in self.storage._data.get("reward_claims", []) - if c.get("id") != claim_id - ] + self.storage.remove_reward_claim(claim_id) await self.storage.async_save() await self.async_refresh() diff --git a/custom_components/taskmate/sensor.py b/custom_components/taskmate/sensor.py index 4421a9a..a8c552e 100644 --- a/custom_components/taskmate/sensor.py +++ b/custom_components/taskmate/sensor.py @@ -23,6 +23,22 @@ _LOGGER = logging.getLogger(__name__) +def _safe_float(value, default: float) -> float: + """Convert value to float, returning default on failure.""" + try: + return float(value) if value is not None else default + except (ValueError, TypeError): + return default + + +def _safe_int(value, default: int) -> int: + """Convert value to int, returning default on failure.""" + try: + return int(value) if value is not None else default + except (ValueError, TypeError): + return default + + async def async_setup_entry( hass: HomeAssistant, entry: ConfigEntry, @@ -147,7 +163,7 @@ def extra_state_attributes(self) -> dict: "completion_id": comp.id, "chore_id": comp.chore_id, "child_id": comp.child_id, - "child_name": child_lookup.get(comp.child_id, None) and child_lookup[comp.child_id].name or "", + "child_name": child_lookup[comp.child_id].name if comp.child_id in child_lookup else "", "chore_name": matched_chore.name if matched_chore else "", "points": matched_chore.points if matched_chore else 0, "approved": comp.approved, @@ -231,11 +247,11 @@ def extra_state_attributes(self) -> dict: return { "today_day_of_week": today_dow, "streak_reset_mode": data.get("settings", {}).get("streak_reset_mode", "reset"), - "weekend_multiplier": float(data.get("settings", {}).get("weekend_multiplier", "2.0") or "2.0"), + "weekend_multiplier": _safe_float(data.get("settings", {}).get("weekend_multiplier"), 2.0), "streak_milestones_enabled": data.get("settings", {}).get("streak_milestones_enabled", "true") == "true", "streak_milestones": data.get("settings", {}).get("streak_milestones", "3:5, 7:10, 14:20, 30:50, 60:100, 100:200"), "perfect_week_enabled": data.get("settings", {}).get("perfect_week_enabled", "true") == "true", - "perfect_week_bonus": int(data.get("settings", {}).get("perfect_week_bonus", "50") or "50"), + "perfect_week_bonus": _safe_int(data.get("settings", {}).get("perfect_week_bonus"), 50), "total_children": len(children), "total_chores": len(chores), "total_rewards": len(rewards), diff --git a/custom_components/taskmate/storage.py b/custom_components/taskmate/storage.py index f16872c..ab31031 100644 --- a/custom_components/taskmate/storage.py +++ b/custom_components/taskmate/storage.py @@ -277,6 +277,12 @@ def update_reward_claim(self, claim: RewardClaim) -> None: claims[i] = claim.to_dict() return + def remove_reward_claim(self, claim_id: str) -> None: + """Remove a reward claim.""" + self._data["reward_claims"] = [ + c for c in self._data.get("reward_claims", []) if c.get("id") != claim_id + ] + # Points transactions management def get_points_transactions(self) -> list[PointsTransaction]: """Get all points transactions.""" diff --git a/tests/test_coordinator_rewards.py b/tests/test_coordinator_rewards.py index 773b38c..2671d0f 100644 --- a/tests/test_coordinator_rewards.py +++ b/tests/test_coordinator_rewards.py @@ -43,6 +43,12 @@ def _make_coord(*, children=None, rewards=None, claims=None): storage.async_save = AsyncMock() storage._data = {"reward_claims": [c.to_dict() for c in _claims]} + def _remove_reward_claim(claim_id): + storage._data["reward_claims"] = [ + c for c in storage._data["reward_claims"] if c.get("id") != claim_id + ] + storage.remove_reward_claim = MagicMock(side_effect=_remove_reward_claim) + coord.storage = storage coord.async_refresh = AsyncMock() return coord