laundry: fix four findings from the final review of guided setup
The one that could run the wrong program: guided setup accepted a name another program already had. The duplicate check only compared names within the form it was handed, and guided setup submits one program at a time, so it never saw the others. Two programs sharing a label resolve to whichever option comes first, so picking the second would have run the first one's payload -- the exact failure the check exists to prevent, working correctly in the bulk form and blind in the guided one. The taken set now includes every other named program, excluding the slot being edited so confirming an unchanged name doesn't reject itself. The rest: - The timeout screen's copy interpolates the same counters as the other two but was shown without placeholders, so it rendered literal braces. - The probe reports whatever payload is loaded, while observe() declines one whose slot the device doesn't advertise. Guided setup could reach the name form for such a slot, take a name, and silently discard it -- there is no record to hang it on and no payload to replay. It now waits instead. - The prefilled Download course came from the raw candidate list while the dropdown filters to courses the appliance still offers, so a stale candidate prefilled a value the selector rejects and the form failed validation on something the user never chose.
This commit is contained in:
@@ -1005,7 +1005,16 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
|
||||
deadline = time.monotonic() + _CLOUD_WAIT_TIMEOUT_S
|
||||
while time.monotonic() < deadline:
|
||||
slot = await coord.async_probe_cloud_courses()
|
||||
if slot is not None and slot != self._cloud_baseline:
|
||||
# Only a slot the store actually recorded. The probe reports
|
||||
# whatever payload is loaded, while observe() declines one whose
|
||||
# slot the device doesn't advertise -- offering to name that would
|
||||
# take a name and silently discard it, since there is no record to
|
||||
# hang it on and no payload to replay.
|
||||
if (
|
||||
slot is not None
|
||||
and slot != self._cloud_baseline
|
||||
and coord.cloud_courses.snapshot()["slots"].get(slot)
|
||||
):
|
||||
return slot
|
||||
await asyncio.sleep(_CLOUD_PROBE_INTERVAL_S)
|
||||
return None
|
||||
@@ -1104,11 +1113,17 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
|
||||
|
||||
@property
|
||||
def _cloud_course(self) -> str | None:
|
||||
"""The best observed candidate, narrowed to what the selector offers.
|
||||
|
||||
Unfiltered, a candidate the appliance's own course list no longer
|
||||
contains would prefill a dropdown that rejects it, and the form would
|
||||
fail validation on a value the user never chose.
|
||||
"""
|
||||
coord = self._coordinator()
|
||||
if coord is None:
|
||||
return None
|
||||
candidates = coord.cloud_courses.download_candidates()
|
||||
return candidates[0] if candidates else None
|
||||
available = cycle_options(coord.canonical_resources(MAIN))
|
||||
return next((c for c in coord.cloud_courses.download_candidates() if c in available), None)
|
||||
|
||||
def _cloud_course_selector(self, coord):
|
||||
"""The appliance's own course codes, observed candidates first.
|
||||
@@ -1134,9 +1149,13 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
|
||||
"""Nothing was selected in time. Offer another round rather than
|
||||
dropping the user out of the flow -- and an explicit finish, for
|
||||
anyone who doesn't think to close the dialog."""
|
||||
coord = self._coordinator()
|
||||
if coord is None:
|
||||
return self.async_abort(reason="not_loaded")
|
||||
return self.async_show_menu(
|
||||
step_id="cloud_timeout",
|
||||
menu_options=["cloud_guided", "cloud_finish"],
|
||||
description_placeholders=self._cloud_progress_placeholders(coord),
|
||||
)
|
||||
|
||||
async def async_step_cloud_finish(
|
||||
@@ -1204,7 +1223,18 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
|
||||
courses come first, so a shared label resolves to the real cycle).
|
||||
"""
|
||||
names = {slot: str(user_input.get(f"name_{slot}", "")).strip() for slot in known}
|
||||
stored_slots = coord.cloud_courses.snapshot()["slots"]
|
||||
taken = {name.casefold() for name in self._device_course_names(coord)}
|
||||
# Programs this form isn't editing. The bulk form edits every slot at
|
||||
# once so this adds nothing there, but guided setup submits one at a
|
||||
# time -- without it, naming two programs the same was accepted, and
|
||||
# the select resolves a shared label to whichever option comes first,
|
||||
# so picking the second would run the first one's payload.
|
||||
taken |= {
|
||||
record["name"].casefold()
|
||||
for slot, record in stored_slots.items()
|
||||
if record["name"] and slot not in known
|
||||
}
|
||||
for name in names.values():
|
||||
if not name:
|
||||
continue
|
||||
|
||||
@@ -709,3 +709,78 @@ async def test_guided_rejects_a_download_course_the_device_does_not_offer(hass:
|
||||
result = await handler.async_step_cloud_name({"name": "Sports", "download_course": "FF"})
|
||||
assert result["errors"] == {"base": "cloud_course_unknown_course"}
|
||||
assert coordinator.cloud_courses.snapshot()["download_course"] is None
|
||||
|
||||
|
||||
async def test_guided_rejects_a_name_another_program_already_has(hass: HomeAssistant):
|
||||
"""Guided setup submits one program at a time, so a within-form check
|
||||
can't see the others. Two programs sharing a label resolve to whichever
|
||||
option comes first, meaning picking the second would run the first one's
|
||||
payload."""
|
||||
coordinator = await _coordinator(hass)
|
||||
coordinator.apply_cloud_courses({"6B": "Sports"}, "87")
|
||||
await _flush(hass)
|
||||
|
||||
handler = await _options_handler(hass, coordinator)
|
||||
await handler.async_step_cloud_guided()
|
||||
_stub_probe(coordinator, ["55"])
|
||||
await _run_wait(handler)
|
||||
|
||||
result = await handler.async_step_cloud_name({"name": "Sports"})
|
||||
await _flush(hass)
|
||||
assert result["errors"] == {"base": "cloud_course_name_duplicate"}
|
||||
assert coordinator.cloud_courses.named() == {"6B": "Sports"}
|
||||
|
||||
|
||||
async def test_renaming_a_program_to_its_own_name_is_allowed(hass: HomeAssistant):
|
||||
"""The slot being edited is excluded from the taken set, or confirming an
|
||||
unchanged name would reject itself."""
|
||||
coordinator = await _coordinator(hass)
|
||||
coordinator.apply_cloud_courses({"55": "Sports"}, "87")
|
||||
await _flush(hass)
|
||||
|
||||
handler = await _options_handler(hass, coordinator)
|
||||
await handler.async_step_cloud_guided()
|
||||
_stub_probe(coordinator, ["55"])
|
||||
await _run_wait(handler)
|
||||
|
||||
result = await handler.async_step_cloud_name({"name": "Sports"})
|
||||
assert result["type"] == "progress" # accepted, straight back to waiting
|
||||
|
||||
|
||||
async def test_the_timeout_screen_fills_in_its_counters(hass: HomeAssistant):
|
||||
"""Its copy interpolates the same placeholders as the other screens, so
|
||||
without them the dialog renders literal braces."""
|
||||
coordinator = await _coordinator(hass)
|
||||
coordinator.apply_cloud_courses({"55": "Sports"}, "87")
|
||||
await _flush(hass)
|
||||
|
||||
handler = await _options_handler(hass, coordinator)
|
||||
result = await handler.async_step_cloud_timeout()
|
||||
assert result["description_placeholders"]["named"] == "1"
|
||||
assert result["description_placeholders"]["named_list"] == "Sports"
|
||||
|
||||
|
||||
async def test_guided_ignores_a_payload_the_store_would_not_record(hass: HomeAssistant):
|
||||
"""The probe reports whatever payload is loaded; observe() declines one
|
||||
whose slot the device doesn't advertise. Offering to name that would take
|
||||
a name and discard it -- there is no record to hang it on."""
|
||||
coordinator = await _coordinator(hass)
|
||||
handler = await _options_handler(hass, coordinator)
|
||||
await handler.async_step_cloud_guided()
|
||||
|
||||
# 'ZZ' is not in this appliance's CloudExtraCourse_ list.
|
||||
_stub_probe(coordinator, ["ZZ"])
|
||||
result = await _run_wait(handler)
|
||||
assert result["step_id"] == "cloud_timeout"
|
||||
|
||||
|
||||
async def test_the_prefilled_course_is_always_one_the_dropdown_offers(hass: HomeAssistant):
|
||||
"""An observed candidate the appliance's course list no longer contains
|
||||
would prefill a value the selector rejects, failing validation on
|
||||
something the user never chose."""
|
||||
coordinator = await _coordinator(hass)
|
||||
coordinator.cloud_courses._candidates["FF"] = 99 # not in the course list
|
||||
handler = await _options_handler(hass, coordinator)
|
||||
|
||||
offered = handler._cloud_course_selector(coordinator).config["options"]
|
||||
assert handler._cloud_course is None or handler._cloud_course in offered
|
||||
|
||||
Reference in New Issue
Block a user