Support reconfigure flow in Brother integration (#117298)

* Add reconfigure flow

* Improve config flow

* Check if it is the same printer

* Improve description

* Add tests

* Improve strings

* Add missing reconfigure_successful string

* Improve test names and comments

* Format

* Mock unload entry

* Use add_suggested_values_to_schema()

* Do not abort when another device's IP has been used

* Remove unnecessary code

* Suggested changes

---------

Co-authored-by: Maciej Bieniek <478555+bieniu@users.noreply.github.com>

homeassistant/components/brother/config_flow.py

@@ -2,15 +2,16 @@
 
 from __future__ import annotations
 
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 from brother import Brother, SnmpError, UnsupportedModelError
 import voluptuous as vol
 
 from homeassistant.components import zeroconf
 from homeassistant.components.snmp import async_get_snmp_engine
-from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
+from homeassistant.config_entries import ConfigEntry, ConfigFlow, ConfigFlowResult
 from homeassistant.const import CONF_HOST, CONF_TYPE
+from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.util.network import is_host_valid
 
@@ -18,10 +19,29 @@ from .const import DOMAIN, PRINTER_TYPES
 
 DATA_SCHEMA = vol.Schema(
     {
-        vol.Required(CONF_HOST, default=""): str,
+        vol.Required(CONF_HOST): str,
         vol.Optional(CONF_TYPE, default="laser"): vol.In(PRINTER_TYPES),
     }
 )
+RECONFIGURE_SCHEMA = vol.Schema({vol.Required(CONF_HOST): str})
+
+
+async def validate_input(
+    hass: HomeAssistant, user_input: dict[str, Any], expected_mac: str | None = None
+) -> tuple[str, str]:
+    """Validate the user input."""
+    if not is_host_valid(user_input[CONF_HOST]):
+        raise InvalidHost
+
+    snmp_engine = await async_get_snmp_engine(hass)
+
+    brother = await Brother.create(user_input[CONF_HOST], snmp_engine=snmp_engine)
+    await brother.async_update()
+
+    if expected_mac is not None and brother.serial.lower() != expected_mac:
+        raise AnotherDevice
+
+    return (brother.model, brother.serial)
 
 
 class BrotherConfigFlow(ConfigFlow, domain=DOMAIN):
@@ -33,6 +53,7 @@ class BrotherConfigFlow(ConfigFlow, domain=DOMAIN):
         """Initialize."""
         self.brother: Brother
         self.host: str | None = None
+        self.entry: ConfigEntry | None = None
 
     async def async_step_user(
         self, user_input: dict[str, Any] | None = None
@@ -42,21 +63,7 @@ class BrotherConfigFlow(ConfigFlow, domain=DOMAIN):
 
         if user_input is not None:
             try:
-                if not is_host_valid(user_input[CONF_HOST]):
+                model, serial = await validate_input(self.hass, user_input)
-                    raise InvalidHost
-
-                snmp_engine = await async_get_snmp_engine(self.hass)
-
-                brother = await Brother.create(
-                    user_input[CONF_HOST], snmp_engine=snmp_engine
-                )
-                await brother.async_update()
-
-                await self.async_set_unique_id(brother.serial.lower())
-                self._abort_if_unique_id_configured()
-
-                title = f"{brother.model} {brother.serial}"
-                return self.async_create_entry(title=title, data=user_input)
             except InvalidHost:
                 errors[CONF_HOST] = "wrong_host"
             except (ConnectionError, TimeoutError):
@@ -65,6 +72,12 @@ class BrotherConfigFlow(ConfigFlow, domain=DOMAIN):
                 errors["base"] = "snmp_error"
             except UnsupportedModelError:
                 return self.async_abort(reason="unsupported_model")
+            else:
+                await self.async_set_unique_id(serial.lower())
+                self._abort_if_unique_id_configured()
+
+                title = f"{model} {serial}"
+                return self.async_create_entry(title=title, data=user_input)
 
         return self.async_show_form(
             step_id="user", data_schema=DATA_SCHEMA, errors=errors
@@ -127,6 +140,61 @@ class BrotherConfigFlow(ConfigFlow, domain=DOMAIN):
             },
         )
 
+    async def async_step_reconfigure(
+        self, _: dict[str, Any] | None = None
+    ) -> ConfigFlowResult:
+        """Handle a reconfiguration flow initialized by the user."""
+        entry = self.hass.config_entries.async_get_entry(self.context["entry_id"])
+
+        if TYPE_CHECKING:
+            assert entry is not None
+
+        self.entry = entry
+
+        return await self.async_step_reconfigure_confirm()
+
+    async def async_step_reconfigure_confirm(
+        self, user_input: dict[str, Any] | None = None
+    ) -> ConfigFlowResult:
+        """Handle a reconfiguration flow initialized by the user."""
+        errors = {}
+
+        if TYPE_CHECKING:
+            assert self.entry is not None
+
+        if user_input is not None:
+            try:
+                await validate_input(self.hass, user_input, self.entry.unique_id)
+            except InvalidHost:
+                errors[CONF_HOST] = "wrong_host"
+            except (ConnectionError, TimeoutError):
+                errors["base"] = "cannot_connect"
+            except SnmpError:
+                errors["base"] = "snmp_error"
+            except AnotherDevice:
+                errors["base"] = "another_device"
+            else:
+                self.hass.config_entries.async_update_entry(
+                    self.entry,
+                    data=self.entry.data | {CONF_HOST: user_input[CONF_HOST]},
+                )
+                await self.hass.config_entries.async_reload(self.entry.entry_id)
+                return self.async_abort(reason="reconfigure_successful")
+
+        return self.async_show_form(
+            step_id="reconfigure_confirm",
+            data_schema=self.add_suggested_values_to_schema(
+                data_schema=RECONFIGURE_SCHEMA,
+                suggested_values=self.entry.data | (user_input or {}),
+            ),
+            description_placeholders={"printer_name": self.entry.title},
+            errors=errors,
+        )
+
 
 class InvalidHost(HomeAssistantError):
     """Error to indicate that hostname/IP address is invalid."""
+
+
+class AnotherDevice(HomeAssistantError):
+    """Error to indicate that hostname/IP address belongs to another device."""

homeassistant/components/brother/strings.json

@@ -17,16 +17,27 @@
         "data": {
           "type": "[%key:component::brother::config::step::user::data::type%]"
         }
+      },
+      "reconfigure_confirm": {
+        "description": "Update configuration for {printer_name}.",
+        "data": {
+          "host": "[%key:common::config_flow::data::host%]"
+        },
+        "data_description": {
+          "host": "[%key:component::brother::config::step::user::data_description::host%]"
+        }
       }
     },
     "error": {
       "wrong_host": "Invalid hostname or IP address.",
       "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
-      "snmp_error": "SNMP server turned off or printer not supported."
+      "snmp_error": "SNMP server turned off or printer not supported.",
+      "another_device": "The IP address or hostname of another Brother printer was used."
     },
     "abort": {
       "unsupported_model": "This printer model is not supported.",
-      "already_configured": "[%key:common::config_flow::abort::already_configured_device%]"
+      "already_configured": "[%key:common::config_flow::abort::already_configured_device%]",
+      "reconfigure_successful": "[%key:common::config_flow::abort::reconfigure_successful%]"
     }
   },
   "entity": {

tests/components/brother/conftest.py

@@ -87,7 +87,16 @@ def mock_setup_entry() -> Generator[AsyncMock]:
 
 
 @pytest.fixture
-def mock_brother_client() -> Generator[AsyncMock]:
+def mock_unload_entry() -> Generator[AsyncMock, None, None]:
+    """Override async_unload_entry."""
+    with patch(
+        "homeassistant.components.brother.async_unload_entry", return_value=True
+    ) as mock_unload_entry:
+        yield mock_unload_entry
+
+
+@pytest.fixture
+def mock_brother_client() -> Generator[AsyncMock, None, None]:
     """Mock Brother client."""
     with (
         patch("homeassistant.components.brother.Brother", autospec=True) as mock_client,

tests/components/brother/test_config_flow.py

@@ -8,7 +8,11 @@ import pytest
 
 from homeassistant.components import zeroconf
 from homeassistant.components.brother.const import DOMAIN
-from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF
+from homeassistant.config_entries import (
+    SOURCE_RECONFIGURE,
+    SOURCE_USER,
+    SOURCE_ZEROCONF,
+)
 from homeassistant.const import CONF_HOST, CONF_TYPE
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
@@ -19,7 +23,7 @@ from tests.common import MockConfigEntry
 
 CONFIG = {CONF_HOST: "127.0.0.1", CONF_TYPE: "laser"}
 
-pytestmark = pytest.mark.usefixtures("mock_setup_entry")
+pytestmark = pytest.mark.usefixtures("mock_setup_entry", "mock_unload_entry")
 
 
 async def test_show_form(hass: HomeAssistant) -> None:
@@ -248,3 +252,154 @@ async def test_zeroconf_confirm_create_entry(
     assert result["title"] == "HL-L2340DW 0123456789"
     assert result["data"][CONF_HOST] == "127.0.0.1"
     assert result["data"][CONF_TYPE] == "laser"
+
+
+async def test_reconfigure_successful(
+    hass: HomeAssistant,
+    mock_brother_client: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+) -> None:
+    """Test starting a reconfigure flow."""
+    await init_integration(hass, mock_config_entry)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": SOURCE_RECONFIGURE,
+            "entry_id": mock_config_entry.entry_id,
+        },
+        data=mock_config_entry.data,
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        user_input={CONF_HOST: "10.10.10.10"},
+    )
+
+    assert result["type"] is FlowResultType.ABORT
+    assert result["reason"] == "reconfigure_successful"
+    assert mock_config_entry.data == {
+        CONF_HOST: "10.10.10.10",
+        CONF_TYPE: "laser",
+    }
+
+
+@pytest.mark.parametrize(
+    ("exc", "base_error"),
+    [
+        (ConnectionError, "cannot_connect"),
+        (TimeoutError, "cannot_connect"),
+        (SnmpError("error"), "snmp_error"),
+    ],
+)
+async def test_reconfigure_not_successful(
+    hass: HomeAssistant,
+    exc: Exception,
+    base_error: str,
+    mock_brother_client: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+) -> None:
+    """Test starting a reconfigure flow but no connection found."""
+    await init_integration(hass, mock_config_entry)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": SOURCE_RECONFIGURE,
+            "entry_id": mock_config_entry.entry_id,
+        },
+        data=mock_config_entry.data,
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+
+    mock_brother_client.async_update.side_effect = exc
+
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        user_input={CONF_HOST: "10.10.10.10"},
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+    assert result["errors"] == {"base": base_error}
+
+    mock_brother_client.async_update.side_effect = None
+
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        user_input={CONF_HOST: "10.10.10.10"},
+    )
+
+    assert result["type"] is FlowResultType.ABORT
+    assert result["reason"] == "reconfigure_successful"
+    assert mock_config_entry.data == {
+        CONF_HOST: "10.10.10.10",
+        CONF_TYPE: "laser",
+    }
+
+
+async def test_reconfigure_invalid_hostname(
+    hass: HomeAssistant,
+    mock_brother_client: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+) -> None:
+    """Test starting a reconfigure flow but no connection found."""
+    await init_integration(hass, mock_config_entry)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": SOURCE_RECONFIGURE,
+            "entry_id": mock_config_entry.entry_id,
+        },
+        data=mock_config_entry.data,
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        user_input={CONF_HOST: "invalid/hostname"},
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+    assert result["errors"] == {CONF_HOST: "wrong_host"}
+
+
+async def test_reconfigure_not_the_same_device(
+    hass: HomeAssistant,
+    mock_brother_client: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+) -> None:
+    """Test starting the reconfiguration process, but with a different printer."""
+    await init_integration(hass, mock_config_entry)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={
+            "source": SOURCE_RECONFIGURE,
+            "entry_id": mock_config_entry.entry_id,
+        },
+        data=mock_config_entry.data,
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+
+    mock_brother_client.serial = "9876543210"
+
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"],
+        user_input={CONF_HOST: "10.10.10.10"},
+    )
+
+    assert result["type"] is FlowResultType.FORM
+    assert result["step_id"] == "reconfigure_confirm"
+    assert result["errors"] == {"base": "another_device"}