mirror of
https://github.com/basnijholt/adaptive-lighting.git
synced 2026-09-11 22:34:04 +02:00
Fix options flow changes silently discarded for pre-refactor UI entries (#1504)
* Fix options flow changes being silently discarded for UI-configured entries
validate() merges config_entry.options then config_entry.data, on the
assumption that data only ever holds YAML-imported settings (which should
win) or, for UI-created entries, just the entry name (harmless to apply
last).
That assumption doesn't hold for entries created before data/options were
split: their data still carries the full settings snapshot from initial
setup. Applying it after options means any change made through the options
flow for a key that already exists in data (e.g. adding a light) is
silently ignored, even though the options flow reports success and the
entry reloads without error.
Reproduced on a real entry: added a light via the options flow, entry
reloaded cleanly, but the light was never picked up by the switch's
service-call interceptor ("No switch found for entity_id=...") because
data still held the old light list and clobbered the updated options.
Fix: only let data win over options for genuinely YAML-imported entries
(config_entry.source == SOURCE_IMPORT), matching the existing use of that
check elsewhere in this file. For UI-configured entries, apply options
last so changes made through the options flow actually take effect.
* Add focused tests for the data/options merge order in validate()
Covers both source-specific contracts the merge logic relies on, per
review feedback on this PR:
- SOURCE_USER: options must win over data (this PR's actual fix - proven
to fail against the pre-fix code, verified locally by reverting
switch.py and re-running).
- SOURCE_IMPORT: data must keep winning over options (the existing,
intentional YAML-precedence behavior - unchanged by this PR, verified
to already pass against the pre-fix code too).
Verified against a real Home Assistant instance's test harness
(pytest-homeassistant-custom-component + the actual installed
homeassistant package), not just reasoned about statically.
---------
Co-authored-by: Bas Nijholt <bas@nijho.lt>
This commit is contained in:
parent
6d46b82313
commit
68c66a0ff3
2 changed files with 56 additions and 3 deletions
|
|
@ -565,8 +565,20 @@ def validate(
|
|||
if config_entry is not None:
|
||||
assert service_data is None
|
||||
assert defaults is None
|
||||
data.update(config_entry.options) # come from options flow
|
||||
data.update(config_entry.data) # all yaml settings come from data
|
||||
if config_entry.source == SOURCE_IMPORT:
|
||||
# YAML-configured entries: `data` is the authoritative YAML config
|
||||
# and must win over any stray `options` from a prior UI setup.
|
||||
data.update(config_entry.options)
|
||||
data.update(config_entry.data)
|
||||
else:
|
||||
# UI-configured entries: settings are meant to live in `options`
|
||||
# (see OptionsFlowHandler in config_flow.py). `data` here is
|
||||
# either just the entry name, or - for entries created before
|
||||
# data/options were split - a stale snapshot from initial setup.
|
||||
# Applying it last would silently discard newer changes made
|
||||
# through the options flow, so `options` must win instead.
|
||||
data.update(config_entry.data)
|
||||
data.update(config_entry.options)
|
||||
else:
|
||||
assert service_data is not None
|
||||
changed_settings = {
|
||||
|
|
|
|||
|
|
@ -75,6 +75,7 @@ from homeassistant.components.adaptive_lighting.switch import (
|
|||
is_our_context,
|
||||
is_our_context_id,
|
||||
short_hash,
|
||||
validate,
|
||||
)
|
||||
from homeassistant.components.light import (
|
||||
ATTR_BRIGHTNESS,
|
||||
|
|
@ -98,7 +99,7 @@ except ImportError:
|
|||
from homeassistant.components.template.light import LightTemplate
|
||||
|
||||
from homeassistant.components.template import light as template_light
|
||||
from homeassistant.config_entries import ConfigEntryState
|
||||
from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER, ConfigEntryState
|
||||
from homeassistant.const import (
|
||||
ATTR_AREA_ID,
|
||||
ATTR_ENTITY_ID,
|
||||
|
|
@ -3235,3 +3236,43 @@ async def test_detect_non_ha_changes_with_separate_turn_on_commands(hass):
|
|||
assert (
|
||||
light.brightness == manual_brightness
|
||||
), f"AL overrode manual brightness {manual_brightness} with {al_brightness}"
|
||||
|
||||
|
||||
def test_validate_ui_options_win_over_stale_data():
|
||||
"""A UI-configured entry's `options` (from the options flow) must win.
|
||||
|
||||
`data` for a `SOURCE_USER` entry either only holds the entry name, or -
|
||||
for entries created before `data`/`options` were split - a stale
|
||||
snapshot from initial setup. Either way, a later change made through
|
||||
the options flow (stored in `options`) must not be silently discarded
|
||||
by that stale/legacy `data`.
|
||||
"""
|
||||
entry = MockConfigEntry(
|
||||
domain=DOMAIN,
|
||||
source=SOURCE_USER,
|
||||
data={CONF_NAME: DEFAULT_NAME, CONF_LIGHTS: ["light.a"]},
|
||||
options={CONF_LIGHTS: ["light.a", "light.b"]},
|
||||
)
|
||||
|
||||
result = validate(entry)
|
||||
|
||||
assert result[CONF_LIGHTS] == ["light.a", "light.b"]
|
||||
|
||||
|
||||
def test_validate_yaml_data_wins_over_stray_options():
|
||||
"""A YAML-imported entry's `data` must keep winning over `options`.
|
||||
|
||||
YAML configuration is the source of truth for a `SOURCE_IMPORT` entry,
|
||||
so any leftover `options` (e.g. from a UI setup that predates the YAML
|
||||
import) must not override it.
|
||||
"""
|
||||
entry = MockConfigEntry(
|
||||
domain=DOMAIN,
|
||||
source=SOURCE_IMPORT,
|
||||
data={CONF_NAME: DEFAULT_NAME, CONF_LIGHTS: ["light.a"]},
|
||||
options={CONF_LIGHTS: ["light.b"]},
|
||||
)
|
||||
|
||||
result = validate(entry)
|
||||
|
||||
assert result[CONF_LIGHTS] == ["light.a"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue