Fix Jewish calendar unique id's (#117985)

* Initial commit

* Fix updating of unique id

* Add testing to check the unique id is being updated correctly

* Reload the config entry and confirm the unique id has not been changed

* Move updating unique_id to __init__.py as suggested

* Change the config_entry variable's name back from config to config_entry

* Move the loop into the update_unique_ids method

* Move test from test_config_flow to test_init

* Try an early optimization to check if we need to update the unique ids

* Mention the correct version

* Implement suggestions

* Ensure all entities are migrated correctly

* Just to be sure keep the previous assertion as well

homeassistant/components/jewish_calendar/__init__.py

@@ -16,11 +16,13 @@ from homeassistant.const import (
     CONF_TIME_ZONE,
     Platform,
 )
-from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant
+from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant, callback
 import homeassistant.helpers.config_validation as cv
+import homeassistant.helpers.entity_registry as er
 from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
 from homeassistant.helpers.typing import ConfigType
 
+from .binary_sensor import BINARY_SENSORS
 from .const import (
     CONF_CANDLE_LIGHT_MINUTES,
     CONF_DIASPORA,
@@ -32,6 +34,7 @@ from .const import (
     DEFAULT_NAME,
     DOMAIN,
 )
+from .sensor import INFO_SENSORS, TIME_SENSORS
 
 PLATFORMS: list[Platform] = [Platform.BINARY_SENSOR, Platform.SENSOR]
 
@@ -131,18 +134,24 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b
         timezone=config_entry.data.get(CONF_TIME_ZONE, hass.config.time_zone),
     )
 
-    prefix = get_unique_prefix(
-        location, language, candle_lighting_offset, havdalah_offset
-    )
     hass.data.setdefault(DOMAIN, {})[config_entry.entry_id] = {
         CONF_LANGUAGE: language,
         CONF_DIASPORA: diaspora,
         CONF_LOCATION: location,
         CONF_CANDLE_LIGHT_MINUTES: candle_lighting_offset,
         CONF_HAVDALAH_OFFSET_MINUTES: havdalah_offset,
-        "prefix": prefix,
     }
 
+    # Update unique ID to be unrelated to user defined options
+    old_prefix = get_unique_prefix(
+        location, language, candle_lighting_offset, havdalah_offset
+    )
+
+    ent_reg = er.async_get(hass)
+    entries = er.async_entries_for_config_entry(ent_reg, config_entry.entry_id)
+    if not entries or any(entry.unique_id.startswith(old_prefix) for entry in entries):
+        async_update_unique_ids(ent_reg, config_entry.entry_id, old_prefix)
+
     await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS)
     return True
 
@@ -157,3 +166,25 @@ async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
         hass.data[DOMAIN].pop(config_entry.entry_id)
 
     return unload_ok
+
+
+@callback
+def async_update_unique_ids(
+    ent_reg: er.EntityRegistry, new_prefix: str, old_prefix: str
+) -> None:
+    """Update unique ID to be unrelated to user defined options.
+
+    Introduced with release 2024.6
+    """
+    platform_descriptions = {
+        Platform.BINARY_SENSOR: BINARY_SENSORS,
+        Platform.SENSOR: (*INFO_SENSORS, *TIME_SENSORS),
+    }
+    for platform, descriptions in platform_descriptions.items():
+        for description in descriptions:
+            new_unique_id = f"{new_prefix}-{description.key}"
+            old_unique_id = f"{old_prefix}_{description.key}"
+            if entity_id := ent_reg.async_get_entity_id(
+                platform, DOMAIN, old_unique_id
+            ):
+                ent_reg.async_update_entity(entity_id, new_unique_id=new_unique_id)

homeassistant/components/jewish_calendar/binary_sensor.py

@@ -70,10 +70,10 @@ async def async_setup_entry(
     async_add_entities: AddEntitiesCallback,
 ) -> None:
     """Set up the Jewish Calendar binary sensors."""
+    entry = hass.data[DOMAIN][config_entry.entry_id]
+
     async_add_entities(
-        JewishCalendarBinarySensor(
+        JewishCalendarBinarySensor(config_entry.entry_id, entry, description)
-            hass.data[DOMAIN][config_entry.entry_id], description
-        )
         for description in BINARY_SENSORS
     )
 
@@ -86,13 +86,14 @@ class JewishCalendarBinarySensor(BinarySensorEntity):
 
     def __init__(
         self,
+        entry_id: str,
         data: dict[str, Any],
         description: JewishCalendarBinarySensorEntityDescription,
     ) -> None:
         """Initialize the binary sensor."""
         self.entity_description = description
         self._attr_name = f"{DEFAULT_NAME} {description.name}"
-        self._attr_unique_id = f'{data["prefix"]}_{description.key}'
+        self._attr_unique_id = f"{entry_id}-{description.key}"
         self._location = data[CONF_LOCATION]
         self._hebrew = data[CONF_LANGUAGE] == "hebrew"
         self._candle_lighting_offset = data[CONF_CANDLE_LIGHT_MINUTES]

homeassistant/components/jewish_calendar/sensor.py

@@ -155,9 +155,13 @@ async def async_setup_entry(
 ) -> None:
     """Set up the Jewish calendar sensors ."""
     entry = hass.data[DOMAIN][config_entry.entry_id]
