Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 54 additions & 3 deletions custom_components/choreops/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Includes UI editor support with selectors for dropdowns and text inputs.
"""

from collections.abc import Iterable
from copy import deepcopy
from datetime import datetime
from typing import TYPE_CHECKING, Any, cast
Expand Down Expand Up @@ -807,8 +808,9 @@ def _build_service_chore_validation_data(
const.OVERDUE_HANDLING_AT_DUE_DATE_ALLOW_STEAL,
]

# Days of week - using raw values since there are no individual DAY_* constants
_DAY_OF_WEEK_VALUES = ["mon", "tue", "wed", "thu", "fri", "sat", "sun"]
# Days of week - keyed off the same mapping used to coerce them to integers,
# so the accepted values and the conversion cannot drift apart.
_DAY_OF_WEEK_VALUES = list(const.WEEKDAY_NAME_TO_INT)

CREATE_CHORE_SCHEMA = vol.Schema(
_with_service_target_fields(
Expand Down Expand Up @@ -1131,6 +1133,49 @@ def _map_service_to_data_keys(
}


def _coerce_applicable_days(raw_days: Iterable[Any] | None) -> list[int]:
"""Normalize applicable days to weekday integers (0=Mon...6=Sun).

The service selector offers weekday name strings ("mon", "wed"), while
storage and every consumer expect integers. Accept either form so that
chores already stored with strings by an earlier version keep working.

Args:
raw_days: Weekday names, weekday integers, or a mix of both.

Returns:
Weekday integers, with unrecognized values dropped.
"""
if not raw_days:
return []

days: list[int] = []
for day in raw_days:
if isinstance(day, bool):
# bool is an int subclass; a boolean here is always a caller error.
continue
if isinstance(day, int):
if 0 <= day <= 6:
days.append(day)
continue
const.LOGGER.warning("Ignoring out-of-range applicable day: %s", day)
continue
mapped = const.WEEKDAY_NAME_TO_INT.get(str(day).strip().lower())
if mapped is not None:
days.append(mapped)
continue
const.LOGGER.warning("Ignoring unrecognized applicable day: %s", day)
return days


def _normalize_chore_applicable_days(data_input: dict[str, Any]) -> None:
"""Coerce applicable days in mapped chore data to integers, in place."""
if const.DATA_CHORE_APPLICABLE_DAYS in data_input:
data_input[const.DATA_CHORE_APPLICABLE_DAYS] = _coerce_applicable_days(
data_input[const.DATA_CHORE_APPLICABLE_DAYS]
)


async def _sync_chore_select_selection(
hass: HomeAssistant,
coordinator: "ChoreOpsDataCoordinator",
Expand Down Expand Up @@ -1279,7 +1324,9 @@ def _ensure_per_assignee_due_dates(
const.DATA_CHORE_APPLICABLE_DAYS,
const.DEFAULT_APPLICABLE_DAYS,
)
applicable_days: list[int] | None = [int(d) for d in raw_days] if raw_days else None
# Chores stored by an earlier version may hold weekday name strings here,
# so coerce rather than casting directly.
applicable_days: list[int] | None = _coerce_applicable_days(raw_days) or None

for uid in assigned_assignee_ids:
# 1. User-provided explicit due date wins
Expand Down Expand Up @@ -1418,6 +1465,8 @@ async def handle_create_chore(call: ServiceCall) -> dict[str, Any]:
data_input = _map_service_to_data_keys(
dict(call.data), _SERVICE_TO_CHORE_DATA_MAPPING
)
# The selector supplies weekday names; storage expects integers.
_normalize_chore_applicable_days(data_input)
# Override assigned assignees with resolved UUIDs
data_input[const.DATA_CHORE_ASSIGNED_USER_IDS] = assignee_ids

Expand Down Expand Up @@ -1602,6 +1651,8 @@ async def handle_update_chore(call: ServiceCall) -> dict[str, Any]:
data_input = _map_service_to_data_keys(
service_data, _SERVICE_TO_CHORE_DATA_MAPPING
)
# The selector supplies weekday names; storage expects integers.
_normalize_chore_applicable_days(data_input)

# Apply assignment_action merge logic when updating assignees.
# "add" / "remove" merge with existing list; "replace" (default)
Expand Down
161 changes: 161 additions & 0 deletions tests/test_chore_crud_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
import voluptuous as vol

from custom_components.choreops import const
from custom_components.choreops.services import (
_DAY_OF_WEEK_VALUES,
_coerce_applicable_days,
)
from tests.helpers import (
DOMAIN,
SERVICE_CREATE_CHORE,
Expand Down Expand Up @@ -1241,3 +1245,160 @@ async def test_replace_is_default_behavior(
)

assert _get_assigned_names(scenario_full.coordinator, chore_id) == ["Lila"]


# ============================================================================
# APPLICABLE DAYS COERCION (regression: issue #257)
# ============================================================================


class TestApplicableDaysCoercion:
"""Weekday names from the service selector must become weekday integers.

