Fix Melcloud import issue (#97673)

* Fix Melcloud import issue * remove old issue * Simplify * Update homeassistant/components/melcloud/strings.json Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com> * Update homeassistant/components/melcloud/strings.json * Update homeassistant/components/melcloud/strings.json * Apply suggestions from code review --------- Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
2025-04-22 16:27:56 +00:00 · 2023-08-05 17:00:33 +02:00 · 2023-08-05 17:00:33 +02:00 · a22aa285d3
commit a22aa285d3
parent 2e263560ec
4 changed files with 174 additions and 25 deletions
--- a/homeassistant/components/melcloud/init.py
+++ b/homeassistant/components/melcloud/init.py
@ -13,13 +13,12 @@ import voluptuous as vol

 from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry
 from homeassistant.const import CONF_TOKEN, CONF_USERNAME, Platform
-from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant
+from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
 from homeassistant.helpers.entity import DeviceInfo
-from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
 from homeassistant.helpers.typing import ConfigType
 from homeassistant.util import Throttle

@ -62,20 +61,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
            data={CONF_USERNAME: username, CONF_TOKEN: token},
        )
    )
-    async_create_issue(
-        hass,
-        HOMEASSISTANT_DOMAIN,
-        f"deprecated_yaml_{DOMAIN}",
-        breaks_in_ha_version="2024.2.0",
-        is_fixable=False,
-        issue_domain=DOMAIN,
-        severity=IssueSeverity.WARNING,
-        translation_key="deprecated_yaml",
-        translation_placeholders={
-            "domain": DOMAIN,
-            "integration_title": "MELCloud",
-        },
-    )
    return True


--- a/homeassistant/components/melcloud/config_flow.py
+++ b/homeassistant/components/melcloud/config_flow.py
@ -11,11 +11,47 @@ import voluptuous as vol

 from homeassistant import config_entries
 from homeassistant.const import CONF_PASSWORD, CONF_TOKEN, CONF_USERNAME
+from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant
+from homeassistant.data_entry_flow import AbortFlow, FlowResultType
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
+from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue

 from .const import DOMAIN


+async def async_create_import_issue(
+    hass: HomeAssistant, source: str, issue: str, success: bool = False
+) -> None:
+    """Create issue from import."""
+    if source != config_entries.SOURCE_IMPORT:
+        return
+    if not success:
+        async_create_issue(
+            hass,
+            DOMAIN,
+            f"deprecated_yaml_import_issue_{issue}",
+            breaks_in_ha_version="2024.2.0",
+            is_fixable=False,
+            severity=IssueSeverity.ERROR,
+            translation_key=f"deprecated_yaml_import_issue_{issue}",
+        )
+        return
+    async_create_issue(
+        hass,
+        HOMEASSISTANT_DOMAIN,
+        f"deprecated_yaml_{DOMAIN}",
+        breaks_in_ha_version="2024.2.0",
+        is_fixable=False,
+        issue_domain=DOMAIN,
+        severity=IssueSeverity.WARNING,
+        translation_key="deprecated_yaml",
+        translation_placeholders={
+            "domain": DOMAIN,
+            "integration_title": "MELCloud",
+        },
+    )
+
+
 class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
    """Handle a config flow."""

@ -24,7 +60,11 @@ class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
    async def _create_entry(self, username: str, token: str):
        """Register new entry."""
        await self.async_set_unique_id(username)
-        self._abort_if_unique_id_configured({CONF_TOKEN: token})
+        try:
+            self._abort_if_unique_id_configured({CONF_TOKEN: token})
+        except AbortFlow:
+            await async_create_import_issue(self.hass, self.context["source"], "", True)
+            raise
        return self.async_create_entry(
            title=username, data={CONF_USERNAME: username, CONF_TOKEN: token}
        )
@ -37,11 +77,6 @@ class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
        token: str | None = None,
    ):
        """Create client."""
-        if password is None and token is None:
-            raise ValueError(
-                "Invalid internal state. Called without either password or token"
-            )
-
        try:
            async with timeout(10):
                if (acquired_token := token) is None:
@ -56,9 +91,18 @@ class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
                )
        except ClientResponseError as err:
            if err.status in (HTTPStatus.UNAUTHORIZED, HTTPStatus.FORBIDDEN):
+                await async_create_import_issue(
+                    self.hass, self.context["source"], "invalid_auth"
+                )
                return self.async_abort(reason="invalid_auth")
+            await async_create_import_issue(
+                self.hass, self.context["source"], "cannot_connect"
+            )
            return self.async_abort(reason="cannot_connect")
        except (asyncio.TimeoutError, ClientError):
+            await async_create_import_issue(
+                self.hass, self.context["source"], "cannot_connect"
+            )
            return self.async_abort(reason="cannot_connect")

        return await self._create_entry(username, acquired_token)
@ -77,6 +121,9 @@ class FlowHandler(config_entries.ConfigFlow, domain=DOMAIN):

    async def async_step_import(self, user_input):
        """Import a config entry."""
-        return await self._create_client(
+        result = await self._create_client(
            user_input[CONF_USERNAME], token=user_input[CONF_TOKEN]
        )
+        if result["type"] == FlowResultType.CREATE_ENTRY:
+            await async_create_import_issue(self.hass, self.context["source"], "", True)
+        return result
--- a/homeassistant/components/melcloud/strings.json
+++ b/homeassistant/components/melcloud/strings.json
@ -40,5 +40,15 @@
        }
      }
    }
+  },
+  "issues": {
+    "deprecated_yaml_import_issue_invalid_auth": {
+      "title": "The MELCloud YAML configuration import failed",
+      "description": "Configuring MELCloud using YAML is being removed but there was an authentication error importing your YAML configuration.\n\nCorrect the YAML configuration and restart Home Assistant to try again or remove the MELCloud YAML configuration from your configuration.yaml file and continue to [set up the integration](/config/integrations/dashboard/add?domain=melcoud) manually."
+    },
+    "deprecated_yaml_import_issue_cannot_connect": {
+      "title": "The MELCloud YAML configuration import failed",
+      "description": "Configuring MELCloud using YAML is being removed but there was an connection error importing your YAML configuration.\n\nEnsure connection to MELCloud works and restart Home Assistant to try again or remove the MELCloud YAML configuration from your configuration.yaml file and continue to [set up the integration](/config/integrations/dashboard/add?domain=melcoud) manually."
+    }
  }
 }
--- a/tests/components/melcloud/test_config_flow.py
+++ b/tests/components/melcloud/test_config_flow.py
@ -7,9 +7,10 @@ from aiohttp import ClientError, ClientResponseError
 import pymelcloud
 import pytest

-from homeassistant import config_entries
+from homeassistant import config_entries, data_entry_flow
 from homeassistant.components.melcloud.const import DOMAIN
-from homeassistant.core import HomeAssistant
+from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant
+import homeassistant.helpers.issue_registry as ir

 from tests.common import MockConfigEntry

@ -119,6 +120,106 @@ async def test_form_response_errors(
    assert result["reason"] == message


+@pytest.mark.parametrize(
+    ("error", "message", "issue"),
+    [
+        (
+            HTTPStatus.UNAUTHORIZED,
+            "invalid_auth",
+            "deprecated_yaml_import_issue_invalid_auth",
+        ),
+        (
+            HTTPStatus.FORBIDDEN,
+            "invalid_auth",
+            "deprecated_yaml_import_issue_invalid_auth",
+        ),
+        (
+            HTTPStatus.INTERNAL_SERVER_ERROR,
+            "cannot_connect",
+            "deprecated_yaml_import_issue_cannot_connect",
+        ),
+    ],
+)
+async def test_step_import_fails(
+    hass: HomeAssistant,
+    mock_login,
+    mock_get_devices,
+    mock_request_info,
+    error: Exception,
+    message: str,
+    issue: str,
+) -> None:
+    """Test raising issues on import."""
+    mock_get_devices.side_effect = ClientResponseError(
+        mock_request_info(), (), status=error
+    )
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={"source": config_entries.SOURCE_IMPORT},
+        data={"username": "test-email@test-domain.com", "token": "test-token"},
+    )
+
+    assert result["type"] == data_entry_flow.FlowResultType.ABORT
+    assert result["reason"] == message
+
+    issue_registry = ir.async_get(hass)
+    assert issue_registry.async_get_issue(DOMAIN, issue)
+
+
+async def test_step_import_fails_ClientError(
+    hass: HomeAssistant,
+    mock_login,
+    mock_get_devices,
+    mock_request_info,
+) -> None:
+    """Test raising issues on import for ClientError."""
+    mock_get_devices.side_effect = ClientError()
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={"source": config_entries.SOURCE_IMPORT},
+        data={"username": "test-email@test-domain.com", "token": "test-token"},
+    )
+
+    assert result["type"] == data_entry_flow.FlowResultType.ABORT
+    assert result["reason"] == "cannot_connect"
+
+    issue_registry = ir.async_get(hass)
+    assert issue_registry.async_get_issue(
+        DOMAIN, "deprecated_yaml_import_issue_cannot_connect"
+    )
+
+
+async def test_step_import_already_exist(
+    hass: HomeAssistant,
+    mock_login,
+    mock_get_devices,
+    mock_request_info,
+) -> None:
+    """Test that errors are shown when duplicates are added."""
+    conf = {"username": "test-email@test-domain.com", "token": "test-token"}
+    config_entry = MockConfigEntry(
+        domain=DOMAIN,
+        data=conf,
+        title=conf["username"],
+        unique_id=conf["username"],
+    )
+    config_entry.add_to_hass(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=conf
+    )
+    assert result["type"] == data_entry_flow.FlowResultType.ABORT
+    assert result["reason"] == "already_configured"
+
+    issue_registry = ir.async_get(hass)
+    issue = issue_registry.async_get_issue(
+        HOMEASSISTANT_DOMAIN, "deprecated_yaml_melcloud"
+    )
+    assert issue.translation_key == "deprecated_yaml"
+
+
 async def test_import_with_token(
    hass: HomeAssistant, mock_login, mock_get_devices
 ) -> None:
@ -144,6 +245,12 @@ async def test_import_with_token(
    assert len(mock_setup.mock_calls) == 1
    assert len(mock_setup_entry.mock_calls) == 1

+    issue_registry = ir.async_get(hass)
+    issue = issue_registry.async_get_issue(
+        HOMEASSISTANT_DOMAIN, "deprecated_yaml_melcloud"
+    )
+    assert issue.translation_key == "deprecated_yaml"
+

 async def test_token_refresh(hass: HomeAssistant, mock_login, mock_get_devices) -> None:
    """Re-configuration with existing username should refresh token."""