From cdfb43d4035724ab58c7d62a83abc76029876182 Mon Sep 17 00:00:00 2001 From: Thijs Walcarius Date: Mon, 13 Mar 2023 15:06:45 +0100 Subject: [PATCH] Address late review comments for frontier_silicon config flow (#89507) Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> Co-authored-by: wlcrs --- .../frontier_silicon/config_flow.py | 62 +++++++++---------- .../components/frontier_silicon/strings.json | 2 - .../frontier_silicon/test_config_flow.py | 40 +++++++++++- 3 files changed, 67 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/frontier_silicon/config_flow.py b/homeassistant/components/frontier_silicon/config_flow.py index 5e9472de62e..a3fbdb52c1c 100644 --- a/homeassistant/components/frontier_silicon/config_flow.py +++ b/homeassistant/components/frontier_silicon/config_flow.py @@ -37,19 +37,14 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): VERSION = 1 - def __init__(self) -> None: - """Initialize flow.""" - - self._webfsapi_url: str | None = None - self._name: str | None = None - self._unique_id: str | None = None + _webfsapi_url: str async def async_step_import(self, import_info: dict[str, Any]) -> FlowResult: """Handle the import of legacy configuration.yaml entries.""" device_url = f"http://{import_info[CONF_HOST]}:{import_info[CONF_PORT]}/device" try: - self._webfsapi_url = await AFSAPI.get_webfsapi_endpoint(device_url) + webfsapi_url = await AFSAPI.get_webfsapi_endpoint(device_url) except FSConnectionError: return self.async_abort(reason="cannot_connect") except Exception as exception: # pylint: disable=broad-except @@ -57,9 +52,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_abort(reason="unknown") try: - afsapi = AFSAPI(self._webfsapi_url, import_info[CONF_PIN]) + afsapi = AFSAPI(webfsapi_url, import_info[CONF_PIN]) - self._unique_id = await afsapi.get_radio_id() + unique_id = await afsapi.get_radio_id() except FSConnectionError: return self.async_abort(reason="cannot_connect") except InvalidPinException: @@ -68,12 +63,16 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): _LOGGER.exception(exception) return self.async_abort(reason="unknown") - await self.async_set_unique_id(self._unique_id, raise_on_progress=False) + await self.async_set_unique_id(unique_id, raise_on_progress=False) self._abort_if_unique_id_configured() - self._name = import_info[CONF_NAME] or "Radio" - - return await self._create_entry(pin=import_info[CONF_PIN]) + return self.async_create_entry( + title=import_info[CONF_NAME] or "Radio", + data={ + CONF_WEBFSAPI_URL: webfsapi_url, + CONF_PIN: import_info[CONF_PIN], + }, + ) async def async_step_user( self, user_input: dict[str, Any] | None = None @@ -112,18 +111,21 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): # try to login with default pin afsapi = AFSAPI(self._webfsapi_url, DEFAULT_PIN) - self._name = await afsapi.get_friendly_name() + name = await afsapi.get_friendly_name() except InvalidPinException: # Ask for a PIN return await self.async_step_device_config() - self.context["title_placeholders"] = {"name": self._name} + self.context["title_placeholders"] = {"name": name} - self._unique_id = await afsapi.get_radio_id() - await self.async_set_unique_id(self._unique_id) + unique_id = await afsapi.get_radio_id() + await self.async_set_unique_id(unique_id) self._abort_if_unique_id_configured() - return await self._create_entry() + return self.async_create_entry( + title=name, + data={CONF_WEBFSAPI_URL: self._webfsapi_url, CONF_PIN: DEFAULT_PIN}, + ) async def async_step_device_config( self, user_input: dict[str, Any] | None = None @@ -132,7 +134,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): We ask for the PIN in this step. """ - assert self._webfsapi_url is not None if user_input is None: return self.async_show_form( @@ -144,7 +145,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): try: afsapi = AFSAPI(self._webfsapi_url, user_input[CONF_PIN]) - self._name = await afsapi.get_friendly_name() + name = await afsapi.get_friendly_name() except FSConnectionError: errors["base"] = "cannot_connect" @@ -154,10 +155,16 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): _LOGGER.exception(exception) errors["base"] = "unknown" else: - self._unique_id = await afsapi.get_radio_id() - await self.async_set_unique_id(self._unique_id) + unique_id = await afsapi.get_radio_id() + await self.async_set_unique_id(unique_id) self._abort_if_unique_id_configured() - return await self._create_entry(pin=user_input[CONF_PIN]) + return self.async_create_entry( + title=name, + data={ + CONF_WEBFSAPI_URL: self._webfsapi_url, + CONF_PIN: user_input[CONF_PIN], + }, + ) data_schema = self.add_suggested_values_to_schema( STEP_DEVICE_CONFIG_DATA_SCHEMA, user_input @@ -167,12 +174,3 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): data_schema=data_schema, errors=errors, ) - - async def _create_entry(self, pin: str | None = None) -> FlowResult: - """Create the entry.""" - assert self._name is not None - assert self._webfsapi_url is not None - - data = {CONF_WEBFSAPI_URL: self._webfsapi_url, CONF_PIN: pin or DEFAULT_PIN} - - return self.async_create_entry(title=self._name, data=data) diff --git a/homeassistant/components/frontier_silicon/strings.json b/homeassistant/components/frontier_silicon/strings.json index 85b0b6958af..3a0a504761b 100644 --- a/homeassistant/components/frontier_silicon/strings.json +++ b/homeassistant/components/frontier_silicon/strings.json @@ -1,9 +1,7 @@ { "config": { - "flow_title": "{name}", "step": { "user": { - "title": "Frontier Silicon Setup", "data": { "host": "[%key:common::config_flow::data::host%]", "port": "[%key:common::config_flow::data::port%]" diff --git a/tests/components/frontier_silicon/test_config_flow.py b/tests/components/frontier_silicon/test_config_flow.py index a643b121c74..6a61f0b6185 100644 --- a/tests/components/frontier_silicon/test_config_flow.py +++ b/tests/components/frontier_silicon/test_config_flow.py @@ -194,7 +194,10 @@ async def test_form_nondefault_pin( ], ) async def test_form_nondefault_pin_invalid( - hass: HomeAssistant, friendly_name_error: Exception, result_error: str + hass: HomeAssistant, + friendly_name_error: Exception, + result_error: str, + mock_setup_entry: AsyncMock, ) -> None: """Test we get the proper errors when trying to validate an user-provided PIN.""" result = await hass.config_entries.flow.async_init( @@ -232,6 +235,20 @@ async def test_form_nondefault_pin_invalid( assert result2["step_id"] == "device_config" assert result3["errors"] == {"base": result_error} + result4 = await hass.config_entries.flow.async_configure( + result3["flow_id"], + {CONF_PIN: "4321"}, + ) + await hass.async_block_till_done() + + assert result4["type"] == FlowResultType.CREATE_ENTRY + assert result4["title"] == "Name of the device" + assert result4["data"] == { + "webfsapi_url": "http://1.1.1.1:80/webfsapi", + "pin": "4321", + } + mock_setup_entry.assert_called_once() + @pytest.mark.parametrize( ("webfsapi_endpoint_error", "result_error"), @@ -241,9 +258,12 @@ async def test_form_nondefault_pin_invalid( ], ) async def test_invalid_device_url( - hass: HomeAssistant, webfsapi_endpoint_error: Exception, result_error: str + hass: HomeAssistant, + webfsapi_endpoint_error: Exception, + result_error: str, + mock_setup_entry: AsyncMock, ) -> None: - """Test we get the form.""" + """Test flow when the user provides an invalid device IP/hostname.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) @@ -264,3 +284,17 @@ async def test_invalid_device_url( assert result2["type"] == FlowResultType.FORM assert result2["step_id"] == "user" assert result2["errors"] == {"base": result_error} + + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], + {CONF_HOST: "1.1.1.1", CONF_PORT: 80}, + ) + await hass.async_block_till_done() + + assert result3["type"] == FlowResultType.CREATE_ENTRY + assert result3["title"] == "Name of the device" + assert result3["data"] == { + "webfsapi_url": "http://1.1.1.1:80/webfsapi", + "pin": "1234", + } + mock_setup_entry.assert_called_once()