From cdc49328be0520258ddd314f10e63d0ef69a8d9c Mon Sep 17 00:00:00 2001 From: Sid <27780930+autinerd@users.noreply.github.com> Date: Wed, 17 Apr 2024 21:08:10 +0200 Subject: [PATCH] Address late reviews for the enigma2 config flow (#115768) * Address late reviews for the enigma2 config flow * fix tests * review comments * test for issues * Apply suggestions from code review Co-authored-by: Joost Lekkerkerker --------- Co-authored-by: Joost Lekkerkerker --- .../components/enigma2/config_flow.py | 67 ++++++++++--------- homeassistant/components/enigma2/strings.json | 15 ++++- tests/components/enigma2/test_config_flow.py | 25 +++++-- 3 files changed, 72 insertions(+), 35 deletions(-) diff --git a/homeassistant/components/enigma2/config_flow.py b/homeassistant/components/enigma2/config_flow.py index c144f2b7dae..ac57bd9d0fa 100644 --- a/homeassistant/components/enigma2/config_flow.py +++ b/homeassistant/components/enigma2/config_flow.py @@ -8,7 +8,6 @@ from openwebif.error import InvalidAuthError import voluptuous as vol from yarl import URL -from homeassistant.components.homeassistant import DOMAIN as HOMEASSISTANT_DOMAIN from homeassistant.config_entries import SOURCE_USER, ConfigFlow, ConfigFlowResult from homeassistant.const import ( CONF_HOST, @@ -18,6 +17,7 @@ from homeassistant.const import ( CONF_USERNAME, CONF_VERIFY_SSL, ) +from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN from homeassistant.helpers import selector from homeassistant.helpers.aiohttp_client import async_create_clientsession from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue @@ -68,17 +68,12 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN): ) OPTIONS_KEYS = (CONF_DEEP_STANDBY, CONF_SOURCE_BOUQUET, CONF_USE_CHANNEL_ICON) - def __init__(self) -> None: - """Initialize the config flow.""" - super().__init__() - self.errors: dict[str, str] = {} - self._data: dict[str, Any] = {} - self._options: dict[str, Any] = {} - - async def validate_user_input(self, user_input: dict[str, Any]) -> dict[str, Any]: + async def validate_user_input( + self, user_input: dict[str, Any] + ) -> dict[str, str] | None: """Validate user input.""" - self.errors = {} + errors = None self._async_abort_entries_match({CONF_HOST: user_input[CONF_HOST]}) @@ -97,16 +92,16 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN): try: about = await OpenWebIfDevice(session).get_about() except InvalidAuthError: - self.errors["base"] = "invalid_auth" + errors = {"base": "invalid_auth"} except ClientError: - self.errors["base"] = "cannot_connect" + errors = {"base": "cannot_connect"} except Exception: # pylint: disable=broad-except - self.errors["base"] = "unknown" + errors = {"base": "unknown"} else: await self.async_set_unique_id(about["info"]["ifaces"][0]["mac"]) self._abort_if_unique_id_configured() - return user_input + return errors async def async_step_user( self, user_input: dict[str, Any] | None = None @@ -115,23 +110,41 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN): if user_input is None: return self.async_show_form(step_id=SOURCE_USER, data_schema=CONFIG_SCHEMA) - data = await self.validate_user_input(user_input) - if "base" in self.errors: + if errors := await self.validate_user_input(user_input): return self.async_show_form( - step_id=SOURCE_USER, data_schema=CONFIG_SCHEMA, errors=self.errors + step_id=SOURCE_USER, data_schema=CONFIG_SCHEMA, errors=errors ) - return self.async_create_entry( - data=data, title=data[CONF_HOST], options=self._options - ) + return self.async_create_entry(data=user_input, title=user_input[CONF_HOST]) async def async_step_import(self, user_input: dict[str, Any]) -> ConfigFlowResult: - """Validate import.""" + """Handle the import step.""" if CONF_PORT not in user_input: user_input[CONF_PORT] = DEFAULT_PORT if CONF_SSL not in user_input: user_input[CONF_SSL] = DEFAULT_SSL user_input[CONF_VERIFY_SSL] = DEFAULT_VERIFY_SSL + data = {key: user_input[key] for key in user_input if key in self.DATA_KEYS} + options = { + key: user_input[key] for key in user_input if key in self.OPTIONS_KEYS + } + + if errors := await self.validate_user_input(user_input): + async_create_issue( + self.hass, + DOMAIN, + f"deprecated_yaml_{DOMAIN}_import_issue_{errors["base"]}", + breaks_in_ha_version="2024.11.0", + is_fixable=False, + issue_domain=DOMAIN, + severity=IssueSeverity.WARNING, + translation_key=f"deprecated_yaml_import_issue_{errors["base"]}", + translation_placeholders={ + "url": "/config/integrations/dashboard/add?domain=enigma2" + }, + ) + return self.async_abort(reason=errors["base"]) + async_create_issue( self.hass, HOMEASSISTANT_DOMAIN, @@ -147,12 +160,6 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN): "integration_title": "Enigma2", }, ) - - self._data = { - key: user_input[key] for key in user_input if key in self.DATA_KEYS - } - self._options = { - key: user_input[key] for key in user_input if key in self.OPTIONS_KEYS - } - - return await self.async_step_user(self._data) + return self.async_create_entry( + data=data, title=data[CONF_HOST], options=options + ) diff --git a/homeassistant/components/enigma2/strings.json b/homeassistant/components/enigma2/strings.json index 888c6d59387..ddeb59ea6d5 100644 --- a/homeassistant/components/enigma2/strings.json +++ b/homeassistant/components/enigma2/strings.json @@ -10,7 +10,6 @@ "ssl": "[%key:common::config_flow::data::ssl%]", "username": "[%key:common::config_flow::data::username%]", "password": "[%key:common::config_flow::data::password%]", - "name": "[%key:common::config_flow::data::name%]", "verify_ssl": "[%key:common::config_flow::data::verify_ssl%]" } } @@ -26,5 +25,19 @@ "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", "unknown": "[%key:common::config_flow::error::unknown%]" } + }, + "issues": { + "deprecated_yaml_import_issue_unknown": { + "title": "The Enigma2 YAML configuration import failed", + "description": "Configuring Enigma2 using YAML is being removed but there was an error importing your YAML configuration.\n\nEnsure connection to the device works, the authentication details are correct and restart Home Assistant to try again or remove the Enigma2 YAML configuration from your configuration.yaml file and continue to [set up the integration]({url}) manually." + }, + "deprecated_yaml_import_issue_invalid_auth": { + "title": "The Enigma2 YAML configuration import failed", + "description": "Configuring Enigma2 using YAML is being removed but there was an error importing your YAML configuration.\n\nEnsure the authentication details are correct and restart Home Assistant to try again or remove the Enigma2 YAML configuration from your configuration.yaml file and continue to [set up the integration]({url}) manually." + }, + "deprecated_yaml_import_issue_cannot_connect": { + "title": "The Enigma2 YAML configuration import failed", + "description": "Configuring Enigma2 using YAML is being removed but there was an error importing your YAML configuration.\n\nEnsure connection to the device works and restart Home Assistant to try again or remove the Enigma2 YAML configuration from your configuration.yaml file and continue to [set up the integration]({url}) manually." + } } } diff --git a/tests/components/enigma2/test_config_flow.py b/tests/components/enigma2/test_config_flow.py index dcd249ad943..dfca569276d 100644 --- a/tests/components/enigma2/test_config_flow.py +++ b/tests/components/enigma2/test_config_flow.py @@ -10,8 +10,9 @@ import pytest from homeassistant import config_entries from homeassistant.components.enigma2.const import DOMAIN from homeassistant.const import CONF_HOST -from homeassistant.core import HomeAssistant +from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from homeassistant.helpers.issue_registry import IssueRegistry from .conftest import ( EXPECTED_OPTIONS, @@ -96,6 +97,7 @@ async def test_form_import( test_config: dict[str, Any], expected_data: dict[str, Any], expected_options: dict[str, Any], + issue_registry: IssueRegistry, ) -> None: """Test we get the form with import source.""" with ( @@ -115,6 +117,12 @@ async def test_form_import( ) await hass.async_block_till_done() + issue = issue_registry.async_get_issue( + HOMEASSISTANT_DOMAIN, f"deprecated_yaml_{DOMAIN}" + ) + + assert issue + assert issue.issue_domain == DOMAIN assert result["type"] == FlowResultType.CREATE_ENTRY assert result["title"] == test_config[CONF_HOST] assert result["data"] == expected_data @@ -132,7 +140,10 @@ async def test_form_import( ], ) async def test_form_import_errors( - hass: HomeAssistant, exception: Exception, error_type: str + hass: HomeAssistant, + exception: Exception, + error_type: str, + issue_registry: IssueRegistry, ) -> None: """Test we handle errors on import.""" with patch( @@ -145,5 +156,11 @@ async def test_form_import_errors( data=TEST_IMPORT_FULL, ) - assert result["type"] == FlowResultType.FORM - assert result["errors"] == {"base": error_type} + issue = issue_registry.async_get_issue( + DOMAIN, f"deprecated_yaml_{DOMAIN}_import_issue_{error_type}" + ) + + assert issue + assert issue.issue_domain == DOMAIN + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == error_type