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 <joostlek@outlook.com>

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,
+        )

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%]"

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"