From 913d3658b8b30e1c78c7a6caa073c8de6f593faf Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 17:30:29 -0400 Subject: [PATCH] Review fix: omitted picker submissions carry the allowlist forward MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- custom_components/nestquest/options_flow.py | 7 ++++ tests/test_options_flow.py | 36 +++++++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/custom_components/nestquest/options_flow.py b/custom_components/nestquest/options_flow.py index 9ab8214..84ac218 100644 --- a/custom_components/nestquest/options_flow.py +++ b/custom_components/nestquest/options_flow.py @@ -213,6 +213,13 @@ class NestQuestOptionsFlow(config_entries.OptionsFlowWithConfigEntry): } if admin_ids_present: 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) def _current_values(self) -> dict[str, Any]: diff --git a/tests/test_options_flow.py b/tests/test_options_flow.py index 0830b5c..bef04c8 100644 --- a/tests/test_options_flow.py +++ b/tests/test_options_flow.py @@ -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: - """Raw callers omitting the key (pre-picker payloads) leave the - allowlist untouched and produce the legacy three-key entry.""" + """Raw callers omitting the key (pre-picker payloads) do not + 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") try: 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))) 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"] finally: _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"] finally: _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())