Add support for native oauth2 in Point #118243

fredrike · 2024-05-27T12:23:17Z

Proposed change

Add support for native oauth2 in Point integration.

Type of change

Change log of pypoint: fredrike/pypoint@v2.3.2...v3.0.0

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 Unable to setup authentication with Minut Point #91583 Unable to setup Minut Point home-assistant.io#34184
This PR is related to issue: Fix issues with Point #100843, Point add redirect uri #101967
Link to documentation pull request: Add documentation for Point oauth2 home-assistant.io#32971

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.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

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

fredrike · 2024-05-27T20:29:57Z

@edenhaus we had a discussion a while ago #100843 (comment) regarding oauth2 in point integration. I did start from scratch with the config_flow and this works fine. The problem is now to migrate existing yaml configuration and using existing entries in core.config_entries. Can you point me in a direction where this is done successfully?

If I have an old entry in core.config_entries this codes fails with this error:

2024-05-27 22:27:46.344 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry Minut Point for point
Traceback (most recent call last):
  File "/Users/fer/Documents/github/home-assistant/homeassistant/config_entries.py", line 594, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fer/Documents/github/home-assistant/homeassistant/components/point/__init__.py", line 113, in async_setup_entry
    await config_entry_oauth2_flow.async_get_config_entry_implementation(
  File "/Users/fer/Documents/github/home-assistant/homeassistant/helpers/config_entry_oauth2_flow.py", line 428, in async_get_config_entry_implementation
    implementation = implementations.get(config_entry.data["auth_implementation"])
                                         ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'auth_implementation'

Please advise how I can build a seemless update to the native oauth2 flow.

homeassistant/components/point/__init__.py

fredrike · 2024-05-29T08:04:55Z

@edenhaus we had a discussion a while ago #100843 (comment) regarding oauth2 in point integration. I did start from scratch with the config_flow and this works fine. The problem is now to migrate existing yaml configuration and using existing entries in core.config_entries. Can you point me in a direction where this is done successfully?

If I have an old entry in core.config_entries this codes fails with this error:
2024-05-27 22:27:46.344 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry Minut Point for point
Traceback (most recent call last):
  File "/Users/fer/Documents/github/home-assistant/homeassistant/config_entries.py", line 594, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fer/Documents/github/home-assistant/homeassistant/components/point/__init__.py", line 113, in async_setup_entry
    await config_entry_oauth2_flow.async_get_config_entry_implementation(
  File "/Users/fer/Documents/github/home-assistant/homeassistant/helpers/config_entry_oauth2_flow.py", line 428, in async_get_config_entry_implementation
    implementation = implementations.get(config_entry.data["auth_implementation"])
                                         ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'auth_implementation'
Please advise how I can build a seemless update to the native oauth2 flow.

I've managed to solve this.

edenhaus · 2024-06-04T11:46:56Z

Please don't ping members asking for a review. This is not allowed.
Please be patient and wait until someone is reviewing your PR

robkirk · 2024-06-25T08:40:18Z

Any news on when this will get reviewed? This integration has been broken for over a year and Fredrike has kindly produced a suggested patch to fix it.

edenhaus

Thanks for bumping the dependency! However, could you please update the PR description to contain at least one (or multiple) of the following:

A link to the release notes of this package version, and all versions in between.
A link to the changelog of this package.
A link to a Git(Hub) diff/compare view from the current version to the bumped version.

This allows us to review upstream changes, which is needed to decide this change is working as intended and/or if we can include it in, for example, a patch release of Home Assistant.

Thanks already! 👍

home-assistant · 2024-06-26T13:45:06Z

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.

fredrike · 2024-06-26T14:49:59Z

Thanks for bumping the dependency! However, could you please update the PR description to contain at least one (or multiple) of the following:

A link to the release notes of this package version, and all versions in between.

A link to the changelog of this package.

A link to a Git(Hub) diff/compare view from the current version to the bumped version.

This allows us to review upstream changes, which is needed to decide this change is working as intended and/or if we can include it in, for example, a patch release of Home Assistant.

Thanks already! 👍

Here is the changed code in pypoint fredrike/pypoint@v2.3.2...v3.0.0

edenhaus · 2024-07-05T14:55:23Z

Sanity check pypoint==3.0.0 ✔️

edenhaus

Please fix the merge conflicts and make sure the CI is passinh

joostlek

Tests are failing, can you take a look?

fredrike · 2024-09-19T10:48:03Z

Tests are failing, can you take a look?

Sure, are we happy with the rest of the integration so far?

…/home-assistant into point-oauth

homeassistant/components/point/config_flow.py

tests/components/point/test_config_flow.py

Changes adressed

* initial oauth2 implementation * fix unload_entry * read old yaml/entry config * update tests * fix: pylint on tests * Apply suggestions from code review Co-authored-by: Robert Resch <robert@resch.dev> * fix constants, formatting * use runtime_data * Apply suggestions from code review Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com> * fix missing import * adopt to PointData dataclass * fix typing * add more strings (copied from weheat) * move the PointData dataclass to avoid circular imports * use configflow inspired by withings * raise ConfigEntryAuthFailed * it is called entry_lock * fix webhook issue * fix oauth_create_entry * stop using async_forward_entry_setup * Fixup * fix strings * fix issue that old config might be without unique_id * parametrize tests * Update homeassistant/components/point/config_flow.py * Update tests/components/point/test_config_flow.py * Fix --------- Co-authored-by: Robert Resch <robert@resch.dev> Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

MartinHjelmare · 2024-09-21T09:58:08Z

homeassistant/components/point/__init__.py

            async_dispatcher_send(
                self._hass, POINT_DISCOVERY_NEW.format(platform, DOMAIN), device_id
            )

        self._is_available = True
        for home_id in self._client.homes:
            if home_id not in self._known_homes:
+                await self._hass.config_entries.async_forward_entry_setups(


We're only allowed to forward the config entry to a platform once until the config entry is unloaded. This looks like it could happen more than once for this config entry.

What if more devices are added, what is the best approach for adding more devices on the fly?

You already have a way to add new entities from new devices using the dispatch helper.

MartinHjelmare · 2024-09-21T09:58:21Z

homeassistant/components/point/__init__.py

                await new_device(home_id, "alarm_control_panel")
                self._known_homes.add(home_id)
        for device in self._client.devices:
            if device.device_id not in self._known_devices:
+                await self._hass.config_entries.async_forward_entry_setups(


* initial oauth2 implementation * fix unload_entry * read old yaml/entry config * update tests * fix: pylint on tests * Apply suggestions from code review Co-authored-by: Robert Resch <robert@resch.dev> * fix constants, formatting * use runtime_data * Apply suggestions from code review Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com> * fix missing import * adopt to PointData dataclass * fix typing * add more strings (copied from weheat) * move the PointData dataclass to avoid circular imports * use configflow inspired by withings * raise ConfigEntryAuthFailed * it is called entry_lock * fix webhook issue * fix oauth_create_entry * stop using async_forward_entry_setup * Fixup * fix strings * fix issue that old config might be without unique_id * parametrize tests * Update homeassistant/components/point/config_flow.py * Update tests/components/point/test_config_flow.py * Fix --------- Co-authored-by: Robert Resch <robert@resch.dev> Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

home-assistant bot added bugfix cla-signed code-quality dependency integration: point by-code-owner Quality Scale: silver labels May 27, 2024

fredrike mentioned this pull request May 28, 2024

Add documentation for Point oauth2 home-assistant/home-assistant.io#32971

Merged

8 tasks

fredrike added 2 commits May 28, 2024 11:26

initial oauth2 implementation

6c59c80

fix unload_entry

cfdf377

fredrike force-pushed the point-oauth branch from 957bafe to cfdf377 Compare May 28, 2024 09:26

read old yaml/entry config

059b1d1

fredrike commented May 28, 2024

View reviewed changes

homeassistant/components/point/__init__.py Outdated Show resolved Hide resolved

fredrike added 2 commits May 28, 2024 19:26

Merge branch 'dev' into point-oauth

a57b284

update tests

2fb90fe

fredrike marked this pull request as ready for review May 29, 2024 07:29

edenhaus requested changes Jun 26, 2024

View reviewed changes

home-assistant bot marked this pull request as draft June 26, 2024 13:45

fredrike marked this pull request as ready for review June 26, 2024 14:50

home-assistant bot requested a review from edenhaus June 26, 2024 14:50

Merge remote-tracking branch 'upstream/dev' into point-oauth

b97699d

edenhaus requested changes Jul 5, 2024

View reviewed changes

home-assistant bot marked this pull request as draft July 5, 2024 14:55

joostlek requested changes Sep 19, 2024

View reviewed changes

home-assistant bot marked this pull request as draft September 19, 2024 08:03

fredrike added 4 commits September 19, 2024 11:05

it is called entry_lock

d510cb9

fix webhook issue

e29dc23

fix oauth_create_entry

72ed8ac

stop using async_forward_entry_setup

f407cc9

joostlek added 3 commits September 19, 2024 13:20

Merge branch 'dev' into point-oauth

716c7ac

Fixup

26a0682

Merge branch 'dev' into point-oauth

7480149

joostlek force-pushed the point-oauth branch from 7480149 to 26a0682 Compare September 19, 2024 12:10

Merge branches 'point-oauth' and 'point-oauth' of github.com:fredrike…

bad8fb7

…/home-assistant into point-oauth

fredrike marked this pull request as ready for review September 19, 2024 12:38

home-assistant bot requested a review from joostlek September 19, 2024 12:39

fix strings

f07d34d

fredrike force-pushed the point-oauth branch from 8f29560 to f07d34d Compare September 19, 2024 12:40

fredrike added 2 commits September 19, 2024 19:40

fix issue that old config might be without unique_id

b3979a4

parametrize tests

58a21ea

joostlek reviewed Sep 20, 2024

View reviewed changes

homeassistant/components/point/config_flow.py Outdated Show resolved Hide resolved

tests/components/point/test_config_flow.py Outdated Show resolved Hide resolved

joostlek added 3 commits September 20, 2024 11:29

Update homeassistant/components/point/config_flow.py

6b7bd29

Update tests/components/point/test_config_flow.py

b9bcb93

Fix

0a8e7ef

joostlek approved these changes Sep 20, 2024

View reviewed changes

joostlek merged commit 1768daf into home-assistant:dev Sep 20, 2024
43 of 44 checks passed

joostlek deleted the point-oauth branch September 20, 2024 10:02

MartinHjelmare reviewed Sep 21, 2024

View reviewed changes

github-actions bot locked and limited conversation to collaborators Sep 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for native oauth2 in Point #118243

Add support for native oauth2 in Point #118243

fredrike commented May 27, 2024 •

edited

Loading

fredrike commented May 27, 2024

fredrike commented May 29, 2024

edenhaus commented Jun 4, 2024

robkirk commented Jun 25, 2024

edenhaus left a comment

home-assistant bot commented Jun 26, 2024

fredrike commented Jun 26, 2024

edenhaus commented Jul 5, 2024

edenhaus left a comment •

edited

Loading

joostlek left a comment

fredrike commented Sep 19, 2024

MartinHjelmare Sep 21, 2024 •

edited

Loading

fredrike Sep 22, 2024

MartinHjelmare Sep 22, 2024

MartinHjelmare Sep 21, 2024

Add support for native oauth2 in Point #118243

Add support for native oauth2 in Point #118243

Conversation

fredrike commented May 27, 2024 • edited Loading

Proposed change

Type of change

Additional information

Checklist

fredrike commented May 27, 2024

fredrike commented May 29, 2024

edenhaus commented Jun 4, 2024

robkirk commented Jun 25, 2024

edenhaus left a comment

Choose a reason for hiding this comment

home-assistant bot commented Jun 26, 2024

fredrike commented Jun 26, 2024

edenhaus commented Jul 5, 2024

edenhaus left a comment • edited Loading

Choose a reason for hiding this comment

joostlek left a comment

Choose a reason for hiding this comment

fredrike commented Sep 19, 2024

MartinHjelmare Sep 21, 2024 • edited Loading

Choose a reason for hiding this comment

fredrike Sep 22, 2024

Choose a reason for hiding this comment

MartinHjelmare Sep 22, 2024

Choose a reason for hiding this comment

MartinHjelmare Sep 21, 2024

Choose a reason for hiding this comment

fredrike commented May 27, 2024 •

edited

Loading

edenhaus left a comment •

edited

Loading

MartinHjelmare Sep 21, 2024 •

edited

Loading