Review fix: omitted picker submissions carry the allowlist forward
Review round 6 (Codex gpt-5.6-terra, MEDIUM): HA replaces entry.options wholesale, so a submission omitting the picker dropped the allowlist copy and left only the stale config-flow copy in entry data — a later database loss would resurrect removed admins. Omitted submissions now carry the current DB allowlist forward on the options copy. The legacy-path test updated to assert the carried copy. Suite: 910 passed.
This commit is contained in:
@@ -213,6 +213,13 @@ class NestQuestOptionsFlow(config_entries.OptionsFlowWithConfigEntry):
|
|||||||
}
|
}
|
||||||
if admin_ids_present:
|
if admin_ids_present:
|
||||||
data[CONF_ADMIN_USER_IDS] = admin_ids
|
data[CONF_ADMIN_USER_IDS] = admin_ids
|
||||||
|
elif admin_default:
|
||||||
|
# HA replaces entry.options wholesale, so an omitted picker
|
||||||
|
# field must carry the CURRENT allowlist forward: dropping
|
||||||
|
# it would leave only the stale config-flow copy in entry
|
||||||
|
# data, and a later database loss would resurrect admins
|
||||||
|
# the owner has since removed.
|
||||||
|
data[CONF_ADMIN_USER_IDS] = list(admin_default)
|
||||||
return self.async_create_entry(title="NestQuest", data=data)
|
return self.async_create_entry(title="NestQuest", data=data)
|
||||||
|
|
||||||
def _current_values(self) -> dict[str, Any]:
|
def _current_values(self) -> dict[str, Any]:
|
||||||
|
|||||||
@@ -554,8 +554,9 @@ def test_options_flow_unknown_selection_refused(tmp_path) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def test_options_flow_submit_without_admin_key_keeps_allowlist(tmp_path) -> None:
|
def test_options_flow_submit_without_admin_key_keeps_allowlist(tmp_path) -> None:
|
||||||
"""Raw callers omitting the key (pre-picker payloads) leave the
|
"""Raw callers omitting the key (pre-picker payloads) do not
|
||||||
allowlist untouched and produce the legacy three-key entry."""
|
touch the allowlist; the CURRENT allowlist is carried forward on
|
||||||
|
the options copy so no removed admin can be resurrected later."""
|
||||||
database = _open_allowlist_db(tmp_path, "picker-legacy.db")
|
database = _open_allowlist_db(tmp_path, "picker-legacy.db")
|
||||||
try:
|
try:
|
||||||
from custom_components.nestquest.admin_allowlist import (
|
from custom_components.nestquest.admin_allowlist import (
|
||||||
@@ -572,7 +573,7 @@ def test_options_flow_submit_without_admin_key_keeps_allowlist(tmp_path) -> None
|
|||||||
)
|
)
|
||||||
result = _run(flow.async_step_init(dict(VALID_INPUT)))
|
result = _run(flow.async_step_init(dict(VALID_INPUT)))
|
||||||
assert result["type"] == "create_entry"
|
assert result["type"] == "create_entry"
|
||||||
assert CONF_ADMIN_USER_IDS not in result["data"]
|
assert result["data"][CONF_ADMIN_USER_IDS] == ["u1"]
|
||||||
assert await_gather(list_admin_ids(database)) == ["u1"]
|
assert await_gather(list_admin_ids(database)) == ["u1"]
|
||||||
finally:
|
finally:
|
||||||
_run(database.close())
|
_run(database.close())
|
||||||
@@ -605,3 +606,32 @@ def test_options_flow_duplicate_selection_refused(tmp_path) -> None:
|
|||||||
assert await_gather(list_admin_ids(database)) == ["u1"]
|
assert await_gather(list_admin_ids(database)) == ["u1"]
|
||||||
finally:
|
finally:
|
||||||
_run(database.close())
|
_run(database.close())
|
||||||
|
|
||||||
|
|
||||||
|
def test_options_flow_omitted_picker_carries_allowlist_forward(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
"""An omitted picker field must not drop the allowlist copy from
|
||||||
|
the returned options: HA replaces entry.options wholesale, and a
|
||||||
|
later database loss must not resurrect removed admins from the
|
||||||
|
stale config-flow copy in entry.data."""
|
||||||
|
database = _open_allowlist_db(tmp_path, "picker-carry.db")
|
||||||
|
try:
|
||||||
|
from custom_components.nestquest.admin_allowlist import (
|
||||||
|
set_admin_ids,
|
||||||
|
)
|
||||||
|
|
||||||
|
_run(set_admin_ids(database, ["u2"]))
|
||||||
|
entry = _make_entry()
|
||||||
|
flow = _make_flow(
|
||||||
|
entry,
|
||||||
|
users=[_user("u1", "Joshua"), _user("u2", "Sam")],
|
||||||
|
database=database,
|
||||||
|
)
|
||||||
|
result = _run(flow.async_step_init(dict(VALID_INPUT)))
|
||||||
|
assert result["type"] == "create_entry"
|
||||||
|
# The current allowlist rides along on the legacy three-key path.
|
||||||
|
assert result["data"][CONF_ADMIN_USER_IDS] == ["u2"]
|
||||||
|
assert result["data"][CONF_HORIZON_DAYS] == VALID_INPUT[CONF_HORIZON_DAYS]
|
||||||
|
finally:
|
||||||
|
_run(database.close())
|
||||||
|
|||||||
Reference in New Issue
Block a user