From 68a4e1a112d34d3eee00dcce2f75f69e78cd6e8b Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Mon, 26 May 2025 09:10:30 -0400 Subject: [PATCH] Add reconfigure config flow to APCUPSD (#143801) * Add reconfigure config flow * Add reconfigure config flow * Add more subtests for wrong device * Reduce the patch scopes * Address comments * Fix --------- Co-authored-by: Joostlek --- .../components/apcupsd/config_flow.py | 31 +++- homeassistant/components/apcupsd/strings.json | 4 +- tests/components/apcupsd/test_config_flow.py | 133 +++++++++++++++++- 3 files changed, 158 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/apcupsd/config_flow.py b/homeassistant/components/apcupsd/config_flow.py index b65c9c33265..bd26aa0a2d4 100644 --- a/homeassistant/components/apcupsd/config_flow.py +++ b/homeassistant/components/apcupsd/config_flow.py @@ -46,11 +46,7 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN): return self.async_show_form(step_id="user", data_schema=_SCHEMA) host, port = user_input[CONF_HOST], user_input[CONF_PORT] - - # Abort if an entry with same host and port is present. self._async_abort_entries_match({CONF_HOST: host, CONF_PORT: port}) - - # Test the connection to the host and get the current status for serial number. try: async with asyncio.timeout(CONNECTION_TIMEOUT): data = APCUPSdData(await aioapcaccess.request_status(host, port)) @@ -67,3 +63,30 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN): title = data.name or data.model or data.serial_no or "APC UPS" return self.async_create_entry(title=title, data=user_input) + + async def async_step_reconfigure( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle reconfiguration of an existing entry.""" + + if user_input is None: + return self.async_show_form(step_id="reconfigure", data_schema=_SCHEMA) + + host, port = user_input[CONF_HOST], user_input[CONF_PORT] + self._async_abort_entries_match({CONF_HOST: host, CONF_PORT: port}) + try: + async with asyncio.timeout(CONNECTION_TIMEOUT): + data = APCUPSdData(await aioapcaccess.request_status(host, port)) + except (OSError, asyncio.IncompleteReadError, TimeoutError): + errors = {"base": "cannot_connect"} + return self.async_show_form( + step_id="reconfigure", data_schema=_SCHEMA, errors=errors + ) + + await self.async_set_unique_id(data.serial_no) + self._abort_if_unique_id_mismatch(reason="wrong_apcupsd_daemon") + + return self.async_update_reload_and_abort( + self._get_reconfigure_entry(), + data_updates=user_input, + ) diff --git a/homeassistant/components/apcupsd/strings.json b/homeassistant/components/apcupsd/strings.json index 27a620491d1..d821b66ef67 100644 --- a/homeassistant/components/apcupsd/strings.json +++ b/homeassistant/components/apcupsd/strings.json @@ -1,7 +1,9 @@ { "config": { "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", + "wrong_apcupsd_daemon": "The reconfigured APC UPS Daemon is not the same as the one already configured.", + "reconfigure_successful": "[%key:common::config_flow::abort::reconfigure_successful%]" }, "error": { "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" diff --git a/tests/components/apcupsd/test_config_flow.py b/tests/components/apcupsd/test_config_flow.py index 0b8386dbb5a..e635b7d6681 100644 --- a/tests/components/apcupsd/test_config_flow.py +++ b/tests/components/apcupsd/test_config_flow.py @@ -1,5 +1,7 @@ """Test APCUPSd config flow setup process.""" +from __future__ import annotations + from copy import copy from unittest.mock import patch @@ -25,7 +27,9 @@ def _patch_setup(): async def test_config_flow_cannot_connect(hass: HomeAssistant) -> None: """Test config flow setup with connection error.""" - with patch("aioapcaccess.request_status") as mock_get: + with patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status" + ) as mock_get: mock_get.side_effect = OSError() result = await hass.config_entries.flow.async_init( @@ -51,7 +55,9 @@ async def test_config_flow_duplicate(hass: HomeAssistant) -> None: mock_entry.add_to_hass(hass) with ( - patch("aioapcaccess.request_status") as mock_request_status, + patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status" + ) as mock_request_status, _patch_setup(), ): mock_request_status.return_value = MOCK_STATUS @@ -98,7 +104,10 @@ async def test_config_flow_duplicate(hass: HomeAssistant) -> None: async def test_flow_works(hass: HomeAssistant) -> None: """Test successful creation of config entries via user configuration.""" with ( - patch("aioapcaccess.request_status", return_value=MOCK_STATUS), + patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status", + return_value=MOCK_STATUS, + ), _patch_setup() as mock_setup, ): result = await hass.config_entries.flow.async_init( @@ -111,7 +120,6 @@ async def test_flow_works(hass: HomeAssistant) -> None: result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input=CONF_DATA ) - await hass.async_block_till_done() assert result["type"] is FlowResultType.CREATE_ENTRY assert result["title"] == MOCK_STATUS["UPSNAME"] assert result["data"] == CONF_DATA @@ -139,7 +147,9 @@ async def test_flow_minimal_status( integration will vary. """ with ( - patch("aioapcaccess.request_status") as mock_request_status, + patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status" + ) as mock_request_status, _patch_setup() as mock_setup, ): status = MOCK_MINIMAL_STATUS | extra_status @@ -153,3 +163,116 @@ async def test_flow_minimal_status( assert result["data"] == CONF_DATA assert result["title"] == expected_title mock_setup.assert_called_once() + + +async def test_reconfigure_flow_works(hass: HomeAssistant) -> None: + """Test successful reconfiguration of an existing entry.""" + mock_entry = MockConfigEntry( + version=1, + domain=DOMAIN, + title="APCUPSd", + data=CONF_DATA, + unique_id=MOCK_STATUS["SERIALNO"], + source=SOURCE_USER, + ) + mock_entry.add_to_hass(hass) + + result = await mock_entry.start_reconfigure_flow(hass) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reconfigure" + + # New configuration data with different host/port. + new_conf_data = {CONF_HOST: "new_host", CONF_PORT: 4321} + + with ( + patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status", + return_value=MOCK_STATUS, + ), + _patch_setup() as mock_setup, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=new_conf_data + ) + await hass.async_block_till_done() + mock_setup.assert_called_once() + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reconfigure_successful" + + # Check that the entry was updated with the new configuration. + assert mock_entry.data[CONF_HOST] == new_conf_data[CONF_HOST] + assert mock_entry.data[CONF_PORT] == new_conf_data[CONF_PORT] + + +async def test_reconfigure_flow_cannot_connect(hass: HomeAssistant) -> None: + """Test reconfiguration with connection error.""" + mock_entry = MockConfigEntry( + version=1, + domain=DOMAIN, + title="APCUPSd", + data=CONF_DATA, + unique_id=MOCK_STATUS["SERIALNO"], + source=SOURCE_USER, + ) + mock_entry.add_to_hass(hass) + + result = await mock_entry.start_reconfigure_flow(hass) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reconfigure" + + # New configuration data with different host/port. + new_conf_data = {CONF_HOST: "new_host", CONF_PORT: 4321} + with patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status", + side_effect=OSError(), + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=new_conf_data + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"]["base"] == "cannot_connect" + + +@pytest.mark.parametrize( + ("unique_id_before", "unique_id_after"), + [ + (None, MOCK_STATUS["SERIALNO"]), + (MOCK_STATUS["SERIALNO"], "Blank"), + (MOCK_STATUS["SERIALNO"], MOCK_STATUS["SERIALNO"] + "ZZZ"), + ], +) +async def test_reconfigure_flow_wrong_device( + hass: HomeAssistant, unique_id_before: str | None, unique_id_after: str | None +) -> None: + """Test reconfiguration with a different device (wrong serial number).""" + mock_entry = MockConfigEntry( + version=1, + domain=DOMAIN, + title="APCUPSd", + data=CONF_DATA, + unique_id=unique_id_before, + source=SOURCE_USER, + ) + mock_entry.add_to_hass(hass) + + result = await mock_entry.start_reconfigure_flow(hass) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reconfigure" + + # New configuration data with different host/port. + new_conf_data = {CONF_HOST: "new_host", CONF_PORT: 4321} + # Make a copy of the status and modify the serial number if needed. + mock_status = {k: v for k, v in MOCK_STATUS.items() if k != "SERIALNO"} + mock_status["SERIALNO"] = unique_id_after + with patch( + "homeassistant.components.apcupsd.coordinator.aioapcaccess.request_status", + return_value=mock_status, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=new_conf_data + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "wrong_apcupsd_daemon"