From e58bd62c68ee86a2d13254174ecea082d212573e Mon Sep 17 00:00:00 2001 From: jb101010-2 <168106462+jb101010-2@users.noreply.github.com> Date: Sat, 28 Dec 2024 16:25:10 +0100 Subject: [PATCH] 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 --- .../components/suez_water/__init__.py | 38 +++++++++++++ .../components/suez_water/config_flow.py | 16 +++--- tests/components/suez_water/conftest.py | 7 ++- .../components/suez_water/test_config_flow.py | 22 ++++---- tests/components/suez_water/test_init.py | 56 +++++++++++++++++++ 5 files changed, 118 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/suez_water/__init__.py b/homeassistant/components/suez_water/__init__.py index cbaac912642..30f8c030c26 100644 --- a/homeassistant/components/suez_water/__init__.py +++ b/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 diff --git a/homeassistant/components/suez_water/config_flow.py b/homeassistant/components/suez_water/config_flow.py index b24dc1815ee..fb8bc2988d6 100644 --- a/homeassistant/components/suez_water/config_flow.py +++ b/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.""" diff --git a/tests/components/suez_water/conftest.py b/tests/components/suez_water/conftest.py index b034d9b00fa..73557fd3bde 100644 --- a/tests/components/suez_water/conftest.py +++ b/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, ) diff --git a/tests/components/suez_water/test_config_flow.py b/tests/components/suez_water/test_config_flow.py index 6779b4c7d02..bebb4fd72ac 100644 --- a/tests/components/suez_water/test_config_flow.py +++ b/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 diff --git a/tests/components/suez_water/test_init.py b/tests/components/suez_water/test_init.py index 78d086af38f..16d32b61dee 100644 --- a/tests/components/suez_water/test_init.py +++ b/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