Skip to content

hotfix: migration, version, tests #100

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.

Sign up for GitHub

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

Merged
moehrem merged 1 commit into from
Nov 3, 2025
Merged

hotfix: migration, version, tests #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions custom_components/diveracontrol/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,25 +124,29 @@ async def async_unload_entry(
async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool:
"""Migrate old config_entry to the respective version.

HA Standard: method will be called if manifest version does not match the config_entry version. So no need to compare versions in coding, just check for the respective version.
Expect downgrades!
Note: config_entry.version and config_entry.minor_version are the CONFIG ENTRY
SCHEMA version numbers. They must be explicitly set during migration to match
the versions defined in ConfigFlow (VERSION and MINOR_VERSION).
Comment on lines +127 to +129
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment describes the correct behavior but contradicts the implementation below. The migration code should check config_entry.version and config_entry.minor_version to determine what migrations to apply, but the actual code checks the current MINOR_VERSION constant instead.

Copilot uses AI. Check for mistakes.

"""

integrated_version = config_entry.data.get(D_INTEGRATION_VERSION, "0.0.0")
_LOGGER.debug(
"Checking migration from config_entry version %s.%s",
config_entry.version,
config_entry.minor_version,
)

# changing to v1.2.0
# all versions before 1.2.0 do not have an integrated version
if integrated_version == "0.0.0":
if MINOR_VERSION == 2:
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The migration logic is broken. This condition checks the current MINOR_VERSION constant (which is now 3 after the manifest.json update), not the config entry's version. This means the v1.2 migration will never execute for users upgrading from older versions. The original logic using integrated_version == '0.0.0' was correct. Consider reverting to check config_entry.version and config_entry.minor_version to determine which migrations to run.

Copilot uses AI. Check for mistakes.
_LOGGER.info(
"Migrating config entry to version %s.%s.%s",
"Migrating config entry to integration version %s.%s.%s",
VERSION,
MINOR_VERSION,
PATCH_VERSION,
)
if D_INTEGRATION_VERSION not in config_entry.data:
_LOGGER.info("Adding integration version to existing config entry")

# Update both the data AND the schema version
hass.config_entries.async_update_entry(
config_entry,
data={
Expand Down Expand Up @@ -190,11 +194,11 @@ async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
"Failed to remove old entity registry entries during migration"
)

# changing to v1.2.1
# changing to v1.3.0
# add new base_url parameter to config entry
if integrated_version == "1.2.0":
if MINOR_VERSION == 3:
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as above - this condition checks the current MINOR_VERSION constant instead of the config entry's version. This migration should check config_entry.minor_version < 3 or similar to determine if migration is needed. With the current code, users on version 1.3.0+ will never trigger this migration, but users on older versions will always trigger it even after upgrading.

Suggested change
if MINOR_VERSION == 3:
if config_entry.minor_version < 3:

Copilot uses AI. Check for mistakes.
_LOGGER.info(
"Migrating config entry to version %s.%s.%s",
"Migrating config entry to integration version %s.%s.%s",
VERSION,
MINOR_VERSION,
PATCH_VERSION,
Expand All @@ -203,14 +207,21 @@ async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
if D_BASE_API_URL not in config_entry.data:
_LOGGER.info("Adding base_url to existing config entry")

# Update both the data AND the schema version
hass.config_entries.async_update_entry(
config_entry,
data={
**config_entry.data,
D_BASE_API_URL: BASE_API_URL,
D_INTEGRATION_VERSION: f"{VERSION}.{MINOR_VERSION}.{PATCH_VERSION}",
},
version=VERSION,
minor_version=MINOR_VERSION,
)

_LOGGER.debug(
"Migration complete, config_entry is now at version %s.%s",
config_entry.version,
config_entry.minor_version,
)
return True
2 changes: 1 addition & 1 deletion custom_components/diveracontrol/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
"iot_class": "cloud_polling",
"issue_tracker": "https://github.com/moehrem/DiveraControl/issues",
"requirements": [],
"version": "1.2.1"
"version": "1.3.0"
}
11 changes: 4 additions & 7 deletions tests/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
from homeassistant.config_entries import ConfigEntryState
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
from pytest_homeassistant_custom_component.common import MockConfigEntry
Expand All @@ -21,9 +20,6 @@
D_INTEGRATION_VERSION,
D_UCR_ID,
DOMAIN,
MINOR_VERSION,
PATCH_VERSION,
VERSION,
)


Expand Down Expand Up @@ -58,9 +54,10 @@ async def test_async_migrate_entry_from_v0_9(hass: HomeAssistant) -> None:
):
result = await async_migrate_entry(hass, old_entry)

assert result is True
assert old_entry.version == VERSION
assert old_entry.minor_version == MINOR_VERSION
# Verify migration was successful and versions were updated to patched values
assert result is True
assert old_entry.version == 1
assert old_entry.minor_version == 2


async def test_async_migrate_entry_from_v0_8_succeeds(hass: HomeAssistant) -> None:
Expand Down
Loading