From 789055fd68551dd1866f356100a7edb5da0cb1f6 Mon Sep 17 00:00:00 2001 From: Isak Nyberg <36712644+IsakNyberg@users.noreply.github.com> Date: Mon, 29 Jan 2024 12:48:55 +0100 Subject: [PATCH] Fix Permobil eula error (#107290) * bump mypermobil to 0.1.8 * add eula check in config flow * Update strings.json * add test for email code with signed eula * fix docstring character limit * add placeholder description for MyPermobil --- .../components/permobil/config_flow.py | 15 ++- .../components/permobil/manifest.json | 2 +- .../components/permobil/strings.json | 3 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/permobil/test_config_flow.py | 93 ++++++++++++++++++- 6 files changed, 106 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/permobil/config_flow.py b/homeassistant/components/permobil/config_flow.py index 644ea29d8a3..2e3e228d512 100644 --- a/homeassistant/components/permobil/config_flow.py +++ b/homeassistant/components/permobil/config_flow.py @@ -5,7 +5,12 @@ from collections.abc import Mapping import logging from typing import Any -from mypermobil import MyPermobil, MyPermobilAPIException, MyPermobilClientException +from mypermobil import ( + MyPermobil, + MyPermobilAPIException, + MyPermobilClientException, + MyPermobilEulaException, +) import voluptuous as vol from homeassistant import config_entries @@ -141,10 +146,16 @@ class PermobilConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): # or the backend returned an error when trying to validate the code _LOGGER.exception("Error verifying code") errors["base"] = "invalid_code" + except MyPermobilEulaException: + # The user has not accepted the EULA + errors["base"] = "unsigned_eula" if errors or not user_input: return self.async_show_form( - step_id="email_code", data_schema=GET_TOKEN_SCHEMA, errors=errors + step_id="email_code", + data_schema=GET_TOKEN_SCHEMA, + errors=errors, + description_placeholders={"app_name": "MyPermobil"}, ) return self.async_create_entry(title=self.data[CONF_EMAIL], data=self.data) diff --git a/homeassistant/components/permobil/manifest.json b/homeassistant/components/permobil/manifest.json index fd937fc6f8a..2d136b28713 100644 --- a/homeassistant/components/permobil/manifest.json +++ b/homeassistant/components/permobil/manifest.json @@ -5,5 +5,5 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/permobil", "iot_class": "cloud_polling", - "requirements": ["mypermobil==0.1.6"] + "requirements": ["mypermobil==0.1.8"] } diff --git a/homeassistant/components/permobil/strings.json b/homeassistant/components/permobil/strings.json index b500bbdb9ea..5070c13d9e5 100644 --- a/homeassistant/components/permobil/strings.json +++ b/homeassistant/components/permobil/strings.json @@ -27,7 +27,8 @@ "region_fetch_error": "Error fetching regions", "code_request_error": "Error requesting application code", "invalid_email": "Invalid email", - "invalid_code": "The code you gave is incorrect" + "invalid_code": "The code you gave is incorrect", + "unsigned_eula": "Please sign the EULA in the {app_name} app" } }, "entity": { diff --git a/requirements_all.txt b/requirements_all.txt index fd3cca21d83..b50feb91260 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1322,7 +1322,7 @@ mutagen==1.47.0 mutesync==0.0.1 # homeassistant.components.permobil -mypermobil==0.1.6 +mypermobil==0.1.8 # homeassistant.components.myuplink myuplink==0.0.9 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index f0f282cfcf1..57e69672dd1 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1055,7 +1055,7 @@ mutagen==1.47.0 mutesync==0.0.1 # homeassistant.components.permobil -mypermobil==0.1.6 +mypermobil==0.1.8 # homeassistant.components.myuplink myuplink==0.0.9 diff --git a/tests/components/permobil/test_config_flow.py b/tests/components/permobil/test_config_flow.py index ad61ead7bfc..0f303cc0482 100644 --- a/tests/components/permobil/test_config_flow.py +++ b/tests/components/permobil/test_config_flow.py @@ -1,7 +1,11 @@ """Test the MyPermobil config flow.""" from unittest.mock import Mock, patch -from mypermobil import MyPermobilAPIException, MyPermobilClientException +from mypermobil import ( + MyPermobilAPIException, + MyPermobilClientException, + MyPermobilEulaException, +) import pytest from homeassistant import config_entries @@ -67,7 +71,11 @@ async def test_sucessful_config_flow(hass: HomeAssistant, my_permobil: Mock) -> async def test_config_flow_incorrect_code( hass: HomeAssistant, my_permobil: Mock ) -> None: - """Test the config flow from start to until email code verification and have the API return error.""" + """Test email code verification with API error. + + Test the config flow from start to until email code verification + and have the API return API error. + """ my_permobil.request_application_token.side_effect = MyPermobilAPIException # init flow with patch( @@ -105,10 +113,75 @@ async def test_config_flow_incorrect_code( assert result["errors"]["base"] == "invalid_code" +async def test_config_flow_unsigned_eula( + hass: HomeAssistant, my_permobil: Mock +) -> None: + """Test email code verification with unsigned eula error. + + Test the config flow from start to until email code verification + and have the API return that the eula is unsigned. + """ + my_permobil.request_application_token.side_effect = MyPermobilEulaException + # init flow + with patch( + "homeassistant.components.permobil.config_flow.MyPermobil", + return_value=my_permobil, + ): + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={"source": config_entries.SOURCE_USER}, + data={CONF_EMAIL: MOCK_EMAIL}, + ) + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "region" + assert result["errors"] == {} + + # select region step + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={CONF_REGION: MOCK_REGION_NAME}, + ) + + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "email_code" + assert result["errors"] == {} + + # request region code + # here the request_application_token raises a MyPermobilEulaException + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={CONF_CODE: MOCK_CODE}, + ) + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "email_code" + assert result["errors"]["base"] == "unsigned_eula" + + # Retry to submit the code again, but this time the user has signed the EULA + with patch.object( + my_permobil, + "request_application_token", + return_value=MOCK_TOKEN, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={CONF_CODE: MOCK_CODE}, + ) + + # Now the method should not raise an exception, and you can proceed with your assertions + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"] == VALID_DATA + + async def test_config_flow_incorrect_region( hass: HomeAssistant, my_permobil: Mock ) -> None: - """Test the config flow from start to until the request for email code and have the API return error.""" + """Test when the user does not exist in the selected region. + + Test the config flow from start to until the request for email + code and have the API return error because there is not user for + that email. + """ my_permobil.request_application_code.side_effect = MyPermobilAPIException # init flow with patch( @@ -140,7 +213,11 @@ async def test_config_flow_incorrect_region( async def test_config_flow_region_request_error( hass: HomeAssistant, my_permobil: Mock ) -> None: - """Test the config flow from start to until the request for regions and have the API return error.""" + """Test region request error. + + Test the config flow from start to until the request for regions + and have the API return an error. + """ my_permobil.request_region_names.side_effect = MyPermobilAPIException # init flow # here the request_region_names raises a MyPermobilAPIException @@ -162,7 +239,13 @@ async def test_config_flow_region_request_error( async def test_config_flow_invalid_email( hass: HomeAssistant, my_permobil: Mock ) -> None: - """Test the config flow from start to until the request for regions and have the API return error.""" + """Test an incorrectly formatted email. + + Test that the email must be formatted correctly. The schema for the + input should already check for this, but since the API does a + separate check that might not overlap 100% with the schema, + this test is still needed. + """ my_permobil.set_email.side_effect = MyPermobilClientException() # init flow # here the set_email raises a MyPermobilClientException