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

Allow entities table to delete helpers #22248

Merged
merged 9 commits into from
Nov 12, 2024

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Oct 5, 2024

Proposed change

If you click on a helper more-info, you are allowed to delete it.

If you select a helper in entities table (w/ multi-selection) and try to delete, the popup tells you it can't be deleted.

This change allows the entities table delete flow to also delete helpers, so it is more consistent (and much easier to clean up large groups of unwanted helpers)

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

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

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.

@bramkragten
Copy link
Member

Not all helpers are config entries, the old style helpers are collection type helpers, like the input helpers. These are not handled in this PR.

@bramkragten
Copy link
Member

Would it maybe make more sense to start adding this to the helper config page (multi delete)?

@karwosts
Copy link
Contributor Author

karwosts commented Oct 7, 2024

These are not handled in this PR.

Are you certain I didn't handle them? I thought I duplicated the same logic that more-info uses to determine deletability.

e.g.:

          if (configEntryId) {
            deleteConfigEntry(this.hass, configEntryId);
          } else {
            removeEntityRegistryEntry(this.hass, entity);
          }

@karwosts
Copy link
Contributor Author

karwosts commented Oct 7, 2024

Would it maybe make more sense to start adding this to the helper config page (multi delete)?

I'd probably be up for that at some point, but I've already got another PR open for helper config with quite a few changes already.

I do think it makes sense to add here though as well, if it's technically feasible.

@bramkragten
Copy link
Member

bramkragten commented Oct 7, 2024

These are not handled in this PR.

Are you certain I didn't handle them? I thought I duplicated the same logic that more-info uses to determine deletability.

e.g.:

          if (configEntryId) {
            deleteConfigEntry(this.hass, configEntryId);
          } else {
            removeEntityRegistryEntry(this.hass, entity);
          }

You would need:

        await HELPERS_CRUD[this.entry.platform].delete(
          this.hass,
          this._item.id
        );

These helpers actually have their own more info settings element entity-settings-helper-tab

@karwosts
Copy link
Contributor Author

karwosts commented Oct 8, 2024

Calling removeEntityRegistryEntry on helpers like input_boolean still seems to work correctly at first glance (removes the entity from the table and I don't find any further vestiges of it).

I wonder what the difference is with the other API method.

But I'll attempt to add the code from that helper-tab file as well.

@karwosts karwosts marked this pull request as draft October 13, 2024 13:48
@karwosts karwosts marked this pull request as ready for review October 14, 2024 14:53
@karwosts
Copy link
Contributor Author

So I did another pass through and I think I've covered all the cases, but this ended up being way more complicated than I originally anticipated 😅 . I imagine it's quite hard to review but since I went through the trouble I'll put it out there.

@silamon
Copy link
Contributor

silamon commented Oct 27, 2024

I still have around 600 helpers that existed to test performance issues some months ago. The yaml configuration has already been remove, but they still remain in the entity table, marked as no longer existing with a red exclamation mark at the end.

These were setup through yaml like this:

counter:
  my_custom_counter:
    initial: 30
    step: 1

At current stable (not this branch), they can be deleted at this time with multi selection.
Sending {type: 'config/entity_registry/remove', entity_id: 'counter.my_custom_counter'}

On this branch, it appears to call right now with multi selection:
Sending {type: 'counter/delete', counter_id: 'my_custom_counter'}
But since the entity no longer exists it fails with
Error {code: 'not_found', message: 'Unable to find counter_id my_custom_counter'}

@karwosts karwosts marked this pull request as draft October 28, 2024 12:49
@karwosts karwosts marked this pull request as ready for review October 28, 2024 22:37
@karwosts
Copy link
Contributor Author

Thanks for checking, I believe I have fixed that case. 👍

silamon
silamon previously approved these changes Nov 12, 2024
@silamon silamon enabled auto-merge (squash) November 12, 2024 18:05
@silamon
Copy link
Contributor

silamon commented Nov 12, 2024

Thanks for checking, I believe I have fixed that case. 👍

Yeah you did. 🥇

@silamon silamon merged commit 28703b3 into home-assistant:dev Nov 12, 2024
11 checks passed
@karwosts karwosts deleted the delete-helpers-entities branch November 12, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants