hotfix: migration, version, tests #100
Conversation
Changes to be committed: modified: custom_components/diveracontrol/__init__.py modified: custom_components/diveracontrol/manifest.json modified: tests/test_init.py
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the migration logic to use direct checks against MINOR_VERSION constants and updates the integration version from 1.2.1 to 1.3.0.
Key Changes:
- Changed migration logic to check
MINOR_VERSIONconstant directly instead of checkingintegrated_versionfrom config entry data - Updated manifest version from 1.2.1 to 1.3.0
- Removed unused imports from test file and hardcoded version assertions in tests
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| custom_components/diveracontrol/init.py | Refactored migration logic to check MINOR_VERSION constant, added integration version update to v1.3.0 migration, and improved comments/logging |
| custom_components/diveracontrol/manifest.json | Bumped version from 1.2.1 to 1.3.0 |
| tests/test_init.py | Removed unused imports and changed assertions to use hardcoded values matching patched VERSION/MINOR_VERSION |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 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: |
There was a problem hiding this comment.
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.
| # changing to v1.3.0 | ||
| # add new base_url parameter to config entry | ||
| if integrated_version == "1.2.0": | ||
| if MINOR_VERSION == 3: |
There was a problem hiding this comment.
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.
| if MINOR_VERSION == 3: | |
| if config_entry.minor_version < 3: |
| 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). |
There was a problem hiding this comment.
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.
Changes to be committed:
modified: custom_components/diveracontrol/init.py
modified: custom_components/diveracontrol/manifest.json
modified: tests/test_init.py