Suez_water: use meter id as unique_id (#133959)

* Suez_water: use meter id as unique_id

* Review fixes

* No more afraid check :)

* review again

* Apply suggestions from code review

---------

Co-authored-by: G Johansson <goran.johansson@shiftit.se>

homeassistant/components/suez_water/__init__.py

@@ -2,13 +2,18 @@
 
 from __future__ import annotations
 
+import logging
+
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
 
+from .const import CONF_COUNTER_ID
 from .coordinator import SuezWaterConfigEntry, SuezWaterCoordinator
 
 PLATFORMS: list[Platform] = [Platform.SENSOR]
 
+_LOGGER = logging.getLogger(__name__)
+
 
 async def async_setup_entry(hass: HomeAssistant, entry: SuezWaterConfigEntry) -> bool:
     """Set up Suez Water from a config entry."""
@@ -26,3 +31,36 @@ async def async_setup_entry(hass: HomeAssistant, entry: SuezWaterConfigEntry) ->
 async def async_unload_entry(hass: HomeAssistant, entry: SuezWaterConfigEntry) -> bool:
     """Unload a config entry."""
     return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
+
+
+async def async_migrate_entry(
+    hass: HomeAssistant, config_entry: SuezWaterConfigEntry
+) -> bool:
+    """Migrate old suez water config entry."""
+    _LOGGER.debug(
+        "Migrating configuration from version %s.%s",
+        config_entry.version,
+        config_entry.minor_version,
+    )
+
+    if config_entry.version > 2:
+        return False
+
+    if config_entry.version == 1:
+        # Migrate to version 2
+        counter_id = config_entry.data.get(CONF_COUNTER_ID)
+        unique_id = str(counter_id)
+
+        hass.config_entries.async_update_entry(
+            config_entry,
+            unique_id=unique_id,
+            version=2,
+        )
+
+    _LOGGER.debug(
+        "Migration to configuration version %s.%s successful",
+        config_entry.version,
+        config_entry.minor_version,
+    )
+
+    return True

homeassistant/components/suez_water/config_flow.py

@@ -55,16 +55,15 @@ async def validate_input(data: dict[str, Any]) -> None:
 class SuezWaterConfigFlow(ConfigFlow, domain=DOMAIN):
     """Handle a config flow for Suez Water."""
 
-    VERSION = 1
+    VERSION = 2
 
     async def async_step_user(
         self, user_input: dict[str, Any] | None = None
     ) -> ConfigFlowResult:
-        """Handle the initial step."""
+        """Handle the initial setup step."""
         errors: dict[str, str] = {}
+
         if user_input is not None:
-            await self.async_set_unique_id(user_input[CONF_USERNAME])
-            self._abort_if_unique_id_configured()
             try:
                 await validate_input(user_input)
             except CannotConnect:
@@ -77,9 +76,10 @@ class SuezWaterConfigFlow(ConfigFlow, domain=DOMAIN):
                 _LOGGER.exception("Unexpected exception")
                 errors["base"] = "unknown"
             else:
