-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Offer to delete no-longer-recorded statistics #21119
Offer to delete no-longer-recorded statistics #21119
Conversation
WalkthroughWalkthroughThe changes made improve the user interaction for deleting long-term statistics in a developer tool. Confirmation dialogs for deletions are now marked as destructive for greater clarity and safety. Additionally, extra information and a new function call enhance the dialog box. A new info text has been added to the English translations to inform users about the option to delete long-term statistics. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as User Interface
participant HaPanelDevStatistics
User->>UI: Initiate deletion process
UI->>HaPanelDevStatistics: Trigger showConfirmationDialog
HaPanelDevStatistics->>User: Present destructive confirmation dialog
User-->>HaPanelDevStatistics: Confirm deletion
HaPanelDevStatistics->>HaPanelDevStatistics: Execute new function call
In this diagram, the user initiates the deletion process, and the UI triggers the updated Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Additional context usedBiome
Gitleaks
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 2
@@ -381,6 +392,7 @@ class HaPanelDevStatistics extends SubscribeMixin(LitElement) { | |||
{ statistic_id: issue.data.statistic_id } | |||
)}`, | |||
confirmText: this.hass.localize("ui.common.delete"), | |||
destructive: true, |
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.
Consider adding a confirmation step or additional user input before executing a destructive action.
Providing a secondary confirmation or more detailed user information before proceeding with data deletion can prevent accidental data loss and improve user experience.
@@ -282,6 +282,7 @@ | |||
{ statistic_id: issue.data.statistic_id } | |||
)}`, | |||
confirmText: this.hass.localize("ui.common.delete"), | |||
destructive: true, |
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.
Avoid using non-null assertions as they can lead to runtime errors if not properly handled.
- destructive: true,
+ destructive: this.hass.states[issue.data.statistic_id] != null,
This change ensures that the destructive
property is only set to true
when the state actually exists, mitigating potential runtime issues.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
destructive: true, | |
destructive: this.hass.states[issue.data.statistic_id] != null, |
Proposed change
If a user excludes an entity from recording, they probably don't want to keep the historical statistics anymore. Add deleting them as an option to the FIX dialog.
This just replaces one issue with another (entity no longer recorded -> entity not recorded), but at least it cleans up the DB.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
destructive
property to emphasize irreversible actions.