diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index dbc26e9..688ae8a 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -242,10 +242,16 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: # record without a live connection (a previous setup's # close-on-failure, or a hand-replaced file) must get the # missing/valid/corrupt classification too, never a raw - # sqlite3 error and never skipped migrations. + # sqlite3 error and never skipped migrations — and the + # same allowlist seeding, so a reopened fresh file is + # never ownerless. db = await _async_open_database( hass, await async_get_db_path(hass) ) + stored_admin_ids = ( + entry.options.get(CONF_ADMIN_USER_IDS) + or entry.data.get(CONF_ADMIN_USER_IDS) + ) context = getattr(entry, "context", None) context_user_id = ( context.get("user_id") @@ -254,7 +260,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) await seed_setup_admin( db, - stored_admin_ids=entry.data.get(CONF_ADMIN_USER_IDS), + stored_admin_ids=stored_admin_ids, context_user_id=context_user_id, ) existing.database = db @@ -270,10 +276,15 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: database = await _async_open_database(hass, await async_get_db_path(hass)) try: # Seed the admin allowlist on first setup so the owner is never - # locked out: an empty database takes the config entry's stored - # admin copy (disaster recovery), else the HA user who - # completed the config flow. Non-empty allowlists are never - # touched (fail-closed narrowing is deliberate). + # locked out: an empty database takes the persisted admin copy + # — the options flow's saved list first (it is the current + # one), else the config flow's user from entry data — and only + # then the HA user still present on the entry context. No-op + # on a non-empty list (fail-closed narrowing is deliberate). + stored_admin_ids = ( + entry.options.get(CONF_ADMIN_USER_IDS) + or entry.data.get(CONF_ADMIN_USER_IDS) + ) context = getattr(entry, "context", None) context_user_id = ( context.get("user_id") @@ -282,7 +293,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) await seed_setup_admin( database, - stored_admin_ids=entry.data.get(CONF_ADMIN_USER_IDS), + stored_admin_ids=stored_admin_ids, context_user_id=context_user_id, ) remove_update_listener = entry.add_update_listener(_async_update_listener) diff --git a/custom_components/nestquest/config_flow.py b/custom_components/nestquest/config_flow.py index a8ccbc2..803b2f1 100644 --- a/custom_components/nestquest/config_flow.py +++ b/custom_components/nestquest/config_flow.py @@ -7,7 +7,7 @@ from homeassistant import config_entries from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult -from .const import DOMAIN +from .const import CONF_ADMIN_USER_IDS, DOMAIN class NestQuestConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -28,7 +28,14 @@ class NestQuestConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> FlowResult: - """Handle the user step of the config flow.""" + """Handle the user step of the config flow. + + The HA user completing the flow is persisted into the entry + data as the initial admin allowlist copy: real HA does not + guarantee the flow context survives to setup, and entry data is + the persisted record first setup seeds from — the owner is + never locked out. + """ await self.async_set_unique_id(DOMAIN, raise_on_progress=False) if self._async_in_progress() or self._async_current_entries(): return self.async_abort(reason="single_instance_allowed") @@ -36,4 +43,12 @@ class NestQuestConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if user_input is None: return self.async_show_form(step_id="user") - return self.async_create_entry(title="NestQuest", data=user_input) \ No newline at end of file + data = dict(user_input) + flow_user_id = ( + self.context.get("user_id") + if isinstance(self.context, dict) + else None + ) + if flow_user_id: + data[CONF_ADMIN_USER_IDS] = [flow_user_id] + return self.async_create_entry(title="NestQuest", data=data) \ No newline at end of file diff --git a/custom_components/nestquest/options_flow.py b/custom_components/nestquest/options_flow.py index c106723..2efe70a 100644 --- a/custom_components/nestquest/options_flow.py +++ b/custom_components/nestquest/options_flow.py @@ -7,6 +7,7 @@ from typing import Any import voluptuous as vol from homeassistant import config_entries from homeassistant.data_entry_flow import FlowResult +from homeassistant.helpers import config_validation as cv from .const import ( CONF_ADMIN_USER_IDS, @@ -31,29 +32,6 @@ def _parse_time(value: str) -> bool: return _TIME_PATTERN.fullmatch(value) is not None -def _multi_select(choices: dict[str, str]): - """A voluptuous validator for a multi-select of ``choices``. - - The mock-only test harness cannot import HA's config-validation - helpers (they pull in real HA), so this mirrors the shape of - ``homeassistant.helpers.config_validation.multi_select``: accepts a - list of keys from ``choices``, returns the list unchanged, rejects - anything else (None becomes the empty selection). - """ - - def _validate(value): - if value is None: - return [] - if not isinstance(value, list): - raise vol.Invalid("expected a list of selections") - for item in value: - if item not in choices: - raise vol.Invalid(f"{item!r} is not a valid choice") - return list(value) - - return _validate - - def _async_validate(user_input: dict[str, Any]) -> dict[str, str]: """Validate user input and return field-level errors instead of raising.""" errors: dict[str, str] = {} @@ -106,11 +84,13 @@ def _build_schema( ): int, } if admin_choices is not None: + # HA's supported multi-select validator: the frontend can + # serialize the field as a real picker of the given choices. fields[ vol.Optional( CONF_ADMIN_USER_IDS, default=list(admin_default or []) ) - ] = _multi_select(admin_choices) + ] = cv.multi_select(admin_choices) return vol.Schema(fields) diff --git a/tests/conftest.py b/tests/conftest.py index 2e67bfd..dd0fc25 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -42,6 +42,8 @@ _HA_MODULES = ( "homeassistant.config_entries", "homeassistant.data_entry_flow", "homeassistant.exceptions", + "homeassistant.helpers", + "homeassistant.helpers.config_validation", ) def _find_spec(name: str): @@ -79,6 +81,8 @@ for _parent, _child in ( ("homeassistant", "data_entry_flow"), ("homeassistant", "core"), ("homeassistant", "exceptions"), + ("homeassistant", "helpers"), + ("homeassistant.helpers", "config_validation"), ): setattr(sys.modules[_parent], _child, sys.modules[f"{_parent}.{_child}"]) @@ -181,6 +185,30 @@ class ConfigEntryNotReady(Exception): _exceptions_mock.ConfigEntryNotReady = ConfigEntryNotReady +import voluptuous as vol + + +def _multi_select(choices: dict): + """Mirror homeassistant.helpers.config_validation.multi_select. + + The options flow's admin picker uses HA's supported multi-select + validator (schema-serializable by HA's frontend); the mock provides + the same contract: a list whose members are keys of ``choices``. + """ + + def _validate(selected): + if not isinstance(selected, list): + raise vol.Invalid("Not a list") + for value in selected: + if value not in choices: + raise vol.Invalid(f"{value} is not a valid option") + return selected + + return _validate + + +_ha_mock("homeassistant.helpers.config_validation").multi_select = _multi_select + # --------------------------------------------------------------------------- # Shared helpers for the behavior tests. diff --git a/tests/test_admin_allowlist.py b/tests/test_admin_allowlist.py index f1ddf81..4ddbf28 100644 --- a/tests/test_admin_allowlist.py +++ b/tests/test_admin_allowlist.py @@ -278,3 +278,27 @@ async def test_setup_does_not_reseed_narrowed_allowlist( finally: await database.close() assert await async_unload_entry(hass, entry_reloaded) is True + + +async def test_setup_seed_prefers_options_over_entry_data( + hass, make_entry +) -> None: + """Seed precedence on a fresh database: the options flow's saved + list (the current one) beats the config flow's original copy.""" + from custom_components.nestquest.const import CONF_ADMIN_USER_IDS + + entry = await _setup_entry( + hass, + make_entry( + options={CONF_ADMIN_USER_IDS: ["user-current"]}, + data={CONF_ADMIN_USER_IDS: ["ha-owner"]}, + context={"user_id": "ha-owner"}, + ), + hass.registry, + ) + database = entry.runtime_data.database + try: + assert await list_admin_ids(database) == ["user-current"] + finally: + await database.close() + assert await async_unload_entry(hass, entry) is True diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index b744bf4..246e59b 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -154,4 +154,25 @@ def test_no_hardcoded_domain_or_db_filename_in_new_modules() -> None: ) assert node.value != "nestquest.db", ( f"Found hard-coded database filename in {py_file.name}:{node.lineno}" - ) \ No newline at end of file + ) + +async def test_flow_persists_flow_user_as_initial_admin(hass, make_flow) -> None: + """The HA user completing the flow lands in entry data as the + initial admin copy: entry.context is not persisted by real HA, so + the durable record must carry the owner.""" + from custom_components.nestquest.const import CONF_ADMIN_USER_IDS + + flow = make_flow(hass) + flow.context = {"user_id": "ha-owner"} + result = await flow.async_step_user({}) + assert result["type"] == "create_entry" + assert result["data"] == {CONF_ADMIN_USER_IDS: ["ha-owner"]} + + +async def test_flow_without_context_user_keeps_data_untouched( + hass, make_flow +) -> None: + flow = make_flow(hass) + result = await flow.async_step_user({"horizon_days": 7}) + assert result["type"] == "create_entry" + assert result["data"] == {"horizon_days": 7}