Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create right import issues in Downloader #114922

Merged
merged 12 commits into from
Apr 5, 2024
57 changes: 37 additions & 20 deletions homeassistant/components/downloader/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand All @@ -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:
Expand Down
12 changes: 8 additions & 4 deletions homeassistant/components/downloader/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
8 changes: 2 additions & 6 deletions homeassistant/components/downloader/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
}
}
}
16 changes: 16 additions & 0 deletions tests/components/downloader/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
64 changes: 62 additions & 2 deletions tests/components/downloader/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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
MartinHjelmare marked this conversation as resolved.
Show resolved Hide resolved
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