-                return self.async_create_entry(
-                    title=user_input[CONF_USERNAME], data=user_input
-                )
+                counter_id = str(user_input[CONF_COUNTER_ID])
+                await self.async_set_unique_id(counter_id)
+                self._abort_if_unique_id_configured()
+                return self.async_create_entry(title=counter_id, data=user_input)
 
         return self.async_show_form(
             step_id="user",
@@ -98,4 +98,4 @@ class InvalidAuth(HomeAssistantError):
 
 
 class CounterNotFound(HomeAssistantError):
-    """Error to indicate we cannot automatically found the counter id."""
+    """Error to indicate we failed to automatically find the counter id."""

tests/components/suez_water/conftest.py

@@ -8,14 +8,14 @@ from pysuez import AggregatedData, PriceResult
 from pysuez.const import ATTRIBUTION
 import pytest
 
-from homeassistant.components.suez_water.const import DOMAIN
+from homeassistant.components.suez_water.const import CONF_COUNTER_ID, DOMAIN
 
 from tests.common import MockConfigEntry
 
 MOCK_DATA = {
     "username": "test-username",
     "password": "test-password",
-    "counter_id": "test-counter",
+    CONF_COUNTER_ID: "test-counter",
 }
 
 
@@ -23,10 +23,11 @@ MOCK_DATA = {
 def mock_config_entry() -> MockConfigEntry:
     """Create mock config_entry needed by suez_water integration."""
     return MockConfigEntry(
-        unique_id=MOCK_DATA["username"],
+        unique_id=MOCK_DATA[CONF_COUNTER_ID],
         domain=DOMAIN,
         title="Suez mock device",
         data=MOCK_DATA,
+        version=2,
     )
 
 

tests/components/suez_water/test_config_flow.py

@@ -32,8 +32,8 @@ async def test_form(
     await hass.async_block_till_done()
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == "test-username"
-    assert result["result"].unique_id == "test-username"
+    assert result["title"] == MOCK_DATA[CONF_COUNTER_ID]
+    assert result["result"].unique_id == MOCK_DATA[CONF_COUNTER_ID]
     assert result["data"] == MOCK_DATA
     assert len(mock_setup_entry.mock_calls) == 1
 
@@ -63,18 +63,20 @@ async def test_form_invalid_auth(
     await hass.async_block_till_done()
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == "test-username"
-    assert result["result"].unique_id == "test-username"
+    assert result["title"] == MOCK_DATA[CONF_COUNTER_ID]
+    assert result["result"].unique_id == MOCK_DATA[CONF_COUNTER_ID]
     assert result["data"] == MOCK_DATA
     assert len(mock_setup_entry.mock_calls) == 1
 
 
-async def test_form_already_configured(hass: HomeAssistant) -> None:
+async def test_form_already_configured(
+    hass: HomeAssistant, suez_client: AsyncMock
+) -> None:
     """Test we abort when entry is already configured."""
 
     entry = MockConfigEntry(
         domain=DOMAIN,
-        unique_id="test-username",
+        unique_id=MOCK_DATA[CONF_COUNTER_ID],
         data=MOCK_DATA,
     )
     entry.add_to_hass(hass)
@@ -124,7 +126,7 @@ async def test_form_error(
     )
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == "test-username"
+    assert result["title"] == MOCK_DATA[CONF_COUNTER_ID]
     assert result["data"] == MOCK_DATA
     assert len(mock_setup_entry.mock_calls) == 1
 
@@ -139,7 +141,7 @@ async def test_form_auto_counter(
     assert result["type"] is FlowResultType.FORM
     assert result["errors"] == {}
 
-    partial_form = {**MOCK_DATA}
+    partial_form = MOCK_DATA.copy()
     partial_form.pop(CONF_COUNTER_ID)
     suez_client.find_counter.side_effect = PySuezError("test counter not found")
 
@@ -160,7 +162,7 @@ async def test_form_auto_counter(
     await hass.async_block_till_done()
 
     assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == "test-username"
-    assert result["result"].unique_id == "test-username"
+    assert result["title"] == MOCK_DATA[CONF_COUNTER_ID]
+    assert result["result"].unique_id == MOCK_DATA[CONF_COUNTER_ID]
     assert result["data"] == MOCK_DATA
     assert len(mock_setup_entry.mock_calls) == 1

tests/components/suez_water/test_init.py

@@ -2,11 +2,14 @@
 
 from unittest.mock import AsyncMock
 
+from homeassistant.components.suez_water.const import CONF_COUNTER_ID, DOMAIN
 from homeassistant.components.suez_water.coordinator import PySuezError
 from homeassistant.config_entries import ConfigEntryState
+from homeassistant.const import CONF_USERNAME
 from homeassistant.core import HomeAssistant
 
 from . import setup_integration
+from .conftest import MOCK_DATA
 
 from tests.common import MockConfigEntry
 
@@ -35,3 +38,56 @@ async def test_initialization_setup_api_error(
     await setup_integration(hass, mock_config_entry)
 
     assert mock_config_entry.state is ConfigEntryState.SETUP_RETRY
+
+
+async def test_migration_version_rollback(
+    hass: HomeAssistant,
+    suez_client: AsyncMock,
+) -> None:
+    """Test that downgrading from a future version is not possible."""
+    future_entry = MockConfigEntry(
+        unique_id=MOCK_DATA[CONF_COUNTER_ID],
+        domain=DOMAIN,
+        title="Suez mock device",
+        data=MOCK_DATA,
+        version=3,
+    )
+    await setup_integration(hass, future_entry)
+    assert future_entry.state is ConfigEntryState.MIGRATION_ERROR
+
+
+async def test_no_migration_current_version(
+    hass: HomeAssistant,
+    suez_client: AsyncMock,
+) -> None:
+    """Test that a current version does not migrate."""
+    current_entry = MockConfigEntry(
+        unique_id=MOCK_DATA[CONF_COUNTER_ID],
+        domain=DOMAIN,
+        title="Suez mock device",
+        data=MOCK_DATA,
+        version=2,
+    )
+    await setup_integration(hass, current_entry)
+    assert current_entry.state is ConfigEntryState.LOADED
+    assert current_entry.unique_id == MOCK_DATA[CONF_COUNTER_ID]
+
+
+async def test_migration_version_1_to_2(
+    hass: HomeAssistant,
+    suez_client: AsyncMock,
+) -> None:
+    """Test that a migration from 1 to 2 changes the unique_id."""
+    past_entry = MockConfigEntry(
+        unique_id=MOCK_DATA[CONF_USERNAME],
+        domain=DOMAIN,
+        title=MOCK_DATA[CONF_USERNAME],
+        data=MOCK_DATA,
+        version=1,
+    )
+
+    await setup_integration(hass, past_entry)
+    assert past_entry.state is ConfigEntryState.LOADED
+    assert past_entry.unique_id == MOCK_DATA[CONF_COUNTER_ID]
+    assert past_entry.title == MOCK_DATA[CONF_USERNAME]
+    assert past_entry.version == 2