From 69f8de2c5959c26d960d0698ca534132ceb8a0c2 Mon Sep 17 00:00:00 2001 From: Joost Lekkerkerker Date: Fri, 5 Apr 2024 12:32:09 +0200 Subject: [PATCH] Create right import issues in Downloader (#114922) * Create right import issues in Downloader * Create right import issues in Downloader * Create right import issues in Downloader * Create right import issues in Downloader * Fix * Fix * Fix * Fix * Apply suggestions from code review Co-authored-by: Martin Hjelmare * Fix --------- Co-authored-by: Martin Hjelmare --- .../components/downloader/__init__.py | 57 +++++++++++------ .../components/downloader/config_flow.py | 12 ++-- .../components/downloader/strings.json | 8 +-- .../components/downloader/test_config_flow.py | 16 +++++ tests/components/downloader/test_init.py | 64 ++++++++++++++++++- 5 files changed, 125 insertions(+), 32 deletions(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index d110c28785a..3fded1215c4 100644 --- a/homeassistant/components/downloader/__init__.py +++ b/homeassistant/components/downloader/__init__.py @@ -11,7 +11,11 @@ import requests import voluptuous as vol from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry -from homeassistant.core import HomeAssistant, ServiceCall +from homeassistant.core import ( + DOMAIN as HOMEASSISTANT_DOMAIN, + HomeAssistant, + ServiceCall, +) from homeassistant.data_entry_flow import FlowResultType import homeassistant.helpers.config_validation as cv from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue @@ -43,7 +47,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: if DOMAIN not in config: return True - hass.async_create_task(_async_import_config(hass, config), eager_start=True) + hass.async_create_task(_async_import_config(hass, config)) return True @@ -58,27 +62,40 @@ async def _async_import_config(hass: HomeAssistant, config: ConfigType) -> None: }, ) - translation_key = "deprecated_yaml" if ( import_result["type"] == FlowResultType.ABORT - and import_result["reason"] == "import_failed" + and import_result["reason"] != "single_instance_allowed" ): - translation_key = "import_failed" - - async_create_issue( - hass, - DOMAIN, - f"deprecated_yaml_{DOMAIN}", - breaks_in_ha_version="2024.10.0", - is_fixable=False, - issue_domain=DOMAIN, - severity=IssueSeverity.WARNING, - translation_key=translation_key, - translation_placeholders={ - "domain": DOMAIN, - "integration_title": "Downloader", - }, - ) + async_create_issue( + hass, + DOMAIN, + f"deprecated_yaml_{DOMAIN}", + breaks_in_ha_version="2024.10.0", + is_fixable=False, + issue_domain=DOMAIN, + severity=IssueSeverity.WARNING, + translation_key="directory_does_not_exist", + translation_placeholders={ + "domain": DOMAIN, + "integration_title": "Downloader", + "url": "/config/integrations/dashboard/add?domain=downloader", + }, + ) + else: + async_create_issue( + hass, + HOMEASSISTANT_DOMAIN, + f"deprecated_yaml_{DOMAIN}", + breaks_in_ha_version="2024.10.0", + is_fixable=False, + issue_domain=DOMAIN, + severity=IssueSeverity.WARNING, + translation_key="deprecated_yaml", + translation_placeholders={ + "domain": DOMAIN, + "integration_title": "Downloader", + }, + ) async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: diff --git a/homeassistant/components/downloader/config_flow.py b/homeassistant/components/downloader/config_flow.py index 15af8b56163..94b33f4e93f 100644 --- a/homeassistant/components/downloader/config_flow.py +++ b/homeassistant/components/downloader/config_flow.py @@ -46,12 +46,16 @@ class DownloaderConfigFlow(ConfigFlow, domain=DOMAIN): errors=errors, ) - async def async_step_import( - self, user_input: dict[str, Any] | None = None - ) -> ConfigFlowResult: + async def async_step_import(self, user_input: dict[str, Any]) -> ConfigFlowResult: """Handle a flow initiated by configuration file.""" + if self._async_current_entries(): + return self.async_abort(reason="single_instance_allowed") - return await self.async_step_user(user_input) + try: + await self._validate_input(user_input) + except DirectoryDoesNotExist: + return self.async_abort(reason="directory_does_not_exist") + return self.async_create_entry(title=DEFAULT_NAME, data=user_input) async def _validate_input(self, user_input: dict[str, Any]) -> None: """Validate the user input if the directory exists.""" diff --git a/homeassistant/components/downloader/strings.json b/homeassistant/components/downloader/strings.json index 77dd0abd9d3..4cadabf96c6 100644 --- a/homeassistant/components/downloader/strings.json +++ b/homeassistant/components/downloader/strings.json @@ -37,13 +37,9 @@ } }, "issues": { - "deprecated_yaml": { - "title": "The {integration_title} YAML configuration is being removed", - "description": "Configuring {integration_title} using YAML is being removed.\n\nYour configuration is already imported.\n\nRemove the `{domain}` configuration from your configuration.yaml file and restart Home Assistant to fix this issue." - }, - "import_failed": { + "directory_does_not_exist": { "title": "The {integration_title} failed to import", - "description": "The {integration_title} integration failed to import.\n\nPlease check the logs for more details." + "description": "The {integration_title} integration failed to import because the configured directory does not exist.\n\nEnsure the directory exists and restart Home Assistant to try again or remove the {integration_title} configuration from your configuration.yaml file and continue to [set up the integration]({url}) manually." } } } diff --git a/tests/components/downloader/test_config_flow.py b/tests/components/downloader/test_config_flow.py index 45c2302b605..b561fae98e9 100644 --- a/tests/components/downloader/test_config_flow.py +++ b/tests/components/downloader/test_config_flow.py @@ -99,3 +99,19 @@ async def test_import_flow_success(hass: HomeAssistant) -> None: assert result["title"] == "Downloader" assert result["data"] == {} assert result["options"] == {} + + +async def test_import_flow_directory_not_found(hass: HomeAssistant) -> None: + """Test import flow.""" + with patch("os.path.isdir", return_value=False): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data={ + CONF_DOWNLOAD_DIR: "download_dir", + }, + ) + await hass.async_block_till_done() + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "directory_does_not_exist" diff --git a/tests/components/downloader/test_init.py b/tests/components/downloader/test_init.py index 8cd0d00b1ab..5832c0402b4 100644 --- a/tests/components/downloader/test_init.py +++ b/tests/components/downloader/test_init.py @@ -8,7 +8,8 @@ from homeassistant.components.downloader import ( SERVICE_DOWNLOAD_FILE, ) from homeassistant.config_entries import ConfigEntryState -from homeassistant.core import HomeAssistant +from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant +from homeassistant.helpers import issue_registry as ir from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry @@ -30,7 +31,7 @@ async def test_initialization(hass: HomeAssistant) -> None: assert config_entry.state is ConfigEntryState.LOADED -async def test_import(hass: HomeAssistant) -> None: +async def test_import(hass: HomeAssistant, issue_registry: ir.IssueRegistry) -> None: """Test the import of the downloader component.""" with patch("os.path.isdir", return_value=True): assert await async_setup_component( @@ -49,3 +50,62 @@ async def test_import(hass: HomeAssistant) -> None: assert config_entry.data == {CONF_DOWNLOAD_DIR: "/test_dir"} assert config_entry.state is ConfigEntryState.LOADED assert hass.services.has_service(DOMAIN, SERVICE_DOWNLOAD_FILE) + assert len(issue_registry.issues) == 1 + issue = issue_registry.async_get_issue( + issue_id="deprecated_yaml_downloader", domain=HOMEASSISTANT_DOMAIN + ) + assert issue + + +async def test_import_directory_missing( + hass: HomeAssistant, issue_registry: ir.IssueRegistry +) -> None: + """Test the import of the downloader component.""" + with patch("os.path.isdir", return_value=False): + assert await async_setup_component( + hass, + DOMAIN, + { + DOMAIN: { + CONF_DOWNLOAD_DIR: "/test_dir", + }, + }, + ) + await hass.async_block_till_done() + + assert len(hass.config_entries.async_entries(DOMAIN)) == 0 + assert len(issue_registry.issues) == 1 + issue = issue_registry.async_get_issue( + issue_id="deprecated_yaml_downloader", domain=DOMAIN + ) + assert issue + + +async def test_import_already_exists( + hass: HomeAssistant, issue_registry: ir.IssueRegistry +) -> None: + """Test the import of the downloader component.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_DOWNLOAD_DIR: "/test_dir", + }, + ) + config_entry.add_to_hass(hass) + with patch("os.path.isdir", return_value=True): + assert await async_setup_component( + hass, + DOMAIN, + { + DOMAIN: { + CONF_DOWNLOAD_DIR: "/test_dir", + }, + }, + ) + await hass.async_block_till_done() + + assert len(issue_registry.issues) == 1 + issue = issue_registry.async_get_issue( + issue_id="deprecated_yaml_downloader", domain=HOMEASSISTANT_DOMAIN + ) + assert issue