From 8d2813fb8b0f83a2c69b41e821ef0f1b3c222713 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Tue, 23 Apr 2024 22:53:13 +0200 Subject: [PATCH] Migrate legacy Ecobee notify service (#115592) * Migrate legacy Ecobee notify service * Correct comment * Update homeassistant/components/ecobee/notify.py Co-authored-by: Joost Lekkerkerker * Use version to check latest entry being used * Use 6 months of deprecation * Add repair flow tests * Only allow migrate_notify fix flow * Simplify repair flow * Use ecobee data to refrence entry * Make entry attrubute puiblic * Use hass.data ro retrieve entry. * Only register issue when legacy service when it is use * Remove backslash * Use ws_client.send_json_auto_id * Cleanup * Import domain from notify integration * Apply suggestions from code review Co-authored-by: Joost Lekkerkerker * Update dependencies * Use Issue_registry fixture * remove `update_before_add` flag * Update homeassistant/components/ecobee/notify.py Co-authored-by: Joost Lekkerkerker * Update homeassistant/components/ecobee/notify.py * Update tests/components/ecobee/conftest.py Co-authored-by: Joost Lekkerkerker * Fix typo and import --------- Co-authored-by: Joost Lekkerkerker --- homeassistant/components/ecobee/__init__.py | 6 +- homeassistant/components/ecobee/const.py | 1 + homeassistant/components/ecobee/manifest.json | 1 + homeassistant/components/ecobee/notify.py | 57 ++++++++++++- homeassistant/components/ecobee/repairs.py | 37 +++++++++ homeassistant/components/ecobee/strings.json | 13 +++ tests/components/ecobee/common.py | 10 ++- tests/components/ecobee/conftest.py | 9 ++- tests/components/ecobee/test_notify.py | 57 +++++++++++++ tests/components/ecobee/test_repairs.py | 79 +++++++++++++++++++ 10 files changed, 259 insertions(+), 11 deletions(-) create mode 100644 homeassistant/components/ecobee/repairs.py create mode 100644 tests/components/ecobee/test_notify.py create mode 100644 tests/components/ecobee/test_repairs.py diff --git a/homeassistant/components/ecobee/__init__.py b/homeassistant/components/ecobee/__init__.py index 8083d0efcb4..6f032fbaae9 100644 --- a/homeassistant/components/ecobee/__init__.py +++ b/homeassistant/components/ecobee/__init__.py @@ -73,6 +73,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + # The legacy Ecobee notify.notify service is deprecated + # was with HA Core 2024.5.0 and will be removed with HA core 2024.11.0 hass.async_create_task( discovery.async_load_platform( hass, @@ -97,7 +99,7 @@ class EcobeeData: ) -> None: """Initialize the Ecobee data object.""" self._hass = hass - self._entry = entry + self.entry = entry self.ecobee = Ecobee( config={ECOBEE_API_KEY: api_key, ECOBEE_REFRESH_TOKEN: refresh_token} ) @@ -117,7 +119,7 @@ class EcobeeData: _LOGGER.debug("Refreshing ecobee tokens and updating config entry") if await self._hass.async_add_executor_job(self.ecobee.refresh_tokens): self._hass.config_entries.async_update_entry( - self._entry, + self.entry, data={ CONF_API_KEY: self.ecobee.config[ECOBEE_API_KEY], CONF_REFRESH_TOKEN: self.ecobee.config[ECOBEE_REFRESH_TOKEN], diff --git a/homeassistant/components/ecobee/const.py b/homeassistant/components/ecobee/const.py index e20acb5cfca..0eed0ab67f9 100644 --- a/homeassistant/components/ecobee/const.py +++ b/homeassistant/components/ecobee/const.py @@ -46,6 +46,7 @@ PLATFORMS = [ Platform.BINARY_SENSOR, Platform.CLIMATE, Platform.HUMIDIFIER, + Platform.NOTIFY, Platform.NUMBER, Platform.SENSOR, Platform.WEATHER, diff --git a/homeassistant/components/ecobee/manifest.json b/homeassistant/components/ecobee/manifest.json index f3f5b59a36f..7e461230600 100644 --- a/homeassistant/components/ecobee/manifest.json +++ b/homeassistant/components/ecobee/manifest.json @@ -3,6 +3,7 @@ "name": "ecobee", "codeowners": [], "config_flow": true, + "dependencies": ["http", "repairs"], "documentation": "https://www.home-assistant.io/integrations/ecobee", "homekit": { "models": ["EB", "ecobee*"] diff --git a/homeassistant/components/ecobee/notify.py b/homeassistant/components/ecobee/notify.py index b2f6ccb05c8..787130c403f 100644 --- a/homeassistant/components/ecobee/notify.py +++ b/homeassistant/components/ecobee/notify.py @@ -2,11 +2,23 @@ from __future__ import annotations -from homeassistant.components.notify import ATTR_TARGET, BaseNotificationService +from functools import partial +from typing import Any + +from homeassistant.components.notify import ( + ATTR_TARGET, + BaseNotificationService, + NotifyEntity, +) +from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant +from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from . import Ecobee, EcobeeData from .const import DOMAIN +from .entity import EcobeeBaseEntity +from .repairs import migrate_notify_issue def get_service( @@ -18,18 +30,25 @@ def get_service( if discovery_info is None: return None - data = hass.data[DOMAIN] + data: EcobeeData = hass.data[DOMAIN] return EcobeeNotificationService(data.ecobee) class EcobeeNotificationService(BaseNotificationService): """Implement the notification service for the Ecobee thermostat.""" - def __init__(self, ecobee): + def __init__(self, ecobee: Ecobee) -> None: """Initialize the service.""" self.ecobee = ecobee - def send_message(self, message="", **kwargs): + async def async_send_message(self, message: str = "", **kwargs: Any) -> None: + """Send a message and raise issue.""" + migrate_notify_issue(self.hass) + await self.hass.async_add_executor_job( + partial(self.send_message, message, **kwargs) + ) + + def send_message(self, message: str = "", **kwargs: Any) -> None: """Send a message.""" targets = kwargs.get(ATTR_TARGET) @@ -39,3 +58,33 @@ class EcobeeNotificationService(BaseNotificationService): for target in targets: thermostat_index = int(target) self.ecobee.send_message(thermostat_index, message) + + +async def async_setup_entry( + hass: HomeAssistant, + config_entry: ConfigEntry, + async_add_entities: AddEntitiesCallback, +) -> None: + """Set up the ecobee thermostat.""" + data: EcobeeData = hass.data[DOMAIN] + async_add_entities( + EcobeeNotifyEntity(data, index) for index in range(len(data.ecobee.thermostats)) + ) + + +class EcobeeNotifyEntity(EcobeeBaseEntity, NotifyEntity): + """Implement the notification entity for the Ecobee thermostat.""" + + _attr_name = None + _attr_has_entity_name = True + + def __init__(self, data: EcobeeData, thermostat_index: int) -> None: + """Initialize the thermostat.""" + super().__init__(data, thermostat_index) + self._attr_unique_id = ( + f"{self.thermostat["identifier"]}_notify_{thermostat_index}" + ) + + def send_message(self, message: str) -> None: + """Send a message.""" + self.data.ecobee.send_message(self.thermostat_index, message) diff --git a/homeassistant/components/ecobee/repairs.py b/homeassistant/components/ecobee/repairs.py new file mode 100644 index 00000000000..66474730b2f --- /dev/null +++ b/homeassistant/components/ecobee/repairs.py @@ -0,0 +1,37 @@ +"""Repairs support for Ecobee.""" + +from __future__ import annotations + +from homeassistant.components.notify import DOMAIN as NOTIFY_DOMAIN +from homeassistant.components.repairs import RepairsFlow +from homeassistant.components.repairs.issue_handler import ConfirmRepairFlow +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers import issue_registry as ir + +from .const import DOMAIN + + +@callback +def migrate_notify_issue(hass: HomeAssistant) -> None: + """Ensure an issue is registered.""" + ir.async_create_issue( + hass, + DOMAIN, + "migrate_notify", + breaks_in_ha_version="2024.11.0", + issue_domain=NOTIFY_DOMAIN, + is_fixable=True, + is_persistent=True, + translation_key="migrate_notify", + severity=ir.IssueSeverity.WARNING, + ) + + +async def async_create_fix_flow( + hass: HomeAssistant, + issue_id: str, + data: dict[str, str | int | float | None] | None, +) -> RepairsFlow: + """Create flow.""" + assert issue_id == "migrate_notify" + return ConfirmRepairFlow() diff --git a/homeassistant/components/ecobee/strings.json b/homeassistant/components/ecobee/strings.json index b1d1df65417..1d64b6d6b94 100644 --- a/homeassistant/components/ecobee/strings.json +++ b/homeassistant/components/ecobee/strings.json @@ -163,5 +163,18 @@ } } } + }, + "issues": { + "migrate_notify": { + "title": "Migration of Ecobee notify service", + "fix_flow": { + "step": { + "confirm": { + "description": "The Ecobee `notify` service has been migrated. A new `notify` entity per Thermostat is available now.\n\nUpdate any automations to use the new `notify.send_message` exposed by these new entities. When this is done, fix this issue and restart Home Assistant.", + "title": "Disable legacy Ecobee notify service" + } + } + } + } } } diff --git a/tests/components/ecobee/common.py b/tests/components/ecobee/common.py index 60f17c3618d..423b0eee320 100644 --- a/tests/components/ecobee/common.py +++ b/tests/components/ecobee/common.py @@ -4,14 +4,19 @@ from unittest.mock import patch from homeassistant.components.ecobee.const import CONF_REFRESH_TOKEN, DOMAIN from homeassistant.const import CONF_API_KEY +from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry -async def setup_platform(hass, platform) -> MockConfigEntry: +async def setup_platform( + hass: HomeAssistant, + platform: str, +) -> MockConfigEntry: """Set up the ecobee platform.""" mock_entry = MockConfigEntry( + title=DOMAIN, domain=DOMAIN, data={ CONF_API_KEY: "ABC123", @@ -22,7 +27,6 @@ async def setup_platform(hass, platform) -> MockConfigEntry: with patch("homeassistant.components.ecobee.const.PLATFORMS", [platform]): assert await async_setup_component(hass, DOMAIN, {}) - - await hass.async_block_till_done() + await hass.async_block_till_done() return mock_entry diff --git a/tests/components/ecobee/conftest.py b/tests/components/ecobee/conftest.py index 952c2f3fba3..27d5a949c58 100644 --- a/tests/components/ecobee/conftest.py +++ b/tests/components/ecobee/conftest.py @@ -1,12 +1,13 @@ """Fixtures for tests.""" +from collections.abc import Generator from unittest.mock import MagicMock, patch import pytest from homeassistant.components.ecobee import ECOBEE_API_KEY, ECOBEE_REFRESH_TOKEN -from tests.common import load_fixture +from tests.common import load_fixture, load_json_object_fixture @pytest.fixture(autouse=True) @@ -23,11 +24,15 @@ def requests_mock_fixture(requests_mock): @pytest.fixture -def mock_ecobee(): +def mock_ecobee() -> Generator[None, MagicMock]: """Mock an Ecobee object.""" ecobee = MagicMock() ecobee.request_pin.return_value = True ecobee.refresh_tokens.return_value = True + ecobee.thermostats = load_json_object_fixture("ecobee-data.json", "ecobee")[ + "thermostatList" + ] + ecobee.get_thermostat = lambda index: ecobee.thermostats[index] ecobee.config = {ECOBEE_API_KEY: "mocked_key", ECOBEE_REFRESH_TOKEN: "mocked_token"} with patch("homeassistant.components.ecobee.Ecobee", return_value=ecobee): diff --git a/tests/components/ecobee/test_notify.py b/tests/components/ecobee/test_notify.py new file mode 100644 index 00000000000..c66f04c752a --- /dev/null +++ b/tests/components/ecobee/test_notify.py @@ -0,0 +1,57 @@ +"""Test Ecobee notify service.""" + +from unittest.mock import MagicMock + +from homeassistant.components.ecobee import DOMAIN +from homeassistant.components.notify import ( + DOMAIN as NOTIFY_DOMAIN, + SERVICE_SEND_MESSAGE, +) +from homeassistant.core import HomeAssistant +from homeassistant.helpers import issue_registry as ir + +from .common import setup_platform + +THERMOSTAT_ID = 0 + + +async def test_notify_entity_service( + hass: HomeAssistant, + mock_ecobee: MagicMock, +) -> None: + """Test the notify entity service.""" + await setup_platform(hass, NOTIFY_DOMAIN) + + entity_id = "notify.ecobee" + state = hass.states.get(entity_id) + assert state is not None + assert hass.services.has_service(NOTIFY_DOMAIN, SERVICE_SEND_MESSAGE) + await hass.services.async_call( + NOTIFY_DOMAIN, + SERVICE_SEND_MESSAGE, + service_data={"entity_id": entity_id, "message": "It is too cold!"}, + blocking=True, + ) + await hass.async_block_till_done() + mock_ecobee.send_message.assert_called_with(THERMOSTAT_ID, "It is too cold!") + + +async def test_legacy_notify_service( + hass: HomeAssistant, + mock_ecobee: MagicMock, + issue_registry: ir.IssueRegistry, +) -> None: + """Test the legacy notify service.""" + await setup_platform(hass, NOTIFY_DOMAIN) + + assert hass.services.has_service(NOTIFY_DOMAIN, DOMAIN) + await hass.services.async_call( + NOTIFY_DOMAIN, + DOMAIN, + service_data={"message": "It is too cold!", "target": THERMOSTAT_ID}, + blocking=True, + ) + await hass.async_block_till_done() + mock_ecobee.send_message.assert_called_with(THERMOSTAT_ID, "It is too cold!") + mock_ecobee.send_message.reset_mock() + assert len(issue_registry.issues) == 1 diff --git a/tests/components/ecobee/test_repairs.py b/tests/components/ecobee/test_repairs.py new file mode 100644 index 00000000000..19fdc6f7bba --- /dev/null +++ b/tests/components/ecobee/test_repairs.py @@ -0,0 +1,79 @@ +"""Test repairs for Ecobee integration.""" + +from http import HTTPStatus +from unittest.mock import MagicMock + +from homeassistant.components.ecobee import DOMAIN +from homeassistant.components.notify import DOMAIN as NOTIFY_DOMAIN +from homeassistant.components.repairs.issue_handler import ( + async_process_repairs_platforms, +) +from homeassistant.components.repairs.websocket_api import ( + RepairsFlowIndexView, + RepairsFlowResourceView, +) +from homeassistant.core import HomeAssistant +from homeassistant.helpers import issue_registry as ir + +from .common import setup_platform + +from tests.typing import ClientSessionGenerator + +THERMOSTAT_ID = 0 + + +async def test_ecobee_repair_flow( + hass: HomeAssistant, + mock_ecobee: MagicMock, + hass_client: ClientSessionGenerator, + issue_registry: ir.IssueRegistry, +) -> None: + """Test the ecobee notify service repair flow is triggered.""" + await setup_platform(hass, NOTIFY_DOMAIN) + await async_process_repairs_platforms(hass) + + http_client = await hass_client() + + # Simulate legacy service being used + assert hass.services.has_service(NOTIFY_DOMAIN, DOMAIN) + await hass.services.async_call( + NOTIFY_DOMAIN, + DOMAIN, + service_data={"message": "It is too cold!", "target": THERMOSTAT_ID}, + blocking=True, + ) + await hass.async_block_till_done() + mock_ecobee.send_message.assert_called_with(THERMOSTAT_ID, "It is too cold!") + mock_ecobee.send_message.reset_mock() + + # Assert the issue is present + assert issue_registry.async_get_issue( + domain=DOMAIN, + issue_id="migrate_notify", + ) + assert len(issue_registry.issues) == 1 + + url = RepairsFlowIndexView.url + resp = await http_client.post( + url, json={"handler": DOMAIN, "issue_id": "migrate_notify"} + ) + assert resp.status == HTTPStatus.OK + data = await resp.json() + + flow_id = data["flow_id"] + assert data["step_id"] == "confirm" + + url = RepairsFlowResourceView.url.format(flow_id=flow_id) + resp = await http_client.post(url) + assert resp.status == HTTPStatus.OK + data = await resp.json() + assert data["type"] == "create_entry" + # Test confirm step in repair flow + await hass.async_block_till_done() + + # Assert the issue is no longer present + assert not issue_registry.async_get_issue( + domain=DOMAIN, + issue_id="migrate_notify", + ) + assert len(issue_registry.issues) == 0