From c91ac22d3cefca6e42fdf1143fe1c9daa44cefc1 Mon Sep 17 00:00:00 2001 From: Rami Mosleh Date: Fri, 22 Dec 2023 16:24:50 +0200 Subject: [PATCH] Add location selector to Islamic prayer times (#105911) * Add location selector to config flow * Simplify entry data * fix abort string * Add migration with minor version * Follow documented migration method --- .../islamic_prayer_times/__init__.py | 34 +++++++++- .../islamic_prayer_times/config_flow.py | 65 +++++++++++++++++-- .../islamic_prayer_times/coordinator.py | 9 ++- .../islamic_prayer_times/strings.json | 2 +- .../islamic_prayer_times/__init__.py | 8 +++ .../islamic_prayer_times/test_config_flow.py | 65 ++++++++++++++++--- .../islamic_prayer_times/test_init.py | 24 ++++++- 7 files changed, 185 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/islamic_prayer_times/__init__.py b/homeassistant/components/islamic_prayer_times/__init__.py index 2925ca527bc..86ef3ce271f 100644 --- a/homeassistant/components/islamic_prayer_times/__init__.py +++ b/homeassistant/components/islamic_prayer_times/__init__.py @@ -1,8 +1,10 @@ """The islamic_prayer_times component.""" from __future__ import annotations +import logging + from homeassistant.config_entries import ConfigEntry -from homeassistant.const import Platform +from homeassistant.const import CONF_LATITUDE, CONF_LONGITUDE, Platform from homeassistant.core import HomeAssistant, callback from homeassistant.helpers import config_validation as cv, entity_registry as er @@ -13,6 +15,8 @@ PLATFORMS = [Platform.SENSOR] CONFIG_SCHEMA = cv.removed(DOMAIN, raise_if_present=False) +_LOGGER = logging.getLogger(__name__) + async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Set up the Islamic Prayer Component.""" @@ -41,6 +45,34 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b return True +async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: + """Migrate old entry.""" + _LOGGER.debug("Migrating from version %s", config_entry.version) + + if config_entry.version > 1: + # This means the user has downgraded from a future version + return False + if config_entry.version == 1: + new = {**config_entry.data} + if config_entry.minor_version < 2: + lat = hass.config.latitude + lon = hass.config.longitude + new = { + CONF_LATITUDE: lat, + CONF_LONGITUDE: lon, + } + unique_id = f"{lat}-{lon}" + config_entry.version = 1 + config_entry.minor_version = 2 + hass.config_entries.async_update_entry( + config_entry, data=new, unique_id=unique_id + ) + + _LOGGER.debug("Migration to version %s successful", config_entry.version) + + return True + + async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Unload Islamic Prayer entry from config_entry.""" if unload_ok := await hass.config_entries.async_unload_platforms( diff --git a/homeassistant/components/islamic_prayer_times/config_flow.py b/homeassistant/components/islamic_prayer_times/config_flow.py index 333b6b36c87..2fde06f576d 100644 --- a/homeassistant/components/islamic_prayer_times/config_flow.py +++ b/homeassistant/components/islamic_prayer_times/config_flow.py @@ -3,16 +3,22 @@ from __future__ import annotations from typing import Any +from prayer_times_calculator import InvalidResponseError, PrayerTimesCalculator +from requests.exceptions import ConnectionError as ConnError import voluptuous as vol from homeassistant import config_entries -from homeassistant.core import callback +from homeassistant.const import CONF_LATITUDE, CONF_LOCATION, CONF_LONGITUDE, CONF_NAME +from homeassistant.core import HomeAssistant, callback from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers.selector import ( + LocationSelector, SelectSelector, SelectSelectorConfig, SelectSelectorMode, + TextSelector, ) +import homeassistant.util.dt as dt_util from .const import ( CALC_METHODS, @@ -32,10 +38,31 @@ from .const import ( ) +async def async_validate_location( + hass: HomeAssistant, lon: float, lat: float +) -> dict[str, str]: + """Check if the selected location is valid.""" + errors = {} + calc = PrayerTimesCalculator( + latitude=lat, + longitude=lon, + calculation_method=DEFAULT_CALC_METHOD, + date=str(dt_util.now().date()), + ) + try: + await hass.async_add_executor_job(calc.fetch_prayer_times) + except InvalidResponseError: + errors["base"] = "invalid_location" + except ConnError: + errors["base"] = "conn_error" + return errors + + class IslamicPrayerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Handle the Islamic Prayer config flow.""" VERSION = 1 + MINOR_VERSION = 2 @staticmethod @callback @@ -49,13 +76,39 @@ class IslamicPrayerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): self, user_input: dict[str, Any] | None = None ) -> FlowResult: """Handle a flow initialized by the user.""" - if self._async_current_entries(): - return self.async_abort(reason="single_instance_allowed") + errors = {} - if user_input is None: - return self.async_show_form(step_id="user") + if user_input is not None: + lat: float = user_input[CONF_LOCATION][CONF_LATITUDE] + lon: float = user_input[CONF_LOCATION][CONF_LONGITUDE] + await self.async_set_unique_id(f"{lat}-{lon}") + self._abort_if_unique_id_configured() - return self.async_create_entry(title=NAME, data=user_input) + if not (errors := await async_validate_location(self.hass, lat, lon)): + return self.async_create_entry( + title=user_input[CONF_NAME], + data={ + CONF_LATITUDE: lat, + CONF_LONGITUDE: lon, + }, + ) + + home_location = { + CONF_LATITUDE: self.hass.config.latitude, + CONF_LONGITUDE: self.hass.config.longitude, + } + return self.async_show_form( + step_id="user", + data_schema=vol.Schema( + { + vol.Optional(CONF_NAME, default=NAME): TextSelector(), + vol.Required( + CONF_LOCATION, default=home_location + ): LocationSelector(), + } + ), + errors=errors, + ) class IslamicPrayerOptionsFlowHandler(config_entries.OptionsFlow): diff --git a/homeassistant/components/islamic_prayer_times/coordinator.py b/homeassistant/components/islamic_prayer_times/coordinator.py index aedaf43411a..be138e7b45b 100644 --- a/homeassistant/components/islamic_prayer_times/coordinator.py +++ b/homeassistant/components/islamic_prayer_times/coordinator.py @@ -9,6 +9,7 @@ from prayer_times_calculator import PrayerTimesCalculator, exceptions from requests.exceptions import ConnectionError as ConnError from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_LATITUDE, CONF_LONGITUDE from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback from homeassistant.helpers.event import async_call_later, async_track_point_in_time from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed @@ -36,12 +37,14 @@ class IslamicPrayerDataUpdateCoordinator(DataUpdateCoordinator[dict[str, datetim def __init__(self, hass: HomeAssistant) -> None: """Initialize the Islamic Prayer client.""" - self.event_unsub: CALLBACK_TYPE | None = None super().__init__( hass, _LOGGER, name=DOMAIN, ) + self.latitude = self.config_entry.data[CONF_LATITUDE] + self.longitude = self.config_entry.data[CONF_LONGITUDE] + self.event_unsub: CALLBACK_TYPE | None = None @property def calc_method(self) -> str: @@ -70,8 +73,8 @@ class IslamicPrayerDataUpdateCoordinator(DataUpdateCoordinator[dict[str, datetim def get_new_prayer_times(self) -> dict[str, Any]: """Fetch prayer times for today.""" calc = PrayerTimesCalculator( - latitude=self.hass.config.latitude, - longitude=self.hass.config.longitude, + latitude=self.latitude, + longitude=self.longitude, calculation_method=self.calc_method, latitudeAdjustmentMethod=self.lat_adj_method, midnightMode=self.midnight_mode, diff --git a/homeassistant/components/islamic_prayer_times/strings.json b/homeassistant/components/islamic_prayer_times/strings.json index e07a38ca107..87703e5fdae 100644 --- a/homeassistant/components/islamic_prayer_times/strings.json +++ b/homeassistant/components/islamic_prayer_times/strings.json @@ -8,7 +8,7 @@ } }, "abort": { - "single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_service%]" } }, "options": { diff --git a/tests/components/islamic_prayer_times/__init__.py b/tests/components/islamic_prayer_times/__init__.py index 8750461c47f..4df733a93fc 100644 --- a/tests/components/islamic_prayer_times/__init__.py +++ b/tests/components/islamic_prayer_times/__init__.py @@ -2,8 +2,16 @@ from datetime import datetime +from homeassistant.const import CONF_LATITUDE, CONF_LOCATION, CONF_LONGITUDE, CONF_NAME import homeassistant.util.dt as dt_util +MOCK_USER_INPUT = { + CONF_NAME: "Home", + CONF_LOCATION: {CONF_LATITUDE: 12.34, CONF_LONGITUDE: 23.45}, +} + +MOCK_CONFIG = {CONF_LATITUDE: 12.34, CONF_LONGITUDE: 23.45} + PRAYER_TIMES = { "Fajr": "2020-01-01T06:10:00+00:00", "Sunrise": "2020-01-01T07:25:00+00:00", diff --git a/tests/components/islamic_prayer_times/test_config_flow.py b/tests/components/islamic_prayer_times/test_config_flow.py index f331c5bf49b..0375c788b11 100644 --- a/tests/components/islamic_prayer_times/test_config_flow.py +++ b/tests/components/islamic_prayer_times/test_config_flow.py @@ -1,5 +1,9 @@ """Tests for Islamic Prayer Times config flow.""" +from unittest.mock import patch + +from prayer_times_calculator import InvalidResponseError import pytest +from requests.exceptions import ConnectionError as ConnError from homeassistant import config_entries, data_entry_flow from homeassistant.components import islamic_prayer_times @@ -12,6 +16,8 @@ from homeassistant.components.islamic_prayer_times.const import ( ) from homeassistant.core import HomeAssistant +from . import MOCK_CONFIG, MOCK_USER_INPUT + from tests.common import MockConfigEntry pytestmark = pytest.mark.usefixtures("mock_setup_entry") @@ -25,13 +31,47 @@ async def test_flow_works(hass: HomeAssistant) -> None: assert result["type"] == data_entry_flow.FlowResultType.FORM assert result["step_id"] == "user" - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={} - ) - await hass.async_block_till_done() + with patch( + "homeassistant.components.islamic_prayer_times.config_flow.async_validate_location", + return_value={}, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=MOCK_USER_INPUT + ) + await hass.async_block_till_done() assert result["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY - assert result["title"] == "Islamic Prayer Times" + assert result["title"] == "Home" + + +@pytest.mark.parametrize( + ("exception", "error"), + [ + (InvalidResponseError, "invalid_location"), + (ConnError, "conn_error"), + ], +) +async def test_flow_error( + hass: HomeAssistant, exception: Exception, error: str +) -> None: + """Test flow errors.""" + result = await hass.config_entries.flow.async_init( + islamic_prayer_times.DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["step_id"] == "user" + + with patch( + "homeassistant.components.islamic_prayer_times.config_flow.PrayerTimesCalculator.fetch_prayer_times", + side_effect=exception, + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=MOCK_USER_INPUT + ) + await hass.async_block_till_done() + + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["errors"]["base"] == error async def test_options(hass: HomeAssistant) -> None: @@ -39,7 +79,7 @@ async def test_options(hass: HomeAssistant) -> None: entry = MockConfigEntry( domain=DOMAIN, title="Islamic Prayer Times", - data={}, + data=MOCK_CONFIG, options={CONF_CALC_METHOD: "isna"}, ) entry.add_to_hass(hass) @@ -68,14 +108,19 @@ async def test_options(hass: HomeAssistant) -> None: async def test_integration_already_configured(hass: HomeAssistant) -> None: """Test integration is already configured.""" entry = MockConfigEntry( - domain=DOMAIN, - data={}, - options={}, + domain=DOMAIN, data=MOCK_CONFIG, options={}, unique_id="12.34-23.45" ) entry.add_to_hass(hass) result = await hass.config_entries.flow.async_init( islamic_prayer_times.DOMAIN, context={"source": config_entries.SOURCE_USER} ) + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["step_id"] == "user" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=MOCK_USER_INPUT + ) + await hass.async_block_till_done() assert result["type"] == data_entry_flow.FlowResultType.ABORT - assert result["reason"] == "single_instance_allowed" + assert result["reason"] == "already_configured" diff --git a/tests/components/islamic_prayer_times/test_init.py b/tests/components/islamic_prayer_times/test_init.py index 0c3f19e43fe..746abf27d43 100644 --- a/tests/components/islamic_prayer_times/test_init.py +++ b/tests/components/islamic_prayer_times/test_init.py @@ -10,7 +10,7 @@ from homeassistant import config_entries from homeassistant.components import islamic_prayer_times from homeassistant.components.islamic_prayer_times.const import CONF_CALC_METHOD from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN -from homeassistant.const import STATE_UNAVAILABLE +from homeassistant.const import CONF_LATITUDE, CONF_LONGITUDE, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er import homeassistant.util.dt as dt_util @@ -185,3 +185,25 @@ async def test_migrate_unique_id( entity_migrated = entity_registry.async_get(entity.entity_id) assert entity_migrated assert entity_migrated.unique_id == f"{entry.entry_id}-{old_unique_id}" + + +async def test_migration_from_1_1_to_1_2(hass: HomeAssistant) -> None: + """Test migrating from version 1.1 to 1.2.""" + entry = MockConfigEntry( + domain=islamic_prayer_times.DOMAIN, + data={}, + ) + entry.add_to_hass(hass) + + with patch( + "prayer_times_calculator.PrayerTimesCalculator.fetch_prayer_times", + return_value=PRAYER_TIMES, + ), freeze_time(NOW): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert entry.data == { + CONF_LATITUDE: hass.config.latitude, + CONF_LONGITUDE: hass.config.longitude, + } + assert entry.minor_version == 2