fix(options): edit apps one row per step in a multiline field, and report discovery

The live Apps page showed the whole map as ONE long line with nowhere to add a
row. Cause: the field was a bare `vol.Optional(...): str`, and HA renders a
selector-less string as a SINGLE-LINE input (pre-fix wire field:
{"type": "string", "name": "app_name_map"} — no selector key). It is a
multiline TextSelector now: the real-HA test pins
selector.text.multiline on the wire and an N-line save/reopen round trip.

HA has no table selector, so the Apps sub-screen is a menu of row actions
(whole list as text / add / edit / remove) instead of one form:

- the EDIT row lists every app the user can act on: the rows they saved AND the
  apps discovery found, marked `[detected]`, each labelled "name (package id)"
  or "package id (no name yet)" — the user's Shield exposes no device labels at
  all, so the unnamed case is the common one and it must not read as its own id
  twice, and picking it is how a name gets set (saving stores the row)
- add: package + name + optional icon, refused by the SAME `_valid_app_row`
  rule the text field uses, with an error on the page
- edit: PREFILLED from the stored row (suggested values — a voluptuous
  `default=` never reaches the browser, so the form would have opened blank);
  the device's mdi: icon is never prefilled into the icon column
- remove: pick a SAVED row, then an explicit confirmation menu; cancelling
  keeps it (a detected app has no line to delete)
- edit/remove rows are offered only while something is stored
- every mutation writes ONE row through `_write_app_row`/`_drop_app_row`, so
  untouched rows and their icons stay byte-for-byte as they were

The Discover row was a silent no-op ("seems to load then do nothing"): it now
returns a one-line verdict the menu shows — N apps found / the Shield reported
none / the sweep could not reach the Shield / ADB is not set up — while still
never raising and never caching a failed sweep as an answer.

Also fixes `async_step_menu_again`, which the real FlowManager dispatches with
user_input and which raised TypeError for every menu offering that row (only
reachable through a stub before, hence unnoticed).

Docs: README and BOTH strings files (byte-identical) describe the editor, the
one-per-line format, the detected-app dropdown and the four Discover verdicts.
This commit is contained in:
James Mackay committed 2026-10-12 10:02:09 +01:00
1 parent eb62011ded
commit 46738063aa
8 files changed
+1369 -81

No files matched your search

+339 -25
View File
@@ -36,6 +36,7 @@ from nsh.const import (
CONF_ADB_ENABLED,
CONF_ADB_PORT,
CONF_ADB_STORE_APP_INFO,
CONF_APP_ICON_MAP,
CONF_APP_NAME_MAP,
CONF_CHANNEL_DIGIT_DELAY,
CONF_COMMAND_SCRIPTS,
@@ -1526,15 +1527,27 @@ class TestCopyContract:
flow.config_entry = SimpleNamespace(options={}, entry_id="e1", title="Shield")
flow.hass = FakeHass(str(tmp_path))
async def walk(step: str) -> dict:
"""The rendered screens from ``step`` down, keyed by the i18n step id."""
opened = await getattr(flow, f"async_step_{step}")(None)
if opened["type"] == "menu":
nested: dict = {step: ("menu", opened)}
for row in opened["menu_options"]:
if row in OPTIONS_ACTION_ROWS:
continue
nested.update(await walk(row))
return nested
return {step: ("form", opened)}
menu = await flow.async_step_init(None)
# the rendered sub-screens, keyed by the i18n step id the menu row maps to
rendered = {
step: vol_mirror.declared_fields(
(await getattr(flow, f"async_step_{step}")(None))["data_schema"]
)
for step in menu["menu_options"]
if step not in ("finish", "discover")
}
# t64: the Apps sub-screen is itself a MENU of row actions, so the walk
# goes as deep as the screen does — a nested step is checked exactly like
# a top-level one, and every row a nested menu offers must have a label.
rendered: dict = {}
for step in menu["menu_options"]:
if step in OPTIONS_ACTION_ROWS:
continue
rendered.update(await walk(step))
for file in (_strings(), _en_strings()):
menu_step = file["options"]["step"]["init"]
@@ -1546,12 +1559,18 @@ class TestCopyContract:
assert option in menu_step.get("menu_options", {}), f"menu row {option}"
steps = file["options"]["step"]
for step, fields in rendered.items():
for step, (kind, opened) in rendered.items():
# a sub-screen's i18n step key is its menu row id (HA dispatches
# a submitted form by its own step_id, so the two must match)
assert steps[step]["title"], step
assert steps[step]["description"], step
for field in fields:
if kind == "menu":
for row in opened["menu_options"]:
assert row in steps[step].get("menu_options", {}), (
f"{step} row {row} has no label"
)
continue
for field in vol_mirror.declared_fields(opened["data_schema"]):
assert field in steps[step]["data"], f"{step}.{field} has no label"
@@ -1586,7 +1605,9 @@ OPTIONS_SUBSTEP_FIELDS = {
CONF_ADB_COMMAND_ENABLED,
},
"receiver": {"receiver_entity", "receiver_input"},
"apps": {"app_name_map", "app_icon_map"},
# t64: the apps table lives in its OWN step now (the Apps sub-screen is a menu
# of row actions), so the field that owns the map is `apps_text`.
"apps_text": {"app_name_map", "app_icon_map"},
"scripts": {
CONF_REMOTE_COMMAND_SCRIPT,
CONF_CHANNEL_DIGIT_DELAY,
@@ -1595,6 +1616,24 @@ OPTIONS_SUBSTEP_FIELDS = {
},
}
#: t64: the Apps editor's rows live one level deeper — the Apps sub-screen is a
#: MENU of row actions, exactly as the browser navigates it: open the parent row,
#: then the row the user picked.
OPTIONS_NESTED_STEPS = {
"apps_text": "apps",
"apps_add": "apps",
"apps_edit": "apps",
"apps_edit_form": "apps",
"apps_remove": "apps",
"apps_remove_confirm": "apps",
}
#: Menu rows that ACT rather than render a form: HA dispatches them with no input,
#: so there is no data_schema and no field labels to check.
OPTIONS_ACTION_ROWS = frozenset(
{"discover", "finish", "menu_again", "apps_remove_yes", "apps_remove_no"}
)
def _options_substep_of(key: str) -> str:
for step, fields in OPTIONS_SUBSTEP_FIELDS.items():
@@ -1619,11 +1658,20 @@ async def _options_submit(flow, payload: dict, *, save=False):
grouped.setdefault(_options_substep_of(key), {})[key] = value
result = await flow.async_step_init(None)
for step, sub_payload in grouped.items():
parent = OPTIONS_NESTED_STEPS.get(step)
if parent is not None:
assert parent in result["menu_options"], (
f"{parent} is not offered by the menu: {result['menu_options']}"
)
result = await getattr(flow, f"async_step_{parent}")(None)
assert step in result["menu_options"], (
f"{step} is not offered by the menu: {result['menu_options']}"
)
result = await getattr(flow, f"async_step_{step}")(sub_payload)
if save:
# t64: a nested row (the apps table) returns to the APPS menu, so walk
# back the way the "Back to the main menu" row does before committing.
result = await flow.async_step_menu_again()
assert "finish" in result["menu_options"], result["menu_options"]
result = await flow.async_step_finish(None)
return result
@@ -1642,6 +1690,10 @@ def _all_script_fields(value):
async def _options_open(flow, step: str):
"""Choose a sub-screen from the menu and open its form."""
result = await flow.async_step_init(None)
parent = OPTIONS_NESTED_STEPS.get(step)
if parent is not None:
assert parent in result["menu_options"], result["menu_options"]
result = await getattr(flow, f"async_step_{parent}")(None)
assert step in result["menu_options"], result["menu_options"]
return await getattr(flow, f"async_step_{step}")(None)
@@ -2036,16 +2088,22 @@ class TestOptionsMenu:
assert hasattr(flow, f"async_step_{row}"), row
if row in action_rows:
continue
form = await getattr(flow, f"async_step_{row}")(None)
assert form["type"] == "form"
assert form["step_id"] == row, form["step_id"]
opened = await getattr(flow, f"async_step_{row}")(None)
# a sub-screen is a FORM — or, for Apps (t64), a MENU of row actions
assert opened["type"] in ("form", "menu"), (row, opened)
assert opened["step_id"] == row, opened["step_id"]
if opened["type"] == "menu":
# a nested menu's rows are dispatched by name as well
for nested in opened["menu_options"]:
assert hasattr(flow, f"async_step_{nested}"), (row, nested)
async def test_every_sub_screen_is_reachable_and_returns_to_the_menu(self, tmp_path):
flow = _options_flow(tmp_path)
for row in ("connection", "receiver", "apps", "scripts"): # sub-screens
opened = await _options_open(flow, row)
assert opened["type"] == "form"
# a sub-screen is a form, except Apps: a MENU of row actions (t64)
assert opened["type"] in ("form", "menu"), (row, opened)
# a submission that changes nothing still returns to the menu
back = await getattr(flow, f"async_step_{row}")({})
assert back["type"] == "menu", (row, back)
@@ -2090,7 +2148,7 @@ class TestAppsTable:
{"app_name_map": {"ar.tvplayer.tv": "TiviMate", "com.plex.hdx": "Plex"}},
)
form = await _options_open(flow, "apps")
form = await _options_open(flow, "apps_text")
suggested = form["data_schema"]["__suggested__"]
assert isinstance(suggested["app_name_map"], str)
@@ -2111,7 +2169,7 @@ class TestAppsTable:
# re-open: the row (icon included) comes back in the text field
reopened = _options_flow(tmp_path, result["data"])
form = await _options_open(reopened, "apps")
form = await _options_open(reopened, "apps_text")
assert (
form["data_schema"]["__suggested__"]["app_name_map"]
== "com.plex.hdx=Plex|http://icons.local/plex.png"
@@ -2140,10 +2198,10 @@ class TestAppsTable:
async def test_the_empty_state_renders_without_data_or_a_crash(self, tmp_path):
flow = _options_flow(tmp_path, options={})
form = await _options_open(flow, "apps")
form = await _options_open(flow, "apps_text")
assert form["type"] == "form"
assert form["step_id"] == "apps"
assert form["step_id"] == "apps_text"
assert form["errors"] == {}
assert list(vol_mirror.declared_fields(form["data_schema"])) == ["app_name_map"]
# no discovery data yet (there is no manager in this harness), so the
@@ -2195,7 +2253,7 @@ class TestDiscoveryFeedsTheAppsTable:
],
)
form = await _options_open(flow, "apps")
form = await _options_open(flow, "apps_text")
text = form["data_schema"]["__suggested__"]["app_name_map"]
lines = text.split("\n")
@@ -2216,7 +2274,7 @@ class TestDiscoveryFeedsTheAppsTable:
[{"package": "com.netflix.ninja", "name": "Netflix", "icon": "mdi:netflix"}],
)
form = await _options_open(flow, "apps")
form = await _options_open(flow, "apps_text")
assert form["data_schema"]["__suggested__"]["app_name_map"] == (
"com.netflix.ninja=Netflix"
@@ -2227,7 +2285,7 @@ class TestDiscoveryFeedsTheAppsTable:
tmp_path, {"app_name_map": {"com.a": "A"}}, [], connected=False
)
form = await _options_open(flow, "apps")
form = await _options_open(flow, "apps_text")
assert form["data_schema"]["__suggested__"]["app_name_map"] == "com.a=A"
@@ -2235,7 +2293,7 @@ class TestDiscoveryFeedsTheAppsTable:
"""A form render must never sweep — discovery is an explicit action."""
flow, manager = self._flow_with(tmp_path, {}, [])
await _options_open(flow, "apps")
await _options_open(flow, "apps_text")
await flow.async_step_init(None)
assert manager.refresh_calls == []
@@ -2292,15 +2350,271 @@ class TestDiscoveryFeedsTheAppsTable:
{},
[{"package": "com.netflix.ninja", "name": "Netflix", "icon": "mdi:netflix"}],
)
form = await _options_open(flow, "apps")
form = await _options_open(flow, "apps_text")
submitted = form["data_schema"]["__suggested__"]["app_name_map"]
await flow.async_step_apps({CONF_APP_NAME_MAP: submitted})
await flow.async_step_apps_text({CONF_APP_NAME_MAP: submitted})
result = await flow.async_step_finish(None)
assert result["data"][CONF_APP_NAME_MAP] == {"com.netflix.ninja": "Netflix"}
#: t64: the apps ROW EDITOR's field names. They are UI only — a projection of one
#: ``package=name[|icon]`` row — so they are asserted against the flow module's own
#: constants (below) instead of being trusted to match.
APP_PACKAGE = "app_package"
APP_NAME = "app_name"
APP_ICON = "app_icon"
class TestAppsRowEditor:
"""t64: HA has no table selector, so a row is edited one step at a time."""
def _flow(self, tmp_path, options=None):
return _options_flow(tmp_path, options or {})
async def test_the_field_is_the_multiline_text_selector(self, tmp_path):
"""The mechanical cause of the live report, pinned without HA.
A selector-less ``str`` field renders as a SINGLE-LINE input, so the map
arrived flattened with nowhere to add a row. The field must carry the text
selector, and it must be the MULTILINE one — the flag the frontend reads
to render a textarea instead of a one-line box.
"""
flow = self._flow(tmp_path)
form = await _options_open(flow, "apps_text")
module = _form_module()
assert (
module.APP_FIELD_PACKAGE,
module.APP_FIELD_NAME,
module.APP_FIELD_ICON,
) == (APP_PACKAGE, APP_NAME, APP_ICON)
field = form["data_schema"][CONF_APP_NAME_MAP]
assert isinstance(field, entity_stublib.TextSelectorStub), field
assert field.config.multiline is True
async def test_the_dropdown_lists_detected_apps_too_and_labels_them(self, tmp_path):
"""t64: the entry point is the DETECTED list, named where a name is known.
The dropdown is what the user picks from, so it carries the saved rows AND
the apps discovery found; an app whose device label is missing is labelled
"(no name yet)" rather than with its own id twice, and the icon column is
never prefilled from the device icon (that is an ``mdi:`` name, which the
column does not accept).
"""
flow = _options_flow(tmp_path, {CONF_APP_NAME_MAP: {"com.a": "Alpha"}})
flow.config_entry.runtime_data = TestDiscoverSaysWhatHappened._Manager(
[
{"package": "org.xbmc.kodi", "name": "Kodi", "icon": "mdi:kodi"},
{"package": "com.unnamed.tv", "name": "com.unnamed.tv", "icon": ""},
{"package": "com.a", "name": "Wrong name", "icon": "mdi:x"},
]
)
placeholder = flow._apps_placeholders()["apps"]
assert placeholder.startswith("3 apps known — ")
assert "Kodi (org.xbmc.kodi) [detected]" in placeholder
assert "com.unnamed.tv (no name yet) [detected]" in placeholder
assert "Alpha (com.a)" in placeholder # the saved row keeps the user's name
choose = await flow.async_step_apps_edit(None)
assert choose["step_id"] == "apps_edit"
form = await flow.async_step_apps_edit({APP_PACKAGE: "com.unnamed.tv"})
suggested_values = form["data_schema"]["__suggested__"]
assert suggested_values[APP_NAME] == "" # no device label: nothing invented
assert suggested_values[APP_ICON] == "" # never the mdi: device icon
await flow.async_step_apps_edit_form(
{APP_NAME: "Unnamed TV", APP_ICON: "http://icons.local/u.png"}
)
assert flow._options_dict[CONF_APP_NAME_MAP] == {
"com.a": "Alpha",
"com.unnamed.tv": "Unnamed TV",
}
async def test_only_the_rows_that_can_act_are_offered(self, tmp_path):
empty = self._flow(tmp_path)
menu = await empty.async_step_apps(None)
assert list(menu["menu_options"]) == ["apps_text", "apps_add", "menu_again"]
assert empty._apps_placeholders()["apps"].startswith("no apps known yet")
flow = self._flow(tmp_path, {CONF_APP_NAME_MAP: {"com.a": "Alpha"}})
menu = await flow.async_step_apps(None)
assert list(menu["menu_options"]) == [
"apps_text",
"apps_add",
"apps_edit",
"apps_remove",
"menu_again",
]
assert flow._apps_placeholders()["apps"] == "1 app known — Alpha (com.a)"
async def test_add_writes_one_row_and_leaves_the_others_alone(self, tmp_path):
flow = self._flow(
tmp_path, {CONF_APP_NAME_MAP: {"com.a": "Alpha"}, CONF_HOST: "10.0.0.9"}
)
result = await flow.async_step_apps_add(
{
APP_PACKAGE: "com.b",
APP_NAME: "Beta",
APP_ICON: "http://icons.local/b.png",
}
)
assert result["type"] == "menu" # back at the editor, nothing committed yet
assert flow._options_dict[CONF_APP_NAME_MAP] == {
"com.a": "Alpha",
"com.b": "Beta",
}
assert flow._options_dict[CONF_APP_ICON_MAP] == {
"com.b": "http://icons.local/b.png"
}
assert flow._options_dict[CONF_HOST] == "10.0.0.9" # another screen's field
async def test_an_invalid_row_is_refused_and_stores_nothing(self, tmp_path):
flow = self._flow(tmp_path)
form = await flow.async_step_apps_add(
{APP_PACKAGE: "has spaces", APP_NAME: "Nope", APP_ICON: ""}
)
assert form["type"] == "form"
assert form["step_id"] == "apps_add"
assert form["errors"] == {"base": "invalid_app_row"}
assert CONF_APP_NAME_MAP not in flow._options_dict
async def test_edit_prefills_the_stored_row_and_changes_only_it(self, tmp_path):
flow = self._flow(
tmp_path,
{
CONF_APP_NAME_MAP: {"com.a": "Alpha", "com.b": "Beta"},
CONF_APP_ICON_MAP: {"com.b": "http://icons.local/b.png"},
},
)
choose = await flow.async_step_apps_edit(None)
assert choose["step_id"] == "apps_edit"
form = await flow.async_step_apps_edit({APP_PACKAGE: "com.b"})
suggested_values = form["data_schema"]["__suggested__"]
assert suggested_values[APP_NAME] == "Beta"
assert suggested_values[APP_ICON] == "http://icons.local/b.png"
result = await flow.async_step_apps_edit_form(
{APP_NAME: "Beta (living room)", APP_ICON: "http://icons.local/b2.png"}
)
assert result["type"] == "menu"
assert flow._options_dict[CONF_APP_NAME_MAP] == {
"com.a": "Alpha",
"com.b": "Beta (living room)",
}
assert flow._options_dict[CONF_APP_ICON_MAP] == {
"com.b": "http://icons.local/b2.png"
}
async def test_remove_needs_the_confirmation_row(self, tmp_path):
flow = self._flow(
tmp_path, {CONF_APP_NAME_MAP: {"com.a": "Alpha"}, CONF_APP_ICON_MAP: {}}
)
choose = await flow.async_step_apps_remove(None)
assert choose["step_id"] == "apps_remove"
confirm = await flow.async_step_apps_remove({APP_PACKAGE: "com.a"})
assert confirm["type"] == "menu"
assert confirm["step_id"] == "apps_remove_confirm"
assert "Alpha (com.a)" in confirm["description_placeholders"]["app"]
kept = await flow.async_step_apps_remove_no(None)
assert kept["type"] == "menu"
assert flow._options_dict[CONF_APP_NAME_MAP] == {"com.a": "Alpha"}
await flow.async_step_apps_remove({APP_PACKAGE: "com.a"})
dropped = await flow.async_step_apps_remove_yes(None)
assert dropped["type"] == "menu"
assert flow._options_dict[CONF_APP_NAME_MAP] == {}
assert flow._options_dict[CONF_APP_ICON_MAP] == {}
class TestDiscoverSaysWhatHappened:
"""t64: "Discover ... seems to load then do nothing" must be impossible."""
class _Manager:
def __init__(self, rows=None, connected=True, fail=False):
self.rows = list(rows or [])
self.connected = connected
self.fail = fail
self.refresh_calls: list[bool] = []
def installed_apps(self):
return [dict(row) for row in self.rows]
async def async_refresh_installed_apps(self, force=False):
self.refresh_calls.append(force)
if self.fail:
raise RuntimeError("adb went away")
return self.installed_apps()
def _flow_with(self, tmp_path, manager):
flow = _options_flow(tmp_path, {})
flow.config_entry.runtime_data = manager
return flow
async def test_no_manager_says_adb_is_not_set_up(self, tmp_path):
flow = _options_flow(tmp_path, {})
await flow.async_step_discover(None)
assert flow._menu_placeholders()["discovery"] == (
"ADB is not set up for this entry — nothing could be discovered."
)
async def test_finding_apps_reports_the_count_and_updates_the_table(
self, tmp_path
):
manager = self._Manager(
[{"package": "org.xbmc.kodi", "name": "Kodi", "icon": "mdi:kodi"}]
)
flow = self._flow_with(tmp_path, manager)
result = await flow.async_step_discover(None)
assert result["type"] == "menu"
assert manager.refresh_calls == [True]
note = flow._menu_placeholders()["discovery"]
assert note.startswith("1 apps found on the Shield") # the t57 wording
assert "updated" in note
form = await _options_open(flow, "apps_text")
assert "org.xbmc.kodi=Kodi" in form["data_schema"]["__suggested__"][
CONF_APP_NAME_MAP
]
async def test_finding_nothing_says_the_saved_rows_stay(self, tmp_path):
flow = self._flow_with(tmp_path, self._Manager([]))
await flow.async_step_discover(None)
note = flow._menu_placeholders()["discovery"]
assert "no installed apps" in note and "keeps your" in note
async def test_a_failed_sweep_says_so_instead_of_nothing(self, tmp_path):
flow = self._flow_with(tmp_path, self._Manager([], connected=False, fail=True))
result = await flow.async_step_discover(None)
assert result["type"] == "menu" # no traceback, no dead end
note = flow._menu_placeholders()["discovery"]
assert "could not reach" in note and "unchanged" in note
class TestReceiverAcceptsASwitch:
"""t56: a soundbar that only knows on/off is a valid receiver."""