feat(cloud): add folder download as zip archive with loading state and blob error parsing (#279) - #297
Conversation
…d blob error parsing
DenizAltunkapan
left a comment
There was a problem hiding this comment.
The frontend work itself is clean. It follows the existing downloadFile pattern, reuses downloadingPaths for the spinner state, the blob error parsing is a nice touch, and the row button handling both kinds is tidy. No conflicts, CI green.
One blocker though: the backend endpoint does not exist yet. Issue #279 explicitly depends on Vault-Web/cloud-page#105 and says the frontend must not start before it lands. That issue is still open with no PR, and there is no GET /folders/archive anywhere in cloud-page. As merged today, every folder download would 404. Please hold this until the backend endpoint is merged, then confirm the path and param name (folderPath) actually match what the backend exposes, since right now both are assumptions.
Two smaller inline notes below. Happy to approve once the backend dependency is in and verified against it.
|
|
||
| getFolderArchive(relativePath: string): Observable<Blob> { | ||
| const path = this.normalizePath(relativePath); | ||
| return this.http.get(`${this.apiUrl}/folders/archive`, { |
There was a problem hiding this comment.
This endpoint is not implemented in cloud-page yet (the dependency Vault-Web/cloud-page#105 is an open issue with no PR). The URL and the folderPath param name are guesses until the backend lands. Once it does, verify both against the actual controller and add rate limit and oversized folder handling if the backend returns specific statuses for those, since the acceptance criteria in #279 call them out.
|
|
||
| component.downloadFolder('/root/sub', 'sub'); | ||
|
|
||
| expect(cloudMock.getFolderArchive).toHaveBeenCalledWith('sub'); |
There was a problem hiding this comment.
The error specs wait on a raw setTimeout of 50ms before asserting, which is a flake risk on slow CI. parseBlobError completes its observable, so you can assert inside the subscribe callback or use fakeAsync with tick instead of a wall clock delay.
|
Update on my earlier review: the blocker I raised is resolved. The backend landed in Vault-Web/cloud-page#106 (merged 2026-07-23), which was roughly the same day I wrote that review, so my comment was already out of date when you read it. Sorry for leaving you waiting on it. The endpoint exists now, but the contract in this PR does not match it, so folder downloads would still 404. Two things in getFolderArchive:
So the call should target ${this.apiUrl}/folders/download with HttpParams().set('path', path). Worth noting the spec mocks CloudService, so the suite stays green either way and will not catch this. One thing that works in your favour: that route is registered under the DOWNLOAD rate limit category in RateLimitFilter, so it really can return a JSON error body on a blob request. The blob error parsing here is the right call, not overengineering. Everything else from my first pass still stands. The downloadFile pattern reuse, the downloadingPaths spinner state and the row button handling are all clean. Fix the URL and the param and I am happy to approve. |
Problem
Users could only download Cloud files individually, which was inefficient—especially on mobile devices when attempting to download an entire directory or photo album.
Solution
Implemented a mobile-friendly folder download feature:
getFolderArchive(relativePath)inCloudServiceto queryGET /folders/archivewithresponseType: 'blob'.downloadingPathsstate to show loading spinners on active download buttons while zip archives are prepared and streamed.parseBlobErrorhelper usingFileReaderto extract and display detailed JSON error messages (e.g. rate limits, oversized folder warnings) surfaced by the backend instead of generic failure toasts.Verification
cloud.component.spec.tstesting zip download triggers and error blob handling.