Bugfix: Electric Kiwi #135231

mikey0000 · 2025-01-09T19:53:53Z

…Assistant Cloud

Breaking change

Proposed change

Fix unique id and migrate, fix small issue where title would be Home Assistant Cloud, now is the customer id

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

…Assistant Cloud

joostlek · 2025-01-09T21:26:26Z

homeassistant/components/electric_kiwi/__init__.py

@@ -24,6 +24,8 @@

 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
    """Set up Electric Kiwi from a config entry."""
+    assert entry.unique_id is not None


So why do we assert it here if we are going to set it down below

I've removed the check further below.

Okay so now we do the migrate so in theory this is true, but I also don't see any reason why we need this here

joostlek · 2025-01-09T21:26:39Z

homeassistant/components/electric_kiwi/__init__.py

+    if entry.unique_id is not None and entry.unique_id.startswith(DOMAIN):
+        ek_session = await ek_api.get_active_session()
+        unique_id = "_".join(str(num) for num in ek_session.customer_numbers)
+        hass.config_entries.async_update_entry(entry, unique_id=unique_id)


Let's do a minor version migration

Also double check if you have any entities that need a migration of unique id

entities are fine, I checked them, I set them up properly the first time :) , minor version migration, let me go look into that.

joostlek · 2025-01-09T21:27:54Z

homeassistant/components/electric_kiwi/config_flow.py

+            return self.async_abort(reason="no_customers")
+
+        unique_id = "_".join(str(num) for num in session.customer_numbers)
+        existing_entry = await self.async_set_unique_id(unique_id)


if we have an unique id, we should use this id to also abort if it is already setup, and when reauthentiucating we should make sure that the unique id is the same

abort if configured added

But what about reauth? Please check an integration like withings for that

You missed the self._abort_if_unique_id_mismatch(reason="wrong_account") where someone reauths with a completely different account

joostlek · 2025-01-09T21:28:21Z

homeassistant/components/electric_kiwi/quality_scale.yaml

Let's add this in a follow up and please check all the rules yourself so the file is synced with the codebase

joostlek · 2025-01-09T22:10:25Z

homeassistant/components/electric_kiwi/__init__.py

+
+            ek_session = await ek_api.get_active_session()
+            unique_id = "_".join(str(num) for num in ek_session.customer_numbers)
+            hass.config_entries.async_update_entry(config_entry, unique_id=unique_id)


Let's also bump and check the minor version as well (next to version)

where are the rules around config entry version bumping?

I would assume they're in the documentation somewhere, but your guess is as good as mine.

Major version is breaking change (and makes integrations unable to load if a newer version is detected), minor version change is able to load if a newer version is found.

So if someone reverts back from 2025.2 to 2025.1, a major version change would not load, minor version change would

Yeah major it is since multiple added EK's wouldn't work if rolled back potentially.

joostlek · 2025-01-09T22:10:55Z

homeassistant/components/electric_kiwi/config_flow.py

+            return self.async_abort(reason="no_customers")
+
+        unique_id = "_".join(str(num) for num in session.customer_numbers)
+        existing_entry = await self.async_set_unique_id(unique_id)


But what about reauth? Please check an integration like withings for that

joostlek · 2025-01-10T11:31:53Z

homeassistant/components/electric_kiwi/__init__.py

@@ -24,6 +24,8 @@

 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
    """Set up Electric Kiwi from a config entry."""
+    assert entry.unique_id is not None


Okay so now we do the migrate so in theory this is true, but I also don't see any reason why we need this here

joostlek · 2025-01-10T11:32:24Z

homeassistant/components/electric_kiwi/__init__.py

+async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool:
+    """Migrate old entry."""
+    # convert title and unique_id to string
+    if config_entry.version == 1 and config_entry.minor_version == 0:


Minor version also starts with 1

joostlek · 2025-01-10T11:32:42Z

homeassistant/components/electric_kiwi/__init__.py

+            ek_session = await ek_api.get_active_session()
+            unique_id = "_".join(str(num) for num in ek_session.customer_numbers)
+            hass.config_entries.async_update_entry(
+                config_entry, unique_id=unique_id, title=unique_id, version=2


We should avoid updating titles as they are considered user land

joostlek · 2025-01-10T11:32:54Z

homeassistant/components/electric_kiwi/__init__.py

+            )
+
+            ek_session = await ek_api.get_active_session()
+            unique_id = "_".join(str(num) for num in ek_session.customer_numbers)


what are customer_numbers

joostlek · 2025-01-10T11:33:51Z

homeassistant/components/electric_kiwi/config_flow.py

+            return self.async_abort(reason="no_customers")
+
+        unique_id = "_".join(str(num) for num in session.customer_numbers)
+        existing_entry = await self.async_set_unique_id(unique_id)


You missed the self._abort_if_unique_id_mismatch(reason="wrong_account") where someone reauths with a completely different account

home-assistant · 2025-01-10T11:34:09Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

joostlek · 2025-01-10T11:34:31Z

But I am missing, why do we do this? What was the unique id before and what will it be now?

fix unique id and migrate, fix small issue where title would be Home …

a268c52

…Assistant Cloud

home-assistant bot added cla-signed integration: electric_kiwi by-code-owner Quality Scale: No score quality-scale labels Jan 9, 2025

Merge branch 'dev' into ek-fixes

6249f54

mikey0000 marked this pull request as draft January 9, 2025 19:58

fix tests

747be15

joostlek requested changes Jan 9, 2025

View reviewed changes

mikey0000 added 3 commits January 10, 2025 10:54

remove quality scale for now and add abort if configured

f63c1e8

add version

538eaba

add version migration

e6330f9

mikey0000 requested a review from joostlek January 9, 2025 22:07

mikey0000 marked this pull request as ready for review January 9, 2025 22:08

joostlek reviewed Jan 9, 2025

View reviewed changes

mikey0000 added 2 commits January 10, 2025 11:27

add check for reauth

879d30a

check minor version

42852f0

mikey0000 requested a review from joostlek January 9, 2025 22:29

update the title while we are at it

48bf863

joostlek requested changes Jan 10, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 10, 2025 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix: Electric Kiwi #135231

Bugfix: Electric Kiwi #135231

mikey0000 commented Jan 9, 2025 •

edited

Loading

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 10, 2025

joostlek Jan 9, 2025

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 10, 2025

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 9, 2025

mikey0000 Jan 9, 2025

joostlek Jan 9, 2025

joostlek Jan 10, 2025

joostlek Jan 10, 2025

joostlek Jan 10, 2025

joostlek Jan 10, 2025

joostlek Jan 10, 2025

home-assistant bot commented Jan 10, 2025

joostlek commented Jan 10, 2025

Bugfix: Electric Kiwi #135231

Are you sure you want to change the base?

Bugfix: Electric Kiwi #135231

Conversation

mikey0000 commented Jan 9, 2025 • edited Loading

Breaking change

Proposed change

Type of change

Additional information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

home-assistant bot commented Jan 10, 2025

joostlek commented Jan 10, 2025

mikey0000 commented Jan 9, 2025 •

edited

Loading