frontend: settings: ask for confirmation when removing logs#3825
Merged
patrickelectric merged 1 commit intobluerobotics:masterfrom Mar 24, 2026
Merged
Conversation
Reviewer's GuideAdds a confirmation dialog before clearing service or MAVLink logs in the Settings view, wiring the existing log removal actions through a shared warning-dialog with type-specific titles and messages and ensuring state is reset after operations complete. Sequence diagram for new log clear confirmation flowsequenceDiagram
actor User
participant SettingsView
participant WarningDialog
participant ServiceLogRemoval
participant MavlinkLogRemoval
User->>SettingsView: Click Clear service logs button
SettingsView->>SettingsView: pending_log_clear_type = service
SettingsView->>SettingsView: show_log_clear_confirm = true
SettingsView->>WarningDialog: Bind v_model(show_log_clear_confirm), title, message
User->>WarningDialog: Click Clear logs (confirm)
WarningDialog-->>SettingsView: confirm event
SettingsView->>SettingsView: onConfirmClearLogs()
alt pending_log_clear_type is service
SettingsView->>ServiceLogRemoval: remove_service_log_files()
ServiceLogRemoval-->>SettingsView: deletion_complete
else pending_log_clear_type is mavlink
SettingsView->>MavlinkLogRemoval: remove_mavlink_log_files()
MavlinkLogRemoval-->>SettingsView: deletion_complete
end
SettingsView->>SettingsView: show_log_clear_confirm = false
SettingsView->>SettingsView: pending_log_clear_type = null
Class diagram for updated SettingsView log clearing logicclassDiagram
class SettingsView {
<<VueComponent>>
// data
bool show_log_clear_confirm
string|null pending_log_clear_type
string current_deletion_path
number current_deletion_size
string current_deletion_status
bool operation_in_progress
// computed
string log_clear_confirm_title()
string log_clear_confirm_message()
// methods
void onConfirmClearLogs()
Promise reset_settings()
Promise remove_service_log_files()
Promise remove_mavlink_log_files()
}
class WarningDialog {
<<VueComponent>>
// props
bool value
string title
string message
string confirmLabel
// events
void confirm()
void input(bool newValue)
}
SettingsView "1" o-- "1" WarningDialog : uses_v_model_and_confirm
class ServiceLogRemoval {
void remove_service_log_files()
}
class MavlinkLogRemoval {
void remove_mavlink_log_files()
}
SettingsView ..> ServiceLogRemoval : delegates_service_log_clear
SettingsView ..> MavlinkLogRemoval : delegates_mavlink_log_clear
class LogClearState {
bool show_log_clear_confirm
string|null pending_log_clear_type
void reset_state()
}
SettingsView *-- LogClearState : manages_confirmation_state
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 found 1 issue, and left some high level feedback:
- The click handlers for the log delete buttons inline multiple state mutations (
show_log_clear_confirmandpending_log_clear_type); consider moving this into a small method (e.g.openLogClearConfirm('service' | 'mavlink')) to keep the template simpler and reduce duplication. - The confirmation dialog state is reset inconsistently: for service logs it is only cleared when
current_deletion_status === 'complete', whereas MAVLink clears it unconditionally after the operation; aligning these to always clearshow_log_clear_confirmandpending_log_clear_typeregardless of success/failure would avoid the dialog getting stuck in an inconsistent state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The click handlers for the log delete buttons inline multiple state mutations (`show_log_clear_confirm` and `pending_log_clear_type`); consider moving this into a small method (e.g. `openLogClearConfirm('service' | 'mavlink')`) to keep the template simpler and reduce duplication.
- The confirmation dialog state is reset inconsistently: for service logs it is only cleared when `current_deletion_status === 'complete'`, whereas MAVLink clears it unconditionally after the operation; aligning these to always clear `show_log_clear_confirm` and `pending_log_clear_type` regardless of success/failure would avoid the dialog getting stuck in an inconsistent state.
## Individual Comments
### Comment 1
<location path="core/frontend/src/views/SettingsView.vue" line_range="715-717" />
<code_context>
this.current_deletion_path = ''
this.current_deletion_size = 0
this.current_deletion_status = ''
+ this.show_log_clear_confirm = false
+ this.pending_log_clear_type = null
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Resetting the log clear dialog only on successful service deletion is inconsistent with MAVLink and may leave the dialog open after failures.
For service logs, `show_log_clear_confirm` and `pending_log_clear_type` are only reset when `current_deletion_status === 'finished'`, unlike the MAVLink flow where they’re reset unconditionally. If deletion fails or is interrupted, the dialog and pending type can remain set, confusing users. Please move these resets to a path that always runs (e.g., with `this.operation_in_progress = false` or in a `finally`), matching the MAVLink behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b37efe6 to
a2085bf
Compare
patrickelectric
approved these changes
Mar 24, 2026
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.
fix: #1486
Summary by Sourcery
Add a confirmation step before clearing system and MAVLink logs from the settings view to prevent accidental deletion.
New Features:
Enhancements: