Axis - Allow manual configuration to update existing configuration (#30467)

* Allow manual configuration to update existing configuration

* Harmonize tests

* Understand what Elupus means...

homeassistant/components/axis/config_flow.py

@@ -64,9 +64,13 @@ class AxisFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
                 device = await get_device(self.hass, self.device_config)
 
                 self.serial_number = device.vapix.params.system_serialnumber
-
+                config_entry = await self.async_set_unique_id(self.serial_number)
-                await self.async_set_unique_id(self.serial_number)
+                if config_entry:
-                self._abort_if_unique_id_configured()
+                    return self._update_entry(
+                        config_entry,
+                        host=user_input[CONF_HOST],
+                        port=user_input[CONF_PORT],
+                    )
 
                 self.model = device.vapix.params.prodnbr
 
@@ -143,15 +147,14 @@ class AxisFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         if discovery_info[CONF_HOST].startswith("169.254"):
             return self.async_abort(reason="link_local_address")
 
-        for entry in self.hass.config_entries.async_entries(DOMAIN):
+        config_entry = await self.async_set_unique_id(serial_number)
-            if serial_number == entry.unique_id:
+        if config_entry:
-                return self._update_entry(
+            return self._update_entry(
-                    entry,
+                config_entry,
-                    host=discovery_info[CONF_HOST],
+                host=discovery_info[CONF_HOST],
-                    port=discovery_info[CONF_PORT],
+                port=discovery_info[CONF_PORT],
-                )
+            )
 
-        await self.async_set_unique_id(serial_number)
         # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167
         self.context["title_placeholders"] = {
             "name": discovery_info["hostname"][:-7],

tests/components/axis/test_config_flow.py

@@ -1,9 +1,6 @@
 """Test Axis config flow."""
 from unittest.mock import Mock, patch
 
-import pytest
-
-import homeassistant
 from homeassistant.components import axis
 from homeassistant.components.axis import config_flow
 
@@ -12,31 +9,37 @@ from .test_device import MAC, setup_axis_integration
 from tests.common import MockConfigEntry, mock_coro
 
 
-async def test_flow_works(hass):
+def setup_mock_axis_device(mock_device):
+    """Prepare mock axis device."""
+
+    def mock_constructor(loop, host, username, password, port, web_proto):
+        """Fake the controller constructor."""
+        mock_device.loop = loop
+        mock_device.host = host
+        mock_device.username = username
+        mock_device.password = password
+        mock_device.port = port
+        return mock_device
+
+    mock_device.side_effect = mock_constructor
+    mock_device.vapix.params.system_serialnumber = MAC
+    mock_device.vapix.params.prodnbr = "prodnbr"
+    mock_device.vapix.params.prodtype = "prodtype"
+    mock_device.vapix.params.firmware_version = "firmware_version"
+
+
+async def test_flow_manual_configuration(hass):
     """Test that config flow works."""
+    result = await hass.config_entries.flow.async_init(
+        config_flow.DOMAIN, context={"source": "user"}
+    )
+
+    assert result["type"] == "form"
+    assert result["step_id"] == "user"
+
     with patch("axis.AxisDevice") as mock_device:
 
-        def mock_constructor(loop, host, username, password, port, web_proto):
+        setup_mock_axis_device(mock_device)
-            """Fake the controller constructor."""
-            mock_device.loop = loop
-            mock_device.host = host
-            mock_device.username = username
-            mock_device.password = password
-            mock_device.port = port
-            return mock_device
-
-        mock_device.side_effect = mock_constructor
-        mock_device.vapix.params.system_serialnumber = "serialnumber"
-        mock_device.vapix.params.prodnbr = "prodnbr"
-        mock_device.vapix.params.prodtype = "prodtype"
-        mock_device.vapix.params.firmware_version = "firmware_version"
-
-        result = await hass.config_entries.flow.async_init(
-            config_flow.DOMAIN, context={"source": "user"}
-        )
-
-        assert result["type"] == "form"
-        assert result["step_id"] == "user"
 
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
@@ -49,7 +52,7 @@ async def test_flow_works(hass):
         )
 
     assert result["type"] == "create_entry"
-    assert result["title"] == f"prodnbr - serialnumber"
+    assert result["title"] == f"prodnbr - {MAC}"
     assert result["data"] == {
         axis.CONF_DEVICE: {
             config_flow.CONF_HOST: "1.2.3.4",
@@ -57,19 +60,22 @@ async def test_flow_works(hass):
             config_flow.CONF_PASSWORD: "pass",
             config_flow.CONF_PORT: 80,
         },
-        config_flow.CONF_MAC: "serialnumber",
+        config_flow.CONF_MAC: MAC,
         config_flow.CONF_MODEL: "prodnbr",
         config_flow.CONF_NAME: "prodnbr 0",
     }
 
 
-async def test_flow_fails_already_configured(hass):
+async def test_manual_configuration_update_configuration(hass):
     """Test that config flow fails on already configured device."""
-    await setup_axis_integration(hass)
+    device = await setup_axis_integration(hass)
 
-    flow = config_flow.AxisFlowHandler()
+    result = await hass.config_entries.flow.async_init(
-    flow.hass = hass
+        config_flow.DOMAIN, context={"source": "user"}
-    flow.context = {}
+    )
+
+    assert result["type"] == "form"
+    assert result["step_id"] == "user"
 
     mock_device = Mock()
     mock_device.vapix.params.system_serialnumber = MAC
@@ -77,33 +83,78 @@ async def test_flow_fails_already_configured(hass):
     with patch(
         "homeassistant.components.axis.config_flow.get_device",
         return_value=mock_coro(mock_device),
-    ), pytest.raises(homeassistant.data_entry_flow.AbortFlow):
+    ):
-        await flow.async_step_user(
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                config_flow.CONF_HOST: "2.3.4.5",
+                config_flow.CONF_USERNAME: "user",
+                config_flow.CONF_PASSWORD: "pass",
+                config_flow.CONF_PORT: 80,
+            },
+        )
+
+    assert result["type"] == "abort"
+    assert result["reason"] == "updated_configuration"
+    assert (
+        device.config_entry.data[config_flow.CONF_DEVICE][config_flow.CONF_HOST]
+        == "2.3.4.5"
+    )
+
+
+async def test_flow_fails_already_configured(hass):
+    """Test that config flow fails on already configured device."""
+    await setup_axis_integration(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        config_flow.DOMAIN, context={"source": "user"}
+    )
+
+    assert result["type"] == "form"
+    assert result["step_id"] == "user"
+
+    mock_device = Mock()
+    mock_device.vapix.params.system_serialnumber = MAC
+
+    with patch(
+        "homeassistant.components.axis.config_flow.get_device",
+        return_value=mock_coro(mock_device),
+    ):
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
             user_input={
                 config_flow.CONF_HOST: "1.2.3.4",
                 config_flow.CONF_USERNAME: "user",
                 config_flow.CONF_PASSWORD: "pass",
                 config_flow.CONF_PORT: 80,
-            }
+            },
         )
 
+    assert result["type"] == "abort"
+    assert result["reason"] == "already_configured"
+
 
 async def test_flow_fails_faulty_credentials(hass):
     """Test that config flow fails on faulty credentials."""
-    flow = config_flow.AxisFlowHandler()
+    result = await hass.config_entries.flow.async_init(
-    flow.hass = hass
+        config_flow.DOMAIN, context={"source": "user"}
+    )
+
+    assert result["type"] == "form"
+    assert result["step_id"] == "user"
 
     with patch(
         "homeassistant.components.axis.config_flow.get_device",
         side_effect=config_flow.AuthenticationRequired,
     ):
-        result = await flow.async_step_user(
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
             user_input={
                 config_flow.CONF_HOST: "1.2.3.4",
                 config_flow.CONF_USERNAME: "user",
                 config_flow.CONF_PASSWORD: "pass",
                 config_flow.CONF_PORT: 80,
-            }
+            },
         )
 
     assert result["errors"] == {"base": "faulty_credentials"}
@@ -111,56 +162,79 @@ async def test_flow_fails_faulty_credentials(hass):
 
 async def test_flow_fails_device_unavailable(hass):
     """Test that config flow fails on device unavailable."""
-    flow = config_flow.AxisFlowHandler()
+    result = await hass.config_entries.flow.async_init(
-    flow.hass = hass
+        config_flow.DOMAIN, context={"source": "user"}
+    )
+
+    assert result["type"] == "form"
+    assert result["step_id"] == "user"
 
     with patch(
         "homeassistant.components.axis.config_flow.get_device",
         side_effect=config_flow.CannotConnect,
     ):
-        result = await flow.async_step_user(
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
             user_input={
                 config_flow.CONF_HOST: "1.2.3.4",
                 config_flow.CONF_USERNAME: "user",
                 config_flow.CONF_PASSWORD: "pass",
                 config_flow.CONF_PORT: 80,
-            }
+            },
         )
 
     assert result["errors"] == {"base": "device_unavailable"}
 
 