-    sensors = [JewishCalendarSensor(entry, description) for description in INFO_SENSORS]
+    sensors = [
+        JewishCalendarSensor(config_entry.entry_id, entry, description)
+        for description in INFO_SENSORS
+    ]
     sensors.extend(
-        JewishCalendarTimeSensor(entry, description) for description in TIME_SENSORS
+        JewishCalendarTimeSensor(config_entry.entry_id, entry, description)
+        for description in TIME_SENSORS
     )
 
     async_add_entities(sensors)
@@ -168,13 +172,14 @@ class JewishCalendarSensor(SensorEntity):
 
     def __init__(
         self,
+        entry_id: str,
         data: dict[str, Any],
         description: SensorEntityDescription,
     ) -> None:
         """Initialize the Jewish calendar sensor."""
         self.entity_description = description
         self._attr_name = f"{DEFAULT_NAME} {description.name}"
-        self._attr_unique_id = f'{data["prefix"]}_{description.key}'
+        self._attr_unique_id = f"{entry_id}-{description.key}"
         self._location = data[CONF_LOCATION]
         self._hebrew = data[CONF_LANGUAGE] == "hebrew"
         self._candle_lighting_offset = data[CONF_CANDLE_LIGHT_MINUTES]

tests/components/jewish_calendar/test_config_flow.py

@@ -93,6 +93,7 @@ async def test_import_with_options(hass: HomeAssistant) -> None:
         }
     }
 
+    # Simulate HomeAssistant setting up the component
     assert await async_setup_component(hass, DOMAIN, conf.copy())
     await hass.async_block_till_done()
 

tests/components/jewish_calendar/test_init.py

@@ -0,0 +1,74 @@
+"""Tests for the Jewish Calendar component's init."""
+
+from hdate import Location
+
+from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSORS
+from homeassistant.components.jewish_calendar import get_unique_prefix
+from homeassistant.components.jewish_calendar.const import (
+    CONF_CANDLE_LIGHT_MINUTES,
+    CONF_DIASPORA,
+    CONF_HAVDALAH_OFFSET_MINUTES,
+    DEFAULT_DIASPORA,
+    DEFAULT_LANGUAGE,
+    DOMAIN,
+)
+from homeassistant.const import CONF_LANGUAGE, CONF_LATITUDE, CONF_LONGITUDE, CONF_NAME
+from homeassistant.core import HomeAssistant
+import homeassistant.helpers.entity_registry as er
+from homeassistant.setup import async_setup_component
+
+
+async def test_import_unique_id_migration(hass: HomeAssistant) -> None:
+    """Test unique_id migration."""
+    yaml_conf = {
+        DOMAIN: {
+            CONF_NAME: "test",
+            CONF_DIASPORA: DEFAULT_DIASPORA,
+            CONF_LANGUAGE: DEFAULT_LANGUAGE,
+            CONF_CANDLE_LIGHT_MINUTES: 20,
+            CONF_HAVDALAH_OFFSET_MINUTES: 50,
+            CONF_LATITUDE: 31.76,
+            CONF_LONGITUDE: 35.235,
+        }
+    }
+
+    # Create an entry in the entity registry with the data from conf
+    ent_reg = er.async_get(hass)
+    location = Location(
+        latitude=yaml_conf[DOMAIN][CONF_LATITUDE],
+        longitude=yaml_conf[DOMAIN][CONF_LONGITUDE],
+        timezone=hass.config.time_zone,
+        altitude=hass.config.elevation,
+        diaspora=DEFAULT_DIASPORA,
+    )
+    old_prefix = get_unique_prefix(location, DEFAULT_LANGUAGE, 20, 50)
+    sample_entity = ent_reg.async_get_or_create(
+        BINARY_SENSORS,
+        DOMAIN,
+        unique_id=f"{old_prefix}_erev_shabbat_hag",
+        suggested_object_id=f"{DOMAIN}_erev_shabbat_hag",
+    )
+    # Save the existing unique_id, DEFAULT_LANGUAGE should be part of it
+    old_unique_id = sample_entity.unique_id
+    assert DEFAULT_LANGUAGE in old_unique_id
+
+    # Simulate HomeAssistant setting up the component
+    assert await async_setup_component(hass, DOMAIN, yaml_conf.copy())
+    await hass.async_block_till_done()
+
+    entries = hass.config_entries.async_entries(DOMAIN)
+    assert len(entries) == 1
+    assert entries[0].data == yaml_conf[DOMAIN]
+
+    # Assert that the unique_id was updated
+    new_unique_id = ent_reg.async_get(sample_entity.entity_id).unique_id
+    assert new_unique_id != old_unique_id
+    assert DEFAULT_LANGUAGE not in new_unique_id
+
+    # Confirm that when the component is reloaded, the unique_id is not changed
+    assert ent_reg.async_get(sample_entity.entity_id).unique_id == new_unique_id
+
+    # Confirm that all the unique_ids are prefixed correctly
+    await hass.config_entries.async_reload(entries[0].entry_id)
+    er_entries = er.async_entries_for_config_entry(ent_reg, entries[0].entry_id)
+    assert all(entry.unique_id.startswith(entries[0].entry_id) for entry in er_entries)