Use new reauth helpers in yale_smart_alarm #128836
Conversation
home-assistant
bot
added
cla-signed
code-quality
Hacktoberfest
integration: yale_smart_alarm
small-pr
|
Hey there @gjohansson-ST, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| await self.hass.config_entries.async_reload(existing_entry.entry_id) | ||
| return self.async_abort(reason="reauth_successful") | ||
| await self.async_set_unique_id(username) | ||
| self._abort_if_unique_id_mismatch() |
There was a problem hiding this comment.
Note: the previous behavior would create a new entry, or updated the other entry if the username changed.
Now it aborts instead.
There was a problem hiding this comment.
We should update the unique id. Not abort.
There was a problem hiding this comment.
Why? Unique id should stay consistent right
There was a problem hiding this comment.
We should update the unique id. Not abort.
That could be done by the code owner either to replace this PR or as a follow-up.
I think the old behavior was incorrect regardless.
There was a problem hiding this comment.
I am the code owner for this one and I also realized with your PR that the previous behavior was incorrect hence clarify to update rather than abort.
I can do a follow-up with that's what you prefer.
There was a problem hiding this comment.
Why? Unique id should stay consistent right
Unique id is the email address (username) which the user can change hence we need to be able to update it if the user changes it.
There was a problem hiding this comment.
But then that unique_id is not a good unique_id
There was a problem hiding this comment.
We should update the unique id. Not abort.
Done
There was a problem hiding this comment.
I'm reconsidering this one. I don't think we should allow to change the username (given it allows to switch account completely which I assume is what @joostlek meant.)
@epenet i suggest you revert to the original you had and then in a follow-up we should remove the username input to only have the password. Thanks
| @@ -2,7 +2,8 @@ | |||
| "config": { | |||
| "abort": { | |||
| "already_configured": "[%key:common::config_flow::abort::already_configured_account%]", | |||
| "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" | |||
| "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", | |||
| "unique_id_mismatch": "The user identifier does not match the previous identifier" | |||
There was a problem hiding this comment.
What I miss in this error is, what must the user do to fix this? Okay, my identifier mismatched, but users never had to do anything with an identifier
Proposed change
SSIA, linked to home-assistant/developers.home-assistant#2356
Note: also prevent the username from changing
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: