From 463c2e8d45bf89199ea2cb64881cc38e37f14627 Mon Sep 17 00:00:00 2001 From: jjlawren Date: Mon, 7 Oct 2019 13:29:12 -0500 Subject: [PATCH] Remove manual config flow step (#27291) --- homeassistant/components/plex/config_flow.py | 59 +----- homeassistant/components/plex/strings.json | 18 +- tests/components/plex/test_config_flow.py | 188 ++++++------------- 3 files changed, 66 insertions(+), 199 deletions(-) diff --git a/homeassistant/components/plex/config_flow.py b/homeassistant/components/plex/config_flow.py index dd5401950e9..38727ccff06 100644 --- a/homeassistant/components/plex/config_flow.py +++ b/homeassistant/components/plex/config_flow.py @@ -12,14 +12,7 @@ from homeassistant.components.http.view import HomeAssistantView from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant import config_entries from homeassistant.components.media_player import DOMAIN as MP_DOMAIN -from homeassistant.const import ( - CONF_HOST, - CONF_PORT, - CONF_URL, - CONF_TOKEN, - CONF_SSL, - CONF_VERIFY_SSL, -) +from homeassistant.const import CONF_URL, CONF_TOKEN, CONF_SSL, CONF_VERIFY_SSL from homeassistant.core import callback from homeassistant.util.json import load_json @@ -30,8 +23,6 @@ from .const import ( # pylint: disable=unused-import CONF_SERVER_IDENTIFIER, CONF_USE_EPISODE_ART, CONF_SHOW_ALL_CONTROLS, - DEFAULT_PORT, - DEFAULT_SSL, DEFAULT_VERIFY_SSL, DOMAIN, PLEX_CONFIG_FILE, @@ -44,8 +35,6 @@ from .const import ( # pylint: disable=unused-import from .errors import NoServersFound, ServerNotSpecified from .server import PlexServer -USER_SCHEMA = vol.Schema({vol.Optional("manual_setup"): bool}) - _LOGGER = logging.getLogger(__package__) @@ -73,23 +62,17 @@ class PlexFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): def __init__(self): """Initialize the Plex flow.""" self.current_login = {} - self.discovery_info = {} self.available_servers = None self.plexauth = None self.token = None async def async_step_user(self, user_input=None): """Handle a flow initialized by the user.""" - errors = {} - if user_input is not None: - if user_input.pop("manual_setup", False): - return await self.async_step_manual_setup(user_input) + return self.async_show_form(step_id="start_website_auth") - return await self.async_step_plex_website_auth() - - return self.async_show_form( - step_id="user", data_schema=USER_SCHEMA, errors=errors - ) + async def async_step_start_website_auth(self, user_input=None): + """Show a form before starting external authentication.""" + return await self.async_step_plex_website_auth() async def async_step_server_validate(self, server_config): """Validate a provided configuration.""" @@ -120,9 +103,7 @@ class PlexFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): return self.async_abort(reason="unknown") if errors: - return self.async_show_form( - step_id="user", data_schema=USER_SCHEMA, errors=errors - ) + return self.async_show_form(step_id="start_website_auth", errors=errors) server_id = plex_server.machine_identifier @@ -152,30 +133,6 @@ class PlexFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): }, ) - async def async_step_manual_setup(self, user_input=None): - """Begin manual configuration.""" - if len(user_input) > 1: - host = user_input.pop(CONF_HOST) - port = user_input.pop(CONF_PORT) - prefix = "https" if user_input.pop(CONF_SSL) else "http" - user_input[CONF_URL] = f"{prefix}://{host}:{port}" - return await self.async_step_server_validate(user_input) - - data_schema = vol.Schema( - { - vol.Required( - CONF_HOST, default=self.discovery_info.get(CONF_HOST) - ): str, - vol.Required( - CONF_PORT, default=self.discovery_info.get(CONF_PORT, DEFAULT_PORT) - ): int, - vol.Optional(CONF_SSL, default=DEFAULT_SSL): bool, - vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): bool, - vol.Optional(CONF_TOKEN, default=user_input.get(CONF_TOKEN, "")): str, - } - ) - return self.async_show_form(step_id="manual_setup", data_schema=data_schema) - async def async_step_select_server(self, user_input=None): """Use selected Plex server.""" config = dict(self.current_login) @@ -210,8 +167,6 @@ class PlexFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): # Skip discovery if a config already exists or is in progress. return self.async_abort(reason="already_configured") - discovery_info[CONF_PORT] = int(discovery_info[CONF_PORT]) - self.discovery_info = discovery_info json_file = self.hass.config.path(PLEX_CONFIG_FILE) file_config = await self.hass.async_add_executor_job(load_json, json_file) @@ -227,7 +182,7 @@ class PlexFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): _LOGGER.info("Imported legacy config, file can be removed: %s", json_file) return await self.async_step_server_validate(server_config) - return await self.async_step_user() + return self.async_abort(reason="discovery_no_file") async def async_step_import(self, import_config): """Import from Plex configuration.""" diff --git a/homeassistant/components/plex/strings.json b/homeassistant/components/plex/strings.json index 6538d8e887e..aff79acc2ed 100644 --- a/homeassistant/components/plex/strings.json +++ b/homeassistant/components/plex/strings.json @@ -2,16 +2,6 @@ "config": { "title": "Plex", "step": { - "manual_setup": { - "title": "Plex server", - "data": { - "host": "Host", - "port": "Port", - "ssl": "Use SSL", - "verify_ssl": "Verify SSL certificate", - "token": "Token (if required)" - } - }, "select_server": { "title": "Select Plex server", "description": "Multiple servers available, select one:", @@ -19,12 +9,9 @@ "server": "Server" } }, - "user": { + "start_website_auth": { "title": "Connect Plex server", - "description": "Continue to authorize at plex.tv or manually configure a server.", - "data": { - "manual_setup": "Manual setup" - } + "description": "Continue to authorize at plex.tv." } }, "error": { @@ -36,6 +23,7 @@ "all_configured": "All linked servers already configured", "already_configured": "This Plex server is already configured", "already_in_progress": "Plex is being configured", + "discovery_no_file": "No legacy config file found", "invalid_import": "Imported configuration is invalid", "token_request_timeout": "Timed out obtaining token", "unknown": "Failed for unknown reason" diff --git a/tests/components/plex/test_config_flow.py b/tests/components/plex/test_config_flow.py index 753d565a82b..e9f48f6a4f8 100644 --- a/tests/components/plex/test_config_flow.py +++ b/tests/components/plex/test_config_flow.py @@ -6,14 +6,7 @@ import plexapi.exceptions import requests.exceptions from homeassistant.components.plex import config_flow -from homeassistant.const import ( - CONF_HOST, - CONF_PORT, - CONF_SSL, - CONF_VERIFY_SSL, - CONF_TOKEN, - CONF_URL, -) +from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TOKEN, CONF_URL from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry @@ -48,34 +41,32 @@ def init_config_flow(hass): async def test_bad_credentials(hass): """Test when provided credentials are rejected.""" + mock_connections = MockConnections() + mm_plex_account = MagicMock() + mm_plex_account.resources = Mock(return_value=[MOCK_SERVER_1]) + mm_plex_account.resource = Mock(return_value=mock_connections) - result = await hass.config_entries.flow.async_init( - config_flow.DOMAIN, context={"source": "user"} - ) - assert result["type"] == "form" - assert result["step_id"] == "user" - - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": True} - ) - assert result["type"] == "form" - assert result["step_id"] == "manual_setup" - - with patch( + with patch("plexapi.myplex.MyPlexAccount", return_value=mm_plex_account), patch( "plexapi.server.PlexServer", side_effect=plexapi.exceptions.Unauthorized + ), asynctest.patch("plexauth.PlexAuth.initiate_auth"), asynctest.patch( + "plexauth.PlexAuth.token", return_value="BAD TOKEN" ): - result = await hass.config_entries.flow.async_configure( - result["flow_id"], - user_input={ - CONF_HOST: MOCK_HOST_1, - CONF_PORT: MOCK_PORT_1, - CONF_SSL: False, - CONF_VERIFY_SSL: False, - CONF_TOKEN: "BAD TOKEN", - }, + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" + + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == "external" + + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == "external_done" + + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + + assert result["type"] == "form" + assert result["step_id"] == "start_website_auth" assert result["errors"]["base"] == "faulty_credentials" @@ -123,8 +114,8 @@ async def test_discovery(hass): context={"source": "discovery"}, data={CONF_HOST: MOCK_HOST_1, CONF_PORT: MOCK_PORT_1}, ) - assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["type"] == "abort" + assert result["reason"] == "discovery_no_file" async def test_discovery_while_in_progress(hass): @@ -201,7 +192,7 @@ async def test_import_bad_hostname(hass): }, ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" assert result["errors"]["base"] == "not_found" @@ -212,26 +203,25 @@ async def test_unknown_exception(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": True} - ) - assert result["type"] == "form" - assert result["step_id"] == "manual_setup" + mock_connections = MockConnections() + mm_plex_account = MagicMock() + mm_plex_account.resources = Mock(return_value=[MOCK_SERVER_1]) + mm_plex_account.resource = Mock(return_value=mock_connections) - with patch("plexapi.server.PlexServer", side_effect=Exception): - result = await hass.config_entries.flow.async_configure( - result["flow_id"], - user_input={ - CONF_HOST: MOCK_HOST_1, - CONF_PORT: MOCK_PORT_1, - CONF_SSL: True, - CONF_VERIFY_SSL: True, - CONF_TOKEN: MOCK_TOKEN, - }, - ) + with patch("plexapi.myplex.MyPlexAccount", return_value=mm_plex_account), patch( + "plexapi.server.PlexServer", side_effect=Exception + ), asynctest.patch("plexauth.PlexAuth.initiate_auth"), asynctest.patch( + "plexauth.PlexAuth.token", return_value="MOCK_TOKEN" + ): + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == "external" + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == "external_done" + + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "abort" assert result["reason"] == "unknown" @@ -245,7 +235,7 @@ async def test_no_servers_found(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" mm_plex_account = MagicMock() mm_plex_account.resources = Mock(return_value=[]) @@ -256,9 +246,7 @@ async def test_no_servers_found(hass): "plexauth.PlexAuth.token", return_value=MOCK_TOKEN ): - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" result = await hass.config_entries.flow.async_configure(result["flow_id"]) @@ -266,7 +254,7 @@ async def test_no_servers_found(hass): result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" assert result["errors"]["base"] == "no_servers" @@ -279,7 +267,7 @@ async def test_single_available_server(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" mock_connections = MockConnections() @@ -304,9 +292,7 @@ async def test_single_available_server(hass): mock_plex_server.return_value )._baseurl = PropertyMock(return_value=mock_connections.connections[0].httpuri) - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" result = await hass.config_entries.flow.async_configure(result["flow_id"]) @@ -336,7 +322,7 @@ async def test_multiple_servers_with_selection(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" mock_connections = MockConnections() mm_plex_account = MagicMock() @@ -360,9 +346,7 @@ async def test_multiple_servers_with_selection(hass): mock_plex_server.return_value )._baseurl = PropertyMock(return_value=mock_connections.connections[0].httpuri) - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" result = await hass.config_entries.flow.async_configure(result["flow_id"]) @@ -406,7 +390,7 @@ async def test_adding_last_unconfigured_server(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" mock_connections = MockConnections() mm_plex_account = MagicMock() @@ -430,9 +414,7 @@ async def test_adding_last_unconfigured_server(hass): mock_plex_server.return_value )._baseurl = PropertyMock(return_value=mock_connections.connections[0].httpuri) - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" result = await hass.config_entries.flow.async_configure(result["flow_id"]) @@ -512,7 +494,7 @@ async def test_all_available_servers_configured(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" mock_connections = MockConnections() mm_plex_account = MagicMock() @@ -525,9 +507,7 @@ async def test_all_available_servers_configured(hass): "plexauth.PlexAuth.token", return_value=MOCK_TOKEN ): - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" result = await hass.config_entries.flow.async_configure(result["flow_id"]) @@ -538,58 +518,6 @@ async def test_all_available_servers_configured(hass): assert result["reason"] == "all_configured" -async def test_manual_config(hass): - """Test creating via manual configuration.""" - - result = await hass.config_entries.flow.async_init( - config_flow.DOMAIN, context={"source": "user"} - ) - assert result["type"] == "form" - assert result["step_id"] == "user" - - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": True} - ) - assert result["type"] == "form" - assert result["step_id"] == "manual_setup" - - mock_connections = MockConnections(ssl=True) - - with patch("plexapi.server.PlexServer") as mock_plex_server: - type(mock_plex_server.return_value).machineIdentifier = PropertyMock( - return_value=MOCK_SERVER_1.clientIdentifier - ) - type(mock_plex_server.return_value).friendlyName = PropertyMock( - return_value=MOCK_SERVER_1.name - ) - type( # pylint: disable=protected-access - mock_plex_server.return_value - )._baseurl = PropertyMock(return_value=mock_connections.connections[0].httpuri) - - result = await hass.config_entries.flow.async_configure( - result["flow_id"], - user_input={ - CONF_HOST: MOCK_HOST_1, - CONF_PORT: MOCK_PORT_1, - CONF_SSL: True, - CONF_VERIFY_SSL: True, - CONF_TOKEN: MOCK_TOKEN, - }, - ) - assert result["type"] == "create_entry" - assert result["title"] == MOCK_SERVER_1.name - assert result["data"][config_flow.CONF_SERVER] == MOCK_SERVER_1.name - assert ( - result["data"][config_flow.CONF_SERVER_IDENTIFIER] - == MOCK_SERVER_1.clientIdentifier - ) - assert ( - result["data"][config_flow.PLEX_SERVER_CONFIG][CONF_URL] - == mock_connections.connections[0].httpuri - ) - assert result["data"][config_flow.PLEX_SERVER_CONFIG][CONF_TOKEN] == MOCK_TOKEN - - async def test_option_flow(hass): """Test config flow selection of one of two bridges.""" @@ -627,15 +555,13 @@ async def test_external_timed_out(hass): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" with asynctest.patch("plexauth.PlexAuth.initiate_auth"), asynctest.patch( "plexauth.PlexAuth.token", return_value=None ): - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" result = await hass.config_entries.flow.async_configure(result["flow_id"]) @@ -655,14 +581,12 @@ async def test_callback_view(hass, aiohttp_client): config_flow.DOMAIN, context={"source": "user"} ) assert result["type"] == "form" - assert result["step_id"] == "user" + assert result["step_id"] == "start_website_auth" with asynctest.patch("plexauth.PlexAuth.initiate_auth"), asynctest.patch( "plexauth.PlexAuth.token", return_value=MOCK_TOKEN ): - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={"manual_setup": False} - ) + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] == "external" client = await aiohttp_client(hass.http.app)