Fix issues with Point #100843

fredrike · 2023-09-25T11:05:09Z

Proposed change

Fixes authentication issues with Point. Also fixes issues with other minor parts.

Changelog: fredrike/pypoint@v2.3.0...v2.3.2

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 Unable to setup authentication with Minut Point #91583
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 Black (black --fast 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 · 2023-09-25T11:18:58Z

homeassistant/components/point/binary_sensor.py

        )
        self._device_name = device_name
        self._async_unsub_hook_dispatcher_connect = None
        self._events = EVENTS[device_name]
        self._attr_unique_id = f"point.{device_id}-{device_name}"
        self._attr_icon = DEVICES[self._device_name].get("icon")
-        self._attr_name = f"{self._name} {device_name.capitalize()}"


This is set in the super().__init__( function.

edenhaus · 2023-09-25T12:28:31Z

homeassistant/components/point/__init__.py

@@ -54,6 +55,7 @@
            {
                vol.Required(CONF_CLIENT_ID): cv.string,
                vol.Required(CONF_CLIENT_SECRET): cv.string,
+                vol.Required(CONF_REDIRECT_URI): cv.string,


For integrations that connect to devices or services, we no longer accept new YAML configurations or changes.

More information about this can be found in Architecture Decision Record:

ADR-0010: https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md#decision

Please consider deprecating the yaml configuration as this integration already has a config flow.

Yes I understand that, but I don't feel I have time right now to fix this. My roadmap for the integration is to move configuration to den config-flow.

As written above, we won't accept any yaml configuration changes.

The code is already importing the yaml config to a config entry. Therefore I suggest implementing the changes for the config entry config only.

In another PR, you should only deprecate the yaml config.

I've removed all references to yaml config.

I meant leaving the yaml configuration untouched.

I reviewed it again and saw that the UI-initialized config flow also requires yaml. Therefore, I suggest deprecating the yaml configuration in another PR first, including moving to the new oauth flow.

After the deprecation PR is merged, this bug-fix PR should be small and easy.

Please implement the yaml depreciation here, including the new oauth flow.

Can I get some guidance on how to depricate yaml?

Please see

Update Fitbit integration to allow UI based configuration #100897

Add config flow to Twitch #93451

https://github.com/home-assistant/core/pulls?q=is%3Apr+sort%3Aupdated-desc+label%3Aconfig-flow+is%3Aclosed

But you want me to split this PR into several?

Adding a config-flow dialog to allow redirect-uri

Depricate yaml

Implement full config-flow (as this pr is doing)

Well it is not possible to authenticate with the current yaml (it requires an uri). Hence we must send more information.

From your message, I assume it has to be one PR.

Adding a config-flow dialog to allow redirect-uri

Depricate yaml

Implement full config-flow (as this pr is doing)

Please make only these changes in this PR and try to keep the PR as small as possible.

home-assistant · 2023-09-25T12:29:24Z

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.

edenhaus · 2023-10-02T13:36:05Z

homeassistant/components/point/__init__.py

@@ -54,6 +55,7 @@
            {
                vol.Required(CONF_CLIENT_ID): cv.string,
                vol.Required(CONF_CLIENT_SECRET): cv.string,
+                vol.Required(CONF_REDIRECT_URI): cv.string,


I meant leaving the yaml configuration untouched.

I reviewed it again and saw that the UI-initialized config flow also requires yaml. Therefore, I suggest deprecating the yaml configuration in another PR first, including moving to the new oauth flow.

After the deprecation PR is merged, this bug-fix PR should be small and easy.

github-actions · 2023-12-12T21:04:36Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

home-assistant bot added bugfix cla-signed code-quality dependency has-tests integration: point by-code-owner Quality Scale: gold labels Sep 25, 2023

fredrike force-pushed the point-redirect_uri branch from a2f0c9a to 9507edc Compare September 25, 2023 11:14

fredrike commented Sep 25, 2023

View reviewed changes

fredrike marked this pull request as ready for review September 25, 2023 11:19

fredrike force-pushed the point-redirect_uri branch from 1c431a8 to 5b297e9 Compare September 25, 2023 11:19

edenhaus requested changes Sep 25, 2023

View reviewed changes

home-assistant bot marked this pull request as draft September 25, 2023 12:29

fredrike force-pushed the point-redirect_uri branch from 5b297e9 to f57d802 Compare September 25, 2023 14:41

fredrike marked this pull request as ready for review September 25, 2023 15:04

home-assistant bot requested a review from edenhaus September 25, 2023 15:04

fredrike force-pushed the point-redirect_uri branch from f57d802 to cad3438 Compare September 25, 2023 19:46

fredrike marked this pull request as draft September 25, 2023 19:52

This was referenced Sep 25, 2023

Update Point doc home-assistant/home-assistant.io#29020

Closed

Update point documentation for full config flow home-assistant/home-assistant.io#29021

Closed

fredrike marked this pull request as ready for review October 2, 2023 06:11

edenhaus requested changes Oct 2, 2023

View reviewed changes

home-assistant bot marked this pull request as draft October 2, 2023 13:36

fredrike added 5 commits October 13, 2023 21:14

fix redirect_uri in auth flow

9e5ba73

fix issue with device_class.capitalize()

2947b66

update config flow

3e8dbd1

update tests

2d5e018

simplify flow, update tests

3b8ada4

fredrike added 2 commits October 13, 2023 21:14

implement full config flow, skip yaml conf

84bda07

type url

c1985e2

fredrike force-pushed the point-redirect_uri branch from ed397c2 to c1985e2 Compare October 13, 2023 19:14

fredrike mentioned this pull request Oct 13, 2023

Point add redirect uri #101967

Closed

20 tasks

github-actions bot added the stale label Dec 12, 2023

github-actions bot closed this Dec 19, 2023

github-actions bot locked and limited conversation to collaborators Dec 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issues with Point #100843

Fix issues with Point #100843

fredrike commented Sep 25, 2023 •

edited

Loading

fredrike Sep 25, 2023

edenhaus Sep 25, 2023

fredrike Sep 25, 2023

edenhaus Sep 27, 2023

fredrike Oct 2, 2023

edenhaus Oct 2, 2023

edenhaus Oct 4, 2023

fredrike Oct 5, 2023

edenhaus Oct 5, 2023

fredrike Oct 5, 2023

edenhaus Oct 11, 2023

home-assistant bot commented Sep 25, 2023

edenhaus Oct 2, 2023

github-actions bot commented Dec 12, 2023

Fix issues with Point #100843

Fix issues with Point #100843

Conversation

fredrike commented Sep 25, 2023 • edited Loading

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

home-assistant bot commented Sep 25, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 12, 2023

fredrike commented Sep 25, 2023 •

edited

Loading