-async def test_flow_create_entry(hass):
+async def test_flow_create_entry_multiple_existing_entries_of_same_model(hass):
-    """Test that create entry can generate a name without other entries."""
-    flow = config_flow.AxisFlowHandler()
-    flow.hass = hass
-    flow.model = "model"
-
-    result = await flow._create_entry()
-
-    assert result["data"][config_flow.CONF_NAME] == "model 0"
-
-
-async def test_flow_create_entry_more_entries(hass):
     """Test that create entry can generate a name with other entries."""
     entry = MockConfigEntry(
         domain=axis.DOMAIN,
-        data={config_flow.CONF_NAME: "model 0", config_flow.CONF_MODEL: "model"},
+        data={config_flow.CONF_NAME: "prodnbr 0", config_flow.CONF_MODEL: "prodnbr"},
     )
     entry.add_to_hass(hass)
     entry2 = MockConfigEntry(
         domain=axis.DOMAIN,
-        data={config_flow.CONF_NAME: "model 1", config_flow.CONF_MODEL: "model"},
+        data={config_flow.CONF_NAME: "prodnbr 1", config_flow.CONF_MODEL: "prodnbr"},
     )
     entry2.add_to_hass(hass)
 
-    flow = config_flow.AxisFlowHandler()
+    result = await hass.config_entries.flow.async_init(
-    flow.hass = hass
+        config_flow.DOMAIN, context={"source": "user"}
-    flow.model = "model"
+    )
 
-    result = await flow._create_entry()
+    assert result["type"] == "form"
+    assert result["step_id"] == "user"
 
-    assert result["data"][config_flow.CONF_NAME] == "model 2"
+    with patch("axis.AxisDevice") as mock_device:
+
+        setup_mock_axis_device(mock_device)
+
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                config_flow.CONF_HOST: "1.2.3.4",
+                config_flow.CONF_USERNAME: "user",
+                config_flow.CONF_PASSWORD: "pass",
+                config_flow.CONF_PORT: 80,
+            },
+        )
+
+    assert result["type"] == "create_entry"
+    assert result["title"] == f"prodnbr - {MAC}"
+    assert result["data"] == {
+        axis.CONF_DEVICE: {
+            config_flow.CONF_HOST: "1.2.3.4",
+            config_flow.CONF_USERNAME: "user",
+            config_flow.CONF_PASSWORD: "pass",
+            config_flow.CONF_PORT: 80,
+        },
+        config_flow.CONF_MAC: MAC,
+        config_flow.CONF_MODEL: "prodnbr",
+        config_flow.CONF_NAME: "prodnbr 2",
+    }
+
+    assert result["data"][config_flow.CONF_NAME] == "prodnbr 2"
 
 
 async def test_zeroconf_flow(hass):
@@ -172,7 +246,7 @@ async def test_zeroconf_flow(hass):
                 config_flow.CONF_HOST: "1.2.3.4",
                 config_flow.CONF_PORT: 80,
                 "hostname": "name",
-                "properties": {"macaddress": "00408C12345"},
+                "properties": {"macaddress": MAC},
             },
             context={"source": "zeroconf"},
         )
@@ -180,6 +254,36 @@ async def test_zeroconf_flow(hass):
     assert result["type"] == "form"
     assert result["step_id"] == "user"
 
+    with patch("axis.AxisDevice") as mock_device:
+
+        setup_mock_axis_device(mock_device)
+
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            user_input={
+                config_flow.CONF_HOST: "1.2.3.4",
+                config_flow.CONF_USERNAME: "user",
+                config_flow.CONF_PASSWORD: "pass",
+                config_flow.CONF_PORT: 80,
+            },
+        )
+
+    assert result["type"] == "create_entry"
+    assert result["title"] == f"prodnbr - {MAC}"
+    assert result["data"] == {
+        axis.CONF_DEVICE: {
+            config_flow.CONF_HOST: "1.2.3.4",
+            config_flow.CONF_USERNAME: "user",
+            config_flow.CONF_PASSWORD: "pass",
+            config_flow.CONF_PORT: 80,
+        },
+        config_flow.CONF_MAC: MAC,
+        config_flow.CONF_MODEL: "prodnbr",
+        config_flow.CONF_NAME: "prodnbr 0",
+    }
+
+    assert result["data"][config_flow.CONF_NAME] == "prodnbr 0"
+
 
 async def test_zeroconf_flow_already_configured(hass):
     """Test that zeroconf doesn't setup already configured devices."""
@@ -192,7 +296,7 @@ async def test_zeroconf_flow_already_configured(hass):
             config_flow.CONF_HOST: "1.2.3.4",
             config_flow.CONF_PORT: 80,
             "hostname": "name",
-            "properties": {"macaddress": "00408C12345"},
+            "properties": {"macaddress": MAC},
         },
         context={"source": "zeroconf"},
     )
@@ -245,10 +349,7 @@ async def test_zeroconf_flow_ignore_link_local_address(hass):
     """Test that zeroconf doesn't setup devices with link local addresses."""
     result = await hass.config_entries.flow.async_init(
         config_flow.DOMAIN,
-        data={
+        data={config_flow.CONF_HOST: "169.254.3.4", "properties": {"macaddress": MAC}},
-            config_flow.CONF_HOST: "169.254.3.4",
-            "properties": {"macaddress": "00408C12345"},
-        },
         context={"source": "zeroconf"},
     )