From 49c4886f40743c939d4468e11685056dc9c8445d Mon Sep 17 00:00:00 2001 From: Austin Mroczek Date: Mon, 15 Nov 2021 09:32:35 -0800 Subject: [PATCH] Fix totalconnect config flow (#59461) * update total_connect_client to 2021.10 * update for total_connect_client changes * remove unused return value * bump total_connect_client to 2021.11.1 * bump total_connect_client to 2021.11.2 * Move to public ResultCode * load locations to prevent 'unknown error occurred' * add test for zero locations * put error message in strings * test for abort and message from strings * handle AuthenticationError in step_user * update tests with exceptions * update reauth with exceptions * use try except else per suggestion * only create schema if necessary * catch auth error in async_setup_entry * one more fix in test_init --- .../components/totalconnect/__init__.py | 11 ++--- .../components/totalconnect/config_flow.py | 37 ++++++++++------- .../components/totalconnect/strings.json | 3 +- .../totalconnect/test_config_flow.py | 41 +++++++++++++++---- tests/components/totalconnect/test_init.py | 5 ++- 5 files changed, 67 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/totalconnect/__init__.py b/homeassistant/components/totalconnect/__init__.py index 8acc7801de8..dcbc1592814 100644 --- a/homeassistant/components/totalconnect/__init__.py +++ b/homeassistant/components/totalconnect/__init__.py @@ -34,12 +34,13 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: temp_codes = conf[CONF_USERCODES] usercodes = {int(code): temp_codes[code] for code in temp_codes} - client = await hass.async_add_executor_job( - TotalConnectClient, username, password, usercodes - ) - if not client.is_logged_in(): - raise ConfigEntryAuthFailed("TotalConnect authentication failed") + try: + client = await hass.async_add_executor_job( + TotalConnectClient, username, password, usercodes + ) + except AuthenticationError as exception: + raise ConfigEntryAuthFailed("TotalConnect authentication failed") from exception coordinator = TotalConnectDataUpdateCoordinator(hass, client) await coordinator.async_config_entry_first_refresh() diff --git a/homeassistant/components/totalconnect/config_flow.py b/homeassistant/components/totalconnect/config_flow.py index f3550722de5..b529bdd80fd 100644 --- a/homeassistant/components/totalconnect/config_flow.py +++ b/homeassistant/components/totalconnect/config_flow.py @@ -1,5 +1,6 @@ """Config flow for the Total Connect component.""" from total_connect_client.client import TotalConnectClient +from total_connect_client.exceptions import AuthenticationError import voluptuous as vol from homeassistant import config_entries @@ -36,18 +37,18 @@ class TotalConnectConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): await self.async_set_unique_id(username) self._abort_if_unique_id_configured() - client = await self.hass.async_add_executor_job( - TotalConnectClient, username, password, None - ) - - if client.is_logged_in(): + try: + client = await self.hass.async_add_executor_job( + TotalConnectClient, username, password, None + ) + except AuthenticationError: + errors["base"] = "invalid_auth" + else: # username/password valid so show user locations self.username = username self.password = password self.client = client return await self.async_step_locations() - # authentication failed / invalid - errors["base"] = "invalid_auth" data_schema = vol.Schema( {vol.Required(CONF_USERNAME): str, vol.Required(CONF_PASSWORD): str} @@ -88,6 +89,12 @@ class TotalConnectConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): }, ) else: + # Force the loading of locations using I/O + number_locations = await self.hass.async_add_executor_job( + self.client.get_number_locations, + ) + if number_locations < 1: + return self.async_abort(reason="no_locations") for location_id in self.client.locations: self.usercodes[location_id] = None @@ -129,14 +136,14 @@ class TotalConnectConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): data_schema=PASSWORD_DATA_SCHEMA, ) - client = await self.hass.async_add_executor_job( - TotalConnectClient, - self.username, - user_input[CONF_PASSWORD], - self.usercodes, - ) - - if not client.is_logged_in(): + try: + await self.hass.async_add_executor_job( + TotalConnectClient, + self.username, + user_input[CONF_PASSWORD], + self.usercodes, + ) + except AuthenticationError: errors["base"] = "invalid_auth" return self.async_show_form( step_id="reauth_confirm", diff --git a/homeassistant/components/totalconnect/strings.json b/homeassistant/components/totalconnect/strings.json index 5c32d19b348..63505c2446c 100644 --- a/homeassistant/components/totalconnect/strings.json +++ b/homeassistant/components/totalconnect/strings.json @@ -26,7 +26,8 @@ }, "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_account%]", - "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", + "no_locations": "No locations are available for this user, check TotalConnect settings" } } } diff --git a/tests/components/totalconnect/test_config_flow.py b/tests/components/totalconnect/test_config_flow.py index a9debb26dd4..631553a4af4 100644 --- a/tests/components/totalconnect/test_config_flow.py +++ b/tests/components/totalconnect/test_config_flow.py @@ -1,6 +1,8 @@ """Tests for the TotalConnect config flow.""" from unittest.mock import patch +from total_connect_client.exceptions import AuthenticationError + from homeassistant import data_entry_flow from homeassistant.components.totalconnect.const import CONF_USERCODES, DOMAIN from homeassistant.config_entries import SOURCE_REAUTH, SOURCE_USER @@ -92,10 +94,7 @@ async def test_abort_if_already_setup(hass): ).add_to_hass(hass) # Should fail, same USERNAME (flow) - with patch( - "homeassistant.components.totalconnect.config_flow.TotalConnectClient" - ) as client_mock: - client_mock.return_value.is_logged_in.return_value = True + with patch("homeassistant.components.totalconnect.config_flow.TotalConnectClient"): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, @@ -111,7 +110,7 @@ async def test_login_failed(hass): with patch( "homeassistant.components.totalconnect.config_flow.TotalConnectClient" ) as client_mock: - client_mock.return_value.is_logged_in.return_value = False + client_mock.side_effect = AuthenticationError() result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, @@ -143,7 +142,7 @@ async def test_reauth(hass): "homeassistant.components.totalconnect.async_setup_entry", return_value=True ): # first test with an invalid password - client_mock.return_value.is_logged_in.return_value = False + client_mock.side_effect = AuthenticationError() result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_PASSWORD: "password"} @@ -153,7 +152,7 @@ async def test_reauth(hass): assert result["errors"] == {"base": "invalid_auth"} # now test with the password valid - client_mock.return_value.is_logged_in.return_value = True + client_mock.side_effect = None result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_PASSWORD: "password"} @@ -163,3 +162,31 @@ async def test_reauth(hass): await hass.async_block_till_done() assert len(hass.config_entries.async_entries()) == 1 + + +async def test_no_locations(hass): + """Test with no user locations.""" + responses = [ + RESPONSE_AUTHENTICATE, + RESPONSE_PARTITION_DETAILS, + RESPONSE_GET_ZONE_DETAILS_SUCCESS, + RESPONSE_DISARMED, + ] + + with patch(TOTALCONNECT_REQUEST, side_effect=responses,) as mock_request, patch( + "homeassistant.components.totalconnect.async_setup_entry", return_value=True + ), patch( + "homeassistant.components.totalconnect.TotalConnectClient.get_number_locations", + return_value=0, + ): + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data=CONFIG_DATA_NO_USERCODES, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "no_locations" + await hass.async_block_till_done() + + assert mock_request.call_count == 1 diff --git a/tests/components/totalconnect/test_init.py b/tests/components/totalconnect/test_init.py index f1797f840ab..4c8c61f7d99 100644 --- a/tests/components/totalconnect/test_init.py +++ b/tests/components/totalconnect/test_init.py @@ -1,6 +1,8 @@ """Tests for the TotalConnect init process.""" from unittest.mock import patch +from total_connect_client.exceptions import AuthenticationError + from homeassistant.components.totalconnect.const import DOMAIN from homeassistant.config_entries import ConfigEntryState from homeassistant.setup import async_setup_component @@ -20,9 +22,8 @@ async def test_reauth_started(hass): with patch( "homeassistant.components.totalconnect.TotalConnectClient", - autospec=True, ) as mock_client: - mock_client.return_value.is_logged_in.return_value = False + mock_client.side_effect = AuthenticationError() assert await async_setup_component(hass, DOMAIN, {}) await hass.async_block_till_done()