✨ Drag a folder tree into the asset library - #28
Merged
Conversation
Laravel skips every non-implicit rule when a string trims to nothing, so `string|min:1|max:4000` lets a 250 KB run of spaces through untouched. Adding `present` does not help, and a closure rule is not implicit either. BoundedString is marked implicit, so it runs on blank and absent values and does the length check itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ensure-paths takes the folder paths of a dropped tree and returns a path to id map, so the browser resolves every folder in a single round trip instead of one create per folder. Existing folders are merged case-insensitively and in NFC, which makes a re-drop idempotent rather than piling up duplicates. Soft-deleted folders are ignored: a drop must not undelete. Segment names are truncated to the column length and fall back to a placeholder when purification empties them, and the response reports what it renamed. The work is bounded by total segments rather than path count, since one path could otherwise carry 2000 of them. A per-space lock serializes callers, measured to leave room under max_execution_time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dataTransfer.files flattens a folder drop and loses the structure. This walks webkitGetAsEntry instead, snapshotting the entries synchronously because the DataTransfer dies when the handler returns. readEntries answers with at most 100 entries per call, so it is called until it comes back empty. Stopping after one call silently truncates any folder with more than 100 children. One unreadable file no longer costs the whole drop: failures are counted and reported instead of rejecting the traversal. Paths leave in NFC so a Finder "Café" merges with one created in the UI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A 400 file drop held the upload dialog open for twenty minutes and locked the asset area behind it. The batch now lives at module scope, so it survives the dialog closing and in-app navigation, and a docked panel carries the progress. Three lanes, since each video upload runs ffmpeg inside the request. Each enqueue keeps its own uploader and settle callback, so files always upload to the space they were staged for even when a second drop joins a running batch. Lanes carry a generation token so one still unwinding from a reset batch cannot settle the next one. Cancel stops the queue and keeps everything already uploaded. Failures carry into the next batch rather than vanishing, and the panel is the only thing that clears them, so a retry button is never left backed by nothing. Logging out resets the batch: the next account must not see the previous one's filenames, let alone retry them. Batch uploads take a silent uploadAsset, which skips the per-file list invalidation and the toast and throws so the batch can record the server message. The single-file path is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
buildUploadTree turns the flat file and directory lists into nested nodes, filling in any intermediate folder that neither list mentions and counting files recursively. Empty folders become nodes of their own, because the drop promised to mirror what was there. UploadTreeItem renders a folder and recurses; UploadFileRow carries the status, progress and per-file actions for both the tree and the flat list. Indentation and the expand affordance follow AssetFolderTree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dropping folders from Finder or Explorer now mirrors the structure into asset folders and uploads the files into it, instead of flattening everything into the folder that happens to be open. A drop carrying folders renders as a tree so the structure is visible before anything uploads. Thumbnails stay below fifty files: four hundred decoded previews is a memory problem, not a nicety. The pre-flight states what was found before committing to it, including skipped system files, entries the browser could not read, and folder names the server will rename. Files past the size limit are marked before the batch starts rather than travelling and failing. Missing required fields are stated once with a count, not per file. Folders are resolved in one ensure-paths call whose answers are reused, so a second Upload click cannot re-post them. A drop containing folders is refused outright without asset_folders.manage: flattening four hundred files into one pile is worse than saying no. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces the old "flatten everything into one pile" folder-drop behavior in the asset library with a feature that mirrors a dragged folder tree into asset folders and uploads files into their correct folders. It reads the drop through webkitGetAsEntry (snapshotted synchronously), materializes the folder tree via a new ensure-paths endpoint in a single locked transaction, and runs the uploads through a new app-scoped, three-lane background batch with a docked progress panel that survives dialog close and navigation.
Changes:
- Backend: new
EnsureAssetFolderPathsaction (NFC + case-folded merge, soft-delete-aware, per-space cache lock), its controller/route/FormRequest, and a reusableBoundedStringvalidation rule; gated onasset_folders.manage. - Frontend drop/tree reading: pure
dropped-tree.ts(drop + directory-picker traversal, junk filtering, NFC, code-point truncation) andupload-tree.ts(nested tree builder), both unit-tested. - Frontend upload flow: module-scoped
useAssetUploadBatchcomposable, a rewrittenUploadDialog, newUploadBatchPanel/UploadTreeItem/UploadFileRowcomponents,useAssetssilentupload mode, i18n (en/de), and app-level wiring that resets the batch on user change.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
app/Actions/Asset/EnsureAssetFolderPaths.php |
Core action: resolves paths to folder ids, merges siblings by NFC+case, locks per space |
app/Http/Requests/Asset/EnsureAssetFolderPathsRequest.php |
Validates payload; caps paths and total segments |
app/Rules/BoundedString.php |
Implicit length-bound rule that still runs on blank strings |
app/Http/Controllers/Mgmt/EnsureAssetFolderPathsController.php |
Authorizes asset_folders.manage and returns paths/folders/renamed |
routes/private_mgmt.php |
Registers the ensure-paths route |
resources/js/lib/dropped-tree.ts |
Reads drop/FileList into a flat tree; junk/NFC/truncation handling |
resources/js/lib/upload-tree.ts |
Builds the nested folder/file tree the dialog renders |
resources/js/composables/useAssetUploadBatch.ts |
App-scoped batch: lanes, groups, generation guard, retry/cancel |
resources/js/composables/useAssets.ts |
Adds silent/signal upload options for batch use |
resources/js/components/assets/UploadDialog.vue |
Rewired to tree ingestion, ensure-paths, batch enqueue |
resources/js/components/assets/UploadBatchPanel.vue |
Docked progress/failure panel + duplicate prompt host |
resources/js/components/assets/UploadTreeItem.vue / UploadFileRow.vue |
Recursive tree row and file row rendering |
resources/js/components/assets/AssetGrid.vue / AssetListView.vue |
Snapshot drop entries; pass initial-tree/allow-folder-upload |
resources/js/app.vue |
Mounts panel; resets batch on account change |
resources/js/api/resources/asset-folders.ts |
ensurePaths API method + types |
resources/js/i18n/{en,de}.json, auto-imports.d.ts |
New strings and auto-import declarations |
tests/** (JS + PHPUnit) |
Comprehensive coverage for traversal, batch, and the endpoint |
I reviewed the implementation against the codebase: space isolation is at the per-space DB connection level (no missing space_id filter), the segment/path caps and lock TTL are internally consistent, the AssetMetadataFieldResolver used during serialization memoizes a single folder map (no N+1), and the extensive JS and PHPUnit tests match the implementation. I did not find a confident, objective defect, and the documented behaviors (duplicate handling defaulting to copies, plaintext delivery tokens, soft-deleted folders ignored) are deliberate per the repo's agent conventions. That said, this is a broad, high-risk change: it introduces module-scoped concurrency state, rewrites the upload/dialog flow, and the author explicitly notes the tree view has only been statically verified (never rendered in a browser) and has unresolved role="tree"/clickable-row accessibility conflicts — all of which warrant human verification before approval.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Dragging a folder from Finder or Explorer into the assets area used to flatten it.
dataTransfer.filesdrops the structure on the floor, so every file landed in whichever folder happened to be open and the nesting had to be rebuilt by hand.Now the folder tree is mirrored into asset folders and the files are uploaded into it.
How it works
The drop is read through
webkitGetAsEntryinstead, snapshotting entries synchronously because the DataTransfer dies when the handler returns.readEntriesanswers with at most 100 entries per call, so it is drained in a loop; stopping after one call silently truncates any folder with more than 100 children.One
ensure-pathscall materializes every folder in a single transaction and hands back a path to id map. Existing folders merge case-insensitively and in NFC, so re-dropping the same tree is idempotent instead of piling up duplicates.Uploads then run in a three-lane background batch that survives the dialog closing and in-app navigation, with a docked progress panel. Three lanes because each video upload runs ffmpeg inside the request.
Worth knowing while reviewing
A drop carrying folders renders as a tree before anything uploads. Thumbnails stay below fifty files, since four hundred decoded previews is a real memory problem.
Some choices were made deliberately and are not oversights: cancel keeps everything already uploaded rather than rolling back, duplicates are detected by the existing 409 and prompt once per batch defaulting to copies, soft-deleted folders are ignored rather than restored, and case-variant sibling folders collapse into one.
A drop containing folders is refused outright without
asset_folders.manage. Flattening four hundred files into one pile is worse than saying no.Verified
Typecheck, lint and the full Vitest suite are green, and the endpoint has PHPUnit coverage for merging, case and NFC folding, soft-deleted folders, permissions, truncation and idempotency.
Exercised against a real 39-file drop across seven nested folders: the tree was read in full, folders were created with correct parents, files were routed correctly, and failures were reported per file with working retry.
Known gaps
The tree view has not been seen in a browser, only static verification. The tree rows carry
aria-expandedbut not fullrole="tree"semantics, which conflict with the clickable file rows; worth a follow-up. And the segment cap returns a 422 rather than being caught in the pre-flight.🤖 Generated with Claude Code