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 <joostlek@outlook.com>

---------

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
This commit is contained in:
Sid 2024-04-17 21:08:10 +02:00 committed by GitHub
parent 8275512130
commit cdc49328be
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 72 additions and 35 deletions

View file

@ -8,7 +8,6 @@ from openwebif.error import InvalidAuthError
import voluptuous as vol import voluptuous as vol
from yarl import URL from yarl import URL
from homeassistant.components.homeassistant import DOMAIN as HOMEASSISTANT_DOMAIN
from homeassistant.config_entries import SOURCE_USER, ConfigFlow, ConfigFlowResult from homeassistant.config_entries import SOURCE_USER, ConfigFlow, ConfigFlowResult
from homeassistant.const import ( from homeassistant.const import (
CONF_HOST, CONF_HOST,
@ -18,6 +17,7 @@ from homeassistant.const import (
CONF_USERNAME, CONF_USERNAME,
CONF_VERIFY_SSL, CONF_VERIFY_SSL,
) )
from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN
from homeassistant.helpers import selector from homeassistant.helpers import selector
from homeassistant.helpers.aiohttp_client import async_create_clientsession from homeassistant.helpers.aiohttp_client import async_create_clientsession
from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue 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) OPTIONS_KEYS = (CONF_DEEP_STANDBY, CONF_SOURCE_BOUQUET, CONF_USE_CHANNEL_ICON)
def __init__(self) -> None: async def validate_user_input(
"""Initialize the config flow.""" self, user_input: dict[str, Any]
super().__init__() ) -> dict[str, str] | None:
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]:
"""Validate user input.""" """Validate user input."""
self.errors = {} errors = None
self._async_abort_entries_match({CONF_HOST: user_input[CONF_HOST]}) self._async_abort_entries_match({CONF_HOST: user_input[CONF_HOST]})
@ -97,16 +92,16 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
try: try:
about = await OpenWebIfDevice(session).get_about() about = await OpenWebIfDevice(session).get_about()
except InvalidAuthError: except InvalidAuthError:
self.errors["base"] = "invalid_auth" errors = {"base": "invalid_auth"}
except ClientError: except ClientError:
self.errors["base"] = "cannot_connect" errors = {"base": "cannot_connect"}
except Exception: # pylint: disable=broad-except except Exception: # pylint: disable=broad-except
self.errors["base"] = "unknown" errors = {"base": "unknown"}
else: else:
await self.async_set_unique_id(about["info"]["ifaces"][0]["mac"]) await self.async_set_unique_id(about["info"]["ifaces"][0]["mac"])
self._abort_if_unique_id_configured() self._abort_if_unique_id_configured()
return user_input return errors
async def async_step_user( async def async_step_user(
self, user_input: dict[str, Any] | None = None self, user_input: dict[str, Any] | None = None
@ -115,23 +110,41 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
if user_input is None: if user_input is None:
return self.async_show_form(step_id=SOURCE_USER, data_schema=CONFIG_SCHEMA) return self.async_show_form(step_id=SOURCE_USER, data_schema=CONFIG_SCHEMA)
data = await self.validate_user_input(user_input) if errors := await self.validate_user_input(user_input):
if "base" in self.errors:
return self.async_show_form( 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( return self.async_create_entry(data=user_input, title=user_input[CONF_HOST])
data=data, title=data[CONF_HOST], options=self._options
)
async def async_step_import(self, user_input: dict[str, Any]) -> ConfigFlowResult: 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: if CONF_PORT not in user_input:
user_input[CONF_PORT] = DEFAULT_PORT user_input[CONF_PORT] = DEFAULT_PORT
if CONF_SSL not in user_input: if CONF_SSL not in user_input:
user_input[CONF_SSL] = DEFAULT_SSL user_input[CONF_SSL] = DEFAULT_SSL
user_input[CONF_VERIFY_SSL] = DEFAULT_VERIFY_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( async_create_issue(
self.hass, self.hass,
HOMEASSISTANT_DOMAIN, HOMEASSISTANT_DOMAIN,
@ -147,12 +160,6 @@ class Enigma2ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
"integration_title": "Enigma2", "integration_title": "Enigma2",
}, },
) )
return self.async_create_entry(
self._data = { data=data, title=data[CONF_HOST], options=options
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)

View file

@ -10,7 +10,6 @@
"ssl": "[%key:common::config_flow::data::ssl%]", "ssl": "[%key:common::config_flow::data::ssl%]",
"username": "[%key:common::config_flow::data::username%]", "username": "[%key:common::config_flow::data::username%]",
"password": "[%key:common::config_flow::data::password%]", "password": "[%key:common::config_flow::data::password%]",
"name": "[%key:common::config_flow::data::name%]",
"verify_ssl": "[%key:common::config_flow::data::verify_ssl%]" "verify_ssl": "[%key:common::config_flow::data::verify_ssl%]"
} }
} }
@ -26,5 +25,19 @@
"cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
"unknown": "[%key:common::config_flow::error::unknown%]" "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."
}
} }
} }

View file

@ -10,8 +10,9 @@ import pytest
from homeassistant import config_entries from homeassistant import config_entries
from homeassistant.components.enigma2.const import DOMAIN from homeassistant.components.enigma2.const import DOMAIN
from homeassistant.const import CONF_HOST 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.data_entry_flow import FlowResultType
from homeassistant.helpers.issue_registry import IssueRegistry
from .conftest import ( from .conftest import (
EXPECTED_OPTIONS, EXPECTED_OPTIONS,
@ -96,6 +97,7 @@ async def test_form_import(
test_config: dict[str, Any], test_config: dict[str, Any],
expected_data: dict[str, Any], expected_data: dict[str, Any],
expected_options: dict[str, Any], expected_options: dict[str, Any],
issue_registry: IssueRegistry,
) -> None: ) -> None:
"""Test we get the form with import source.""" """Test we get the form with import source."""
with ( with (
@ -115,6 +117,12 @@ async def test_form_import(
) )
await hass.async_block_till_done() 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["type"] == FlowResultType.CREATE_ENTRY
assert result["title"] == test_config[CONF_HOST] assert result["title"] == test_config[CONF_HOST]
assert result["data"] == expected_data assert result["data"] == expected_data
@ -132,7 +140,10 @@ async def test_form_import(
], ],
) )
async def test_form_import_errors( async def test_form_import_errors(
hass: HomeAssistant, exception: Exception, error_type: str hass: HomeAssistant,
exception: Exception,
error_type: str,
issue_registry: IssueRegistry,
) -> None: ) -> None:
"""Test we handle errors on import.""" """Test we handle errors on import."""
with patch( with patch(
@ -145,5 +156,11 @@ async def test_form_import_errors(
data=TEST_IMPORT_FULL, data=TEST_IMPORT_FULL,
) )
assert result["type"] == FlowResultType.FORM issue = issue_registry.async_get_issue(
assert result["errors"] == {"base": error_type} 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