-
Notifications
You must be signed in to change notification settings - Fork 114
frontend: refactor log download in the settings menu #3644
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
base: master
Are you sure you want to change the base?
frontend: refactor log download in the settings menu #3644
Conversation
Reviewer's GuideRefactored the log download flow by replacing direct filebrowser calls with a new FolderManager that streams downloads via axios, added a reusable ProgressInformation component for both download and deletion progress, and updated the SettingsMenu to use these abstractions and disable controls during operations. Sequence diagram for log download flow with FolderManagersequenceDiagram
actor User
participant SettingsMenu
participant FolderManager
participant filebrowser
participant back_axios
participant ProgressInformation
User->>SettingsMenu: Clicks 'Download log files'
SettingsMenu->>FolderManager: downloadFolder('system_logs')
FolderManager->>filebrowser: fetchFolder('system_logs')
filebrowser-->>FolderManager: returns folder
FolderManager->>filebrowser: downloadFolder(folder, progressHandler)
filebrowser->>back_axios: GET /raw/{folder.path}/?algo=zip
back_axios-->>filebrowser: Streams download progress
filebrowser-->>FolderManager: Calls progressHandler(event)
FolderManager->>ProgressInformation: Updates progress props
FolderManager-->>SettingsMenu: Operation complete
SettingsMenu->>ProgressInformation: Show completion
Class diagram for new FolderManager and ProgressInformation componentsclassDiagram
class FolderManager {
+downloadedBytes: number
+totalBytes: number
+startTime: number
+downloadSpeed: number
+inProgress: boolean
+downloadFolder(logs: string): Promise<void>
+resetDownloadVariables(): void
}
class ProgressInformation {
+type: string
+operation: string
+currentSize: number
+totalSize: number
+downloadSpeed: number
+currentPath: string
+status: string
+formatSize(kb_bytes: number): string
}
SettingsMenu --> FolderManager : uses
SettingsMenu --> ProgressInformation : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Remove the debug console.log statements from FolderManager once the progress logic is verified so the console isn’t cluttered in production.
- Instead of showing the progress bar as always indeterminate, consider switching to a determinate mode that uses currentSize/totalSize to display actual download percentage.
- Unify the async style in filebrowser.downloadFolder by using try/catch around await rather than mixing await with .then/.catch for clearer error handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Remove the debug console.log statements from FolderManager once the progress logic is verified so the console isn’t cluttered in production.
- Instead of showing the progress bar as always indeterminate, consider switching to a determinate mode that uses currentSize/totalSize to display actual download percentage.
- Unify the async style in filebrowser.downloadFolder by using try/catch around await rather than mixing await with .then/.catch for clearer error handling.
## Individual Comments
### Comment 1
<location> `core/frontend/src/libs/filebrowser.ts:179-183` </location>
<code_context>
+ onDownloadProgress: progressHandler,
+ })
+ .then((response) => {
+ const blob = new Blob([response.data], { type: 'application/zip' })
+ const blobURL = window.URL.createObjectURL(blob)
+ const a = document.createElement('a')
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Missing responseType configuration for binary data.
Set 'responseType: "blob"' in the axios request to prevent data corruption when handling binary downloads.
```suggestion
await back_axios({
method: 'get',
url: url,
responseType: 'blob',
onDownloadProgress: progressHandler,
})
```
</issue_to_address>
### Comment 2
<location> `core/frontend/src/components/common/ProgressInformation.vue:18` </location>
<code_context>
+ Total: {{ formatSize(totalSize / 1024) }}
+ </div>
+ <div class="text-caption">
+ Speed: {{ formatSize(downloadSpeed / 1024) }}/ms
+ </div>
+ </div>
</code_context>
<issue_to_address>
**nitpick:** Unit for download speed is likely incorrect.
Use 'KB/s' or 'MB/s' instead of 'KB/ms' to improve clarity and align with standard conventions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
I don't think that's a problem. I think the original idea for that size was the disk usage size, not the compressed download size |
joaoantoniocardoso
left a comment
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.
The browser doesn't show me the zip file after I wait for the download preparation:
cant_download_logs.mp4
cf35665 to
beb8990
Compare
beb8990 to
5e9984d
Compare


fix: #3642, fix: #3284
Had to change the folder download logic to know when the download operation has finished to disable/enable the buttons.
For some reason the size of the loaded file presented to the user in the UI is sometimes smaller than the resulting size of the compressed file. From what I understood, this is related to axios compressing the files on the go; thus it can not inform the total size during the operation
Summary by Sourcery
Refactor the settings menu’s log download flow to use a new FolderManager and axios-based downloads with progress tracking, add a reusable ProgressInformation component for UI feedback, and prevent concurrent operations by disabling buttons while activities are in progress.
New Features:
Enhancements: