Skip to content
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

Better disabled/error handling on config/helpers page #22237

Merged
merged 5 commits into from
Nov 9, 2024

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Oct 4, 2024

Proposed change

Changes:

  • Disabled helper entities now show a disabled icon, show an entity id, and load the more-info when you click them. Previous behavior was to show an error icon, entity id was empty, and clicking would show OptionsFlow. This meant for disabled helpers it was really unclear why they were showing an error and there was no obvious way to fix it.

image

  • When adding a new helper, add a call to refetch entity sources. Previously after adding a new entity like a new integral sensor, it would just remain in an error state until the page was manually refreshed.

  • For config entries with an error, add an error info item, and a delete item to the overflow menu:

image

First option will show the reason for the error (which I think is not otherwise visible anywhere):

image

Second option will allow to delete the config entry, as I think there is no other current way to delete a broken helper. Only way I know is to go to the hidden integration page for the helper, which is only accessible by knowing and typing in the URL because we hide it otherwise.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • 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

Checklist

  • The code change is tested and works locally.
  • 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:

This comment was marked as off-topic.

message.entry.state === "loaded" &&
this._configEntries[message.entry.entry_id]?.state !== "loaded"
) {
this._debouncedFetchEntitySources();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to add some delay here because I think this is racy, if I just refetch sources here immediately after state changes to "loaded" about 10% of the time the newly loaded entity would not be present in the sources list returned from core.

Comment on lines +410 to +422
...(helper.configEntry &&
helper.editable &&
ERROR_STATES.includes(helper.configEntry.state) &&
helper.entity === undefined
? [
{
path: mdiTrashCan,
label: this.hass.localize("ui.common.delete"),
warning: true,
action: () => this._deleteEntry(helper),
},
]
: []),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added delete option here only for helpers in the error state, but was there a reason we don't just allow delete for all helpers here?

src/translations/en.json Outdated Show resolved Hide resolved
silamon
silamon previously approved these changes Oct 30, 2024
Copy link
Contributor

@silamon silamon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't answer to open question though about the need for a delay.

@kgn3400
Copy link

kgn3400 commented Oct 30, 2024

Hi,
When will this PR be released/merged?

Best Regards
Kurt G. Nielsen

@silamon silamon merged commit e183047 into home-assistant:dev Nov 9, 2024
15 checks passed
@karwosts karwosts deleted the helpers-fix-broken branch November 10, 2024 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment