From b3bb4015c261425d7cd96aa0fee99e0c49fede64 Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 10:33:30 +0200 Subject: [PATCH 01/10] Create right import issues in Downloader --- .../components/downloader/__init__.py | 31 ++++++++++++++----- .../components/downloader/config_flow.py | 13 +++++--- .../components/downloader/strings.json | 8 ++--- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index 3ca503a2167b6c..ca74bccaf86c47 100644 --- 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 @@ -51,22 +55,35 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: }, ) - 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.9.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", + }, + ) async_create_issue( hass, - DOMAIN, + HOMEASSISTANT_DOMAIN, f"deprecated_yaml_{DOMAIN}", - breaks_in_ha_version="2024.9.0", + breaks_in_ha_version="2024.10.0", is_fixable=False, issue_domain=DOMAIN, severity=IssueSeverity.WARNING, - translation_key=translation_key, + translation_key="deprecated_yaml", translation_placeholders={ "domain": DOMAIN, "integration_title": "Downloader", diff --git a/homeassistant/components/downloader/config_flow.py b/homeassistant/components/downloader/config_flow.py index 15af8b56163bf7..6ab2718ac25e7f 100644 --- a/homeassistant/components/downloader/config_flow.py +++ b/homeassistant/components/downloader/config_flow.py @@ -46,12 +46,17 @@ 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") + else: + 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 77dd0abd9d32a0..ab3d63e6f388c6 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 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." } } } From 86f0ff1d23c5b57035d687d5519cba160afd7fc3 Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 10:35:11 +0200 Subject: [PATCH 02/10] Create right import issues in Downloader --- homeassistant/components/downloader/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index ca74bccaf86c47..067e9aa458aa52 100644 --- a/homeassistant/components/downloader/__init__.py +++ b/homeassistant/components/downloader/__init__.py @@ -63,7 +63,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: hass, DOMAIN, f"deprecated_yaml_{DOMAIN}", - breaks_in_ha_version="2024.9.0", + breaks_in_ha_version="2024.10.0", is_fixable=False, issue_domain=DOMAIN, severity=IssueSeverity.WARNING, From c160f765e493aa2dcb034d6b37dbf3dab059e851 Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 10:33:30 +0200 Subject: [PATCH 03/10] Create right import issues in Downloader --- .../components/downloader/__init__.py | 29 +++++++++++++++---- .../components/downloader/config_flow.py | 13 ++++++--- .../components/downloader/strings.json | 8 ++--- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index d110c28785aa30..43549dce493be1 100644 --- 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 @@ -58,22 +62,35 @@ 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.9.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", + }, + ) async_create_issue( hass, - DOMAIN, + HOMEASSISTANT_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_key="deprecated_yaml", translation_placeholders={ "domain": DOMAIN, "integration_title": "Downloader", diff --git a/homeassistant/components/downloader/config_flow.py b/homeassistant/components/downloader/config_flow.py index 15af8b56163bf7..6ab2718ac25e7f 100644 --- a/homeassistant/components/downloader/config_flow.py +++ b/homeassistant/components/downloader/config_flow.py @@ -46,12 +46,17 @@ 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") + else: + 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 77dd0abd9d32a0..ab3d63e6f388c6 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 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." } } } From bdbdf0cd567f47620b38987cb0a0659ac91d9dbc Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 10:35:11 +0200 Subject: [PATCH 04/10] Create right import issues in Downloader --- homeassistant/components/downloader/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index 43549dce493be1..c508e5969033c2 100644 --- a/homeassistant/components/downloader/__init__.py +++ b/homeassistant/components/downloader/__init__.py @@ -70,7 +70,7 @@ async def _async_import_config(hass: HomeAssistant, config: ConfigType) -> None: hass, DOMAIN, f"deprecated_yaml_{DOMAIN}", - breaks_in_ha_version="2024.9.0", + breaks_in_ha_version="2024.10.0", is_fixable=False, issue_domain=DOMAIN, severity=IssueSeverity.WARNING, From 447ec23831fe651c05bba9faf32cb7f5a30ae69e Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 11:35:42 +0200 Subject: [PATCH 05/10] Fix --- tests/components/downloader/test_init.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/components/downloader/test_init.py b/tests/components/downloader/test_init.py index 8cd0d00b1abcfa..c329ed53d47caa 100644 --- a/tests/components/downloader/test_init.py +++ b/tests/components/downloader/test_init.py @@ -9,6 +9,7 @@ ) from homeassistant.config_entries import ConfigEntryState from homeassistant.core import 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,4 @@ 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 From 2d69fb71a9c7f53fc0a850071fe000d6c284a5e9 Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 11:47:43 +0200 Subject: [PATCH 06/10] Fix --- .../components/downloader/__init__.py | 2 +- .../components/downloader/test_config_flow.py | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index c508e5969033c2..99a0b71f6736d5 100644 --- a/homeassistant/components/downloader/__init__.py +++ b/homeassistant/components/downloader/__init__.py @@ -47,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 diff --git a/tests/components/downloader/test_config_flow.py b/tests/components/downloader/test_config_flow.py index 45c2302b605e20..3c714e06b558b2 100644 --- a/tests/components/downloader/test_config_flow.py +++ b/tests/components/downloader/test_config_flow.py @@ -99,3 +99,24 @@ 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" From 3c7e6b1a22bdd194b13ac167b0d0db9f21639f89 Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 11:48:23 +0200 Subject: [PATCH 07/10] Fix --- tests/components/downloader/test_config_flow.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/components/downloader/test_config_flow.py b/tests/components/downloader/test_config_flow.py index 3c714e06b558b2..3924716dd0c860 100644 --- a/tests/components/downloader/test_config_flow.py +++ b/tests/components/downloader/test_config_flow.py @@ -103,11 +103,9 @@ async def test_import_flow_success(hass: HomeAssistant) -> None: async def test_import_flow_directory_not_found(hass: HomeAssistant) -> None: """Test import flow.""" - with ( - patch( - "os.path.isdir", - return_value=False, - ), + with patch( + "os.path.isdir", + return_value=False, ): result = await hass.config_entries.flow.async_init( DOMAIN, From ad7a3f44d76978740c4674f6441f55361c321b1b Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 11:48:31 +0200 Subject: [PATCH 08/10] Fix --- tests/components/downloader/test_config_flow.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/components/downloader/test_config_flow.py b/tests/components/downloader/test_config_flow.py index 3924716dd0c860..b561fae98e9a09 100644 --- a/tests/components/downloader/test_config_flow.py +++ b/tests/components/downloader/test_config_flow.py @@ -103,10 +103,7 @@ async def test_import_flow_success(hass: HomeAssistant) -> None: async def test_import_flow_directory_not_found(hass: HomeAssistant) -> None: """Test import flow.""" - with patch( - "os.path.isdir", - return_value=False, - ): + with patch("os.path.isdir", return_value=False): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, From adbee868c674c6394bf842e5e3136d3762140314 Mon Sep 17 00:00:00 2001 From: Joost Lekkerkerker Date: Fri, 5 Apr 2024 12:20:15 +0200 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Martin Hjelmare --- homeassistant/components/downloader/config_flow.py | 3 +-- homeassistant/components/downloader/strings.json | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/downloader/config_flow.py b/homeassistant/components/downloader/config_flow.py index 6ab2718ac25e7f..94b33f4e93fa58 100644 --- a/homeassistant/components/downloader/config_flow.py +++ b/homeassistant/components/downloader/config_flow.py @@ -55,8 +55,7 @@ async def async_step_import(self, user_input: dict[str, Any]) -> ConfigFlowResul await self._validate_input(user_input) except DirectoryDoesNotExist: return self.async_abort(reason="directory_does_not_exist") - else: - return self.async_create_entry(title=DEFAULT_NAME, data=user_input) + 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 ab3d63e6f388c6..4cadabf96c6783 100644 --- a/homeassistant/components/downloader/strings.json +++ b/homeassistant/components/downloader/strings.json @@ -39,7 +39,7 @@ "issues": { "directory_does_not_exist": { "title": "The {integration_title} failed to import", - "description": "The {integration_title} integration failed to import because the 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." + "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." } } } From efc55d9f63ff6bc2205876a5379021fec39e96fd Mon Sep 17 00:00:00 2001 From: Joostlek Date: Fri, 5 Apr 2024 12:25:31 +0200 Subject: [PATCH 10/10] Fix --- .../components/downloader/__init__.py | 30 +++++----- tests/components/downloader/test_init.py | 60 ++++++++++++++++++- 2 files changed, 74 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/downloader/__init__.py b/homeassistant/components/downloader/__init__.py index 99a0b71f6736d5..3fded1215c48ca 100644 --- a/homeassistant/components/downloader/__init__.py +++ b/homeassistant/components/downloader/__init__.py @@ -81,21 +81,21 @@ async def _async_import_config(hass: HomeAssistant, config: ConfigType) -> None: "url": "/config/integrations/dashboard/add?domain=downloader", }, ) - - 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", - }, - ) + 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/tests/components/downloader/test_init.py b/tests/components/downloader/test_init.py index c329ed53d47caa..5832c0402b4207 100644 --- a/tests/components/downloader/test_init.py +++ b/tests/components/downloader/test_init.py @@ -8,7 +8,7 @@ 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 @@ -51,3 +51,61 @@ async def test_import(hass: HomeAssistant, issue_registry: ir.IssueRegistry) -> 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