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 <[email protected]> * Fix --------- Co-authored-by: Martin Hjelmare <[email protected]>
home-assistant · Apr 5, 2024 · ed3daed · ed3daed
1 parent 5d5dc24
commit ed3daed
Show file tree

Hide file tree

Showing 5 changed files with 125 additions and 32 deletions.
diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py
@@ -11,7 +11,11 @@
 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
@@ -46,12 +46,16 @@ async def async_step_user(
             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
@@ -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
@@ -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
@@ -8,7 +8,8 @@
     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