``services.yaml`` offers ``applicable_days`` as names ("mon", "wed"), while
storage and every consumer expect integers (0=Mon...6=Sun). Storing the raw
strings made any later ``update_chore`` raise
``invalid literal for int() with base 10: 'wed'``.
"""

@pytest.mark.asyncio
async def test_create_stores_weekday_names_as_integers(
self,
hass: HomeAssistant,
scenario_full: SetupResult,
) -> None:
"""create_chore converts selector day names to integers before storing."""
with patch.object(scenario_full.coordinator, "_persist", new=MagicMock()):
response = await hass.services.async_call(
DOMAIN,
SERVICE_CREATE_CHORE,
{
"name": "Applicable Days Chore",
"assigned_user_names": ["Zoë"],
"frequency": "weekly",
"applicable_days": ["mon", "wed"],
"due_date": "2099-01-06T18:00:00",
},
blocking=True,
return_response=True,
)

assert response is not None
chore_id = response["id"]
stored = scenario_full.coordinator.chores_data[chore_id]
assert stored[const.DATA_CHORE_APPLICABLE_DAYS] == [0, 2]

@pytest.mark.asyncio
async def test_update_after_create_with_weekday_names(
self,
hass: HomeAssistant,
scenario_full: SetupResult,
) -> None:
"""Reproduces #257: a chore created with day names could not be updated."""
with patch.object(scenario_full.coordinator, "_persist", new=MagicMock()):
response = await hass.services.async_call(
DOMAIN,
SERVICE_CREATE_CHORE,
{
"name": "Trash Day",
"assigned_user_names": ["Zoë"],
"frequency": "weekly",
"applicable_days": ["wed"],
"due_date": "2099-01-06T18:00:00",
},
blocking=True,
return_response=True,
)
assert response is not None
chore_id = response["id"]

# Before the fix this raised ValueError from int("wed").
await hass.services.async_call(
DOMAIN,
SERVICE_UPDATE_CHORE,
{
"id": chore_id,
"assignment_action": "replace",
"assigned_user_names": ["Max!"],
},
blocking=True,
)

assert _get_assigned_names(scenario_full.coordinator, chore_id) == ["Max!"]

@pytest.mark.asyncio
async def test_update_tolerates_legacy_string_days_in_storage(
self,
hass: HomeAssistant,
scenario_full: SetupResult,
) -> None:
"""Chores already stored with day names by an earlier version still update.

Uses an INDEPENDENT chore so the update path reaches
``_ensure_per_assignee_due_dates``, which is where stored days are read
back. A shared chore never calls it, so it would not exercise the fix.
"""
chore_id = scenario_full.chore_ids["Pick up Lëgo!"]
coordinator = scenario_full.coordinator
coordinator.chores_data[chore_id][const.DATA_CHORE_APPLICABLE_DAYS] = [
"wed",
"fri",
]

with patch.object(coordinator, "_persist", new=MagicMock()):
await hass.services.async_call(
DOMAIN,
SERVICE_UPDATE_CHORE,
{
"id": chore_id,
"assignment_action": "replace",
"assigned_user_names": ["Lila"],
},
blocking=True,
)

assert _get_assigned_names(coordinator, chore_id) == ["Lila"]

@pytest.mark.asyncio
async def test_update_rewrites_stored_days_to_integers(
self,
hass: HomeAssistant,
scenario_full: SetupResult,
) -> None:
"""Passing day names to update_chore stores integers, not strings."""
chore_id = scenario_full.chore_ids["Täke Öut Trash"]

with patch.object(scenario_full.coordinator, "_persist", new=MagicMock()):
await hass.services.async_call(
DOMAIN,
SERVICE_UPDATE_CHORE,
{
"id": chore_id,
"applicable_days": ["sat", "sun"],
},
blocking=True,
)

stored = scenario_full.coordinator.chores_data[chore_id]
assert stored[const.DATA_CHORE_APPLICABLE_DAYS] == [5, 6]

def test_coerce_accepts_names_integers_and_mixtures(self) -> None:
"""The coercion helper accepts either convention, or both together."""
assert _coerce_applicable_days(["mon", "wed"]) == [0, 2]
assert _coerce_applicable_days([0, 2]) == [0, 2]
assert _coerce_applicable_days(["MON", " wed "]) == [0, 2]
assert _coerce_applicable_days([0, "wed"]) == [0, 2]

def test_coerce_drops_unusable_values(self) -> None:
"""Unrecognized, out-of-range, and empty inputs are dropped, not raised."""
assert _coerce_applicable_days(None) == []
assert _coerce_applicable_days([]) == []
assert _coerce_applicable_days(["notaday"]) == []
assert _coerce_applicable_days([7, -1]) == []
assert _coerce_applicable_days([True]) == []
assert _coerce_applicable_days(["mon", "notaday"]) == [0]

def test_schema_day_values_match_the_coercion_mapping(self) -> None:
"""Every value the schema accepts must be convertible."""
assert set(_DAY_OF_WEEK_VALUES) == set(const.WEEKDAY_NAME_TO_INT)
assert _coerce_applicable_days(_DAY_OF_WEEK_VALUES) == [0, 1, 2, 3, 4, 5, 6]
Loading