core: frontend: SettingsView: Prevent system log delete while downloading#3782
Open
patrickelectric wants to merge 1 commit intobluerobotics:masterfrom
Open
core: frontend: SettingsView: Prevent system log delete while downloading#3782patrickelectric wants to merge 1 commit intobluerobotics:masterfrom
patrickelectric wants to merge 1 commit intobluerobotics:masterfrom
Conversation
…ding Disable delete button during download and download button during deletion to prevent corrupt or failed log operations. Closes bluerobotics#3284 Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePrevents users from deleting system logs while they are being downloaded by tracking download state and wiring it into the log download and delete controls. Sequence diagram for system log download and guarded deletionsequenceDiagram
actor User
participant SettingsView
participant filebrowser
User->>SettingsView: click download_service_log_files
SettingsView->>SettingsView: set downloading = true
SettingsView->>filebrowser: fetchFolder(system_logs)
filebrowser-->>SettingsView: Folder
SettingsView->>filebrowser: downloadFolder(Folder)
filebrowser-->>SettingsView: Download complete
SettingsView->>SettingsView: set downloading = false
User->>SettingsView: click remove_service_log_files
SettingsView->>SettingsView: Check disable_remove, deletion_in_progress, downloading
SettingsView-->>User: Button disabled if any flag is true
SettingsView->>SettingsView: If enabled, start deletion (set deletion_in_progress = true)
Class diagram for SettingsView state and methods related to log download and deletionclassDiagram
class SettingsView {
// data
boolean deletion_in_progress
boolean downloading
boolean disable_remove
// methods
async download_service_log_files()
async remove_service_log_files()
}
class filebrowser {
async fetchFolder(name)
async downloadFolder(folder)
}
SettingsView --> filebrowser: uses
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider also disabling the download button when
downloadingis true (e.g.:disabled="deletion_in_progress || downloading") to prevent multiple concurrent download requests via repeated clicks. - If
remove_service_log_filescan be triggered from anywhere other than this button, it may be safer to add a runtime guard (e.g. early return whenthis.downloadingis true) so that deletions cannot start while a download is in progress even if the UI constraint is bypassed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider also disabling the download button when `downloading` is true (e.g. `:disabled="deletion_in_progress || downloading"`) to prevent multiple concurrent download requests via repeated clicks.
- If `remove_service_log_files` can be triggered from anywhere other than this button, it may be safer to add a runtime guard (e.g. early return when `this.downloading` is true) so that deletions cannot start while a download is in progress even if the UI constraint is bypassed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Closes #3284
Summary by Sourcery
Prevent system logs from being deleted while a log download is in progress by tracking download state and disabling relevant actions accordingly.
Bug Fixes:
Enhancements: