frontend
frontend copied to clipboard
home-assistant
Prompt user to remove application credentials when deleting the integration configuration
Proposed change
Prompt user to remove application credentials when deleting the integration configuration. This is to avoid user confusion where they delete the integration thinking they have a clean slate, but still have application credentials hidden (they don't know where to look in the overflow menu yet).
Related Core PR: https://github.com/home-assistant/core/pull/74825
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [X] New feature (thank you!)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Example configuration
Additional information
- This PR fixes or closes issue:
- This PR is related to issue or discussion: https://github.com/home-assistant/core/issues/74552 https://github.com/home-assistant/home-assistant.io/issues/23330
- Link to documentation pull request:
Checklist
- [X] The code change is tested and works locally.
- [X] There is no commented out code in this PR.
- [ ] 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
As an end user I would be puzzled by this dialog and what it's asking of me. What are the ramifications of picking either cancel or remove and how does it affect "my" future?
I think it needs to explain what the differences are between the 2 choices or just always remove credentials if they're no longer needed?
As an end user I would be puzzled by this dialog and what it's asking of me. What are the ramifications of picking either cancel or remove and how does it affect "my" future?
I think it needs to explain what the differences are between the 2 choices or just always remove credentials if they're no longer needed?
Thanks that is very helpful feedback.
So, let me see if i can explain the choice here, then we can discuss the wording.
- If you remove them, then you are prompted to re-add them next time the integration is setup. Maybe you have them handy or if not then you'll see a link of where to go to find them on the google cloud side
- If you leave them, they will be automatically re-used when setting up the integration so you don't have to re-enter them again which is handy.
There is a desire to have integrations and the credentials operate separately, certainly at the schema level, but it does create this user tension of having to know they MAY operate independently even if they don't care, so as an intermediate step I at least want to raise awareness these exist. Note that you can also use the credentials for multiple config entries.
So, maybe we want to rename "Cancel" to "Keep".
Maybe something like this is better?
Would you like to keep or remove them? If you keep them, they will automatically be used when setting up the integration again in the future. If you remove them, you will be prompted to re-enter the credentials when setting up the integration again in the future.
I'd also love input on improvements.
Two initial ideas:
- Add a link (labeled "Help me decide"?) at the bottom of the dialog pointing to a FAQ in the HA docs where we explain what you wrote in your comment above.
- Or if we do not want the online dependency to HA, perhaps an expandable/collapsible section in the dialog where then the explanation is shown. That way the dialog stays nice/small/simple, but offers more context if needed.
I also was recently looking at this ticket https://github.com/home-assistant/frontend/issues/9773 that I wanted to tackle at some point, but this credentials dialog here made me pause: "Delete" for an integration is fine, but in the case of the credentials we are technically only "removing" them from HA as the credentials themselves (configured on some other platform outside HA) of course stay valid and are not "deleted" or invalidated by that action.
So not sure if that is a distinction we have to care about?
I also was recently looking at this ticket #9773 that I wanted to tackle at some point, but this credentials dialog here made me pause: "Delete" for an integration is fine, but in the case of the credentials we are technically only "removing" them from HA as the credentials themselves (configured on some other platform outside HA) of course stay valid and are not "deleted" or invalidated by that action.
So not sure if that is a distinction we have to care about?
Hi, my interpretation is that you are saying the text should say "remove" when referring to application credentials which is what it is doing now. Am i understanding that right? or are you pointing out that this is saying remove which is inconsistent with delete?
I am saying that your currently chosen wording makes sense to me, but I remember that there is the linked issue where a rename was proposed, but I just realized, that this ticket is purely talking about "entities", so probably does not really apply here. I have pinged @matthiasdebaat to chime in to confirm.
I am saying that your currently chosen wording makes sense to me, but I remember that there is the linked issue where a rename was proposed, but I just realized, that this ticket is purely talking about "entities", so probably does not really apply here. I have pinged @matthiasdebaat to chime in to confirm.
OK thanks for pointing this out. Definitely happy to defer to the experts here and follow whatever pattern is recommended.
I've updated dialog text close to what I described above, then also added a Learn about application credentials link to the integration docs. Here is the updated view:
This has also been updated to reflect the changes in core, moving the logic for checking the config entry into a separate websocket API.
As commented here. We should follow the guidelines of this is great article about Delete vs. Remove.
In this case we remove credentials from Home Assistant, they kept in existence on the service you've created the credentials on.
Backend PR was merged: https://github.com/home-assistant/core/pull/74825