-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
Add new helper for matching reauth/reconfigure config flows #127565
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.
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
Conversation
Hey there @frenck, @joostlek, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
self.source == SOURCE_REAUTH | ||
and self._get_reauth_entry().unique_id != self.unique_id | ||
) or ( | ||
self.source == SOURCE_RECONFIGURE | ||
and self._get_reconfigure_entry().unique_id != self.unique_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we should assume the unique ID is being used for detecting these cases.
Can you elaborate on that? There is nothing in the PR description that explain the foundation behind it. |
It's a pattern I've bumped into a few times. Axis and abode for example use this pattern. |
It's also partially linked to #127527 |
Here's another example I've just bumped into, which I think would benefit from this: |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I find the use-case/gain to shallow to add a helper. While helpers can be helpful, they can also make code harder to read / harder to follow (as one has to jump between more logic).
I'm not sure on this one. Going to ask for a second opinion from another reviewer.
../Frenck
One more note: it is a mirror to |
I think it's nice since it will help integrations to do the right thing. I think there are many integrations that have reauth flows where they don't check that the flow is reauthenticating matching flow data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @epenet 👍
../Frenck
Breaking change
Proposed change
When using
reauth
/reconfigure
, there is currently no easy way to check that theunique_id
of the original entry still matches what the newunique_id
would be with the latest input.This is especially obvious when
reauth
/reconfigure
flows reuse existing functionnality fromuser
flow.I propose that we add this helper to make it easier to bail out if there is a mismatch:
Sample use:
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: