fix: accept weekday names in chore applicable_days - #258
Merged
Conversation
The applicable_days service field is a select whose options are weekday
names ("mon", "wed"), but create_chore stored those strings unchanged and
update_chore later cast them with int(), so any update to such a chore
raised "invalid literal for int() with base 10: 'wed'".
Storing the raw strings was also silently wrong on its own: consumers
compare against weekday integers (calendar.py does `current.weekday() in
applicable_days`), so a chore created this way never matched a day.
Every other path already converts or tolerates names - flow_helpers and
options_flow map through WEEKDAY_NAME_TO_INT, chore_engine lowercases and
looks up, and the pre-v50 migration rewrites stored names to integers.
Only the service path was missing the conversion.
Normalize at the service boundary so stored data is canonical integers,
and coerce on read in _ensure_per_assignee_due_dates so chores already
stored with names by an earlier version stop failing rather than needing
a migration.
Also derive _DAY_OF_WEEK_VALUES from WEEKDAY_NAME_TO_INT instead of
repeating the day literals, so the values the schema accepts and the
values the conversion understands cannot drift apart.
Closes ccpk1#257
Owner
|
Thank you for submitting the issue and the fix it was well built. There is a minor update to the shared read path I mentioned in the issue comments, but I'm going to go ahead and handle that one. Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
applicable_daysis a select whose options are weekday names ("mon","wed"), butcreate_chorestored those strings unchanged andupdate_chorelater cast them withint(). Any update to such a chore raised:Storing the names was silently wrong on its own, independent of the crash: consumers compare against weekday integers —
calendar.pydoesif current.weekday() in applicable_days:— so a chore created through the service never matched a day.Every other path in the codebase already converts or tolerates names:
helpers/flow_helpers.py:1558WEEKDAY_NAME_TO_INToptions_flow.py:1179,:1517WEEKDAY_NAME_TO_INTengines/chore_engine.py:1463WEEKDAY_NAME_TO_INT.get(d.lower())engines/schedule_engine.py:1209strand mapsmigrations/pre_v50.py:2547services.pyint(d)So this is the one place that never got the conversion.
The change:
create_choreandupdate_chore) so stored data is canonical integers, matching what the config flow already writes._ensure_per_assignee_due_dates, so chores already stored with names by an earlier version stop failing without needing a migration._DAY_OF_WEEK_VALUESfromWEEKDAY_NAME_TO_INTrather than repeating the day literals, so the values the schema accepts and the values the conversion understands cannot drift apart. (This replaces the# using raw values since there are no individual DAY_* constantscomment — the mapping constant already existed.)Unrecognized or out-of-range values are dropped with a warning rather than raising, consistent with how
flow_helpersandchore_enginealready filter.I did not touch
flow_helpersoroptions_flow, which already convert correctly — happy to consolidate the four conversion sites into the shared helper as a follow-up refactor if you'd prefer that.Linked issue
Main merge automation checks
Closes #...).github/release.ymlcategorization — I don't have permission to set labels on this repo. Suggested:bug+area: automation/services.Change type
Scope
Dashboard source boundary
custom_components/choreops/dashboards/ccpk1/ChoreOps-DashboardsValidation
./utils/quick_lint.sh --fix— ruff check,ruff format,mypy --config-file mypy_quick.ini, andutils/check_boundaries.pyall cleanpython -m pytest tests/ -v --tb=lineTests were run on Linux CI rather than locally: the suite cannot run on Windows, because
homeassistant/runner.pyimportsfcntlunconditionally andpytest-homeassistant-custom-componentimports that runner. Results onubuntu-latest, Python 3.13:The 7 added tests are the new ones below. The 1 error is pre-existing on an unmodified
mainand unrelated to this change —tests/test_workflow_gamification_pending_queue.py::test_achievement_selected_unassigned_chore_not_awardedfails in teardown withValueError: Assignee non-existent-assignee does not exist (may have been deleted). Happy to open a separate issue for it.Tests added
Added
TestApplicableDaysCoerciontotests/test_chore_crud_services.py, followingtests/SERVICE_TESTING_PATTERNS.md(calls go throughhass.services.async_callso the real schema validates):test_create_stores_weekday_names_as_integers— day names are stored as[0, 2]test_update_after_create_with_weekday_names— the create_chore accepts applicable_days as day-name strings, but the code casts them with int() — update_chore then 500s #257 reprotest_update_tolerates_legacy_string_days_in_storage— a chore already holding["wed", "fri"]still updates. Uses an INDEPENDENT chore deliberately:_ensure_per_assignee_due_datesis only reached for those, so a shared chore here passes without the fix and pins nothing.test_update_rewrites_stored_days_to_integers—update_chorealso normalizestest_coerce_accepts_names_integers_and_mixturestest_coerce_drops_unusable_valuestest_schema_day_values_match_the_coercion_mapping— pins the constant the schema and conversion now shareI also ran these seven against unmodified
mainto confirm they actually catch the bug rather than passing vacuously: all 7 fail there, two of them with the reportedValueError: invalid literal for int() with base 10: 'wed'.Documentation impact
services.yamlalready documents the field as names (example: "['mon', 'wed', 'fri']"), which is now the behavior.Release notes
No release notes needed
Release notes needed (summarize user-visible changes below)
Release note summary: Fixed
create_chore/update_chorefailing withinvalid literal for int() with base 10whenapplicable_dayswas set. Weekday names from the selector are now converted to weekday integers, and chores already saved with day names are repaired on the next update.Breaking changes
Both conventions are accepted, so existing automations and stored chores keep working either way.