Scope Asset Picker uploads to the field that opened the picker - #37372
Scope Asset Picker uploads to the field that opened the picker#37372nicobytes wants to merge 2 commits into
Conversation
- The OS file dialog, drag-and-drop, and the Asset/File prompt all ignored the mimetype restriction already used to filter browsing, so an Image field or a Story Block video/audio node could upload any file type, producing assets that then vanished from the filtered list. - Add a pure `upload-restriction.ts` module (matcher, `accept` builder, label resolver) driven by the picker's existing `config.mimeTypes`, wire it into the hidden file input, a pre-upload guard on every upload route, and scoped copy in the Asset/File prompt, while leaving Content Drive and the unrestricted File/browse modes unchanged.
|
Claude finished @nicobytes's task in 1m 35s —— View job Code Review — PR #37372
I reviewed all 14 changed files, focusing on the enforcement logic ( New IssuesNo issues found. The design holds up well against the failure modes I checked:
Notes (non-blocking)
Nice work — the pure module + single convergence guard is the right shape, and the test coverage across all upload routes is thorough. |
There was a problem hiding this comment.
🟢 Approval recommended
The upload restriction is implemented centrally and enforced at all upload entry points with strong unit/component coverage while preserving unrestricted Content Drive behavior.
Pull request overview
This PR fixes an Asset Picker UX/behavior gap in core-web where browsing was scoped by the opening field’s mimeTypes, but uploads were not—allowing “wrong-type” uploads to succeed and then disappear from the filtered list. The change centralizes upload restriction logic and applies it consistently across OS file dialog selection, drag-and-drop, and pinned-folder upload flows, while explicitly preserving Content Drive’s unrestricted behavior.
Changes:
- Added a pure upload restriction module (
isUploadAllowed,buildUploadAccept,resolveUploadRestrictionLabel) and wired it into the Asset Picker as both an early guard (drop) and a mandatory convergence-point guard before upload. - Scoped the OS file dialog via the hidden file input’s
[attr.accept](removed entirely when unrestricted) and updated the upload selector copy to optionally reflect a restriction label. - Added i18n keys plus unit/component regression coverage for restricted picker modes and for unrestricted Content Drive behavior.
File summaries
| File | Description |
|---|---|
| specs/37365-asset-picker-upload-scope/spec.md | Spec-kit spec documenting the bug, scope, ACs, and verification strategy. |
| specs/37365-asset-picker-upload-scope/data-model.md | Defines the in-memory restriction shape, validation rules, and guard placement. |
| specs/37365-asset-picker-upload-scope/contracts/upload-restriction.contract.md | Pins component inputs/DOM/message-key contracts to prevent regressions and over-reach. |
| dotCMS/src/main/webapp/WEB-INF/messages/Language.properties | Adds new message keys for scoped upload selector copy + upload rejection toast + family labels. |
| core-web/libs/ui/src/lib/components/dot-upload-type-selector/dot-upload-type-selector.component.ts | Adds optional restrictionLabel input to support scoped copy without changing option availability. |
| core-web/libs/ui/src/lib/components/dot-upload-type-selector/dot-upload-type-selector.component.html | Switches descriptions to scoped keys when restrictionLabel is present. |
| core-web/libs/ui/src/lib/components/dot-upload-type-selector/constants.ts | Adds per-option scopedDescriptionKey metadata. |
| core-web/libs/ui/src/lib/components/dot-upload-type-selector/dot-upload-type-selector.component.spec.ts | Verifies scoped vs unscoped copy and ensures both options always render. |
| core-web/libs/ui/src/lib/components/dot-asset-picker/upload-restriction.ts | New pure module implementing matching, accept-building, and user-label resolution for restrictions. |
| core-web/libs/ui/src/lib/components/dot-asset-picker/upload-restriction.spec.ts | Unit tests for restriction matching/accept/label behavior (including unclassifiable file types). |
| core-web/libs/ui/src/lib/components/dot-asset-picker/dot-asset-picker.component.ts | Applies restriction to uploads via early + mandatory guards and computes accept/label from config.mimeTypes. |
| core-web/libs/ui/src/lib/components/dot-asset-picker/dot-asset-picker.component.html | Adds [attr.accept] binding and passes restriction label into the upload type selector. |
| core-web/libs/ui/src/lib/components/dot-asset-picker/dot-asset-picker.component.spec.ts | Adds specs covering refusal across all upload routes + accept attribute presence/absence + selector label wiring. |
| core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts | Regression test ensuring Content Drive remains unrestricted (empty restrictionLabel). |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…d-flow-to-the-field-that-opened-the-picker
Problem
The Asset Picker already restricts what you can browse to the field that opened it (e.g. an Image field only lists images), but it never applied that same restriction to uploads. From inside an Image field or a Story Block video/audio node, you could upload any file type — via the OS file dialog, drag-and-drop, or the Asset/File prompt — and the upload would succeed but then silently disappear from the list because it didn't match the browse filter.
Solution
The upload flow now reads the same
mimeTypesrestriction already used for browsing and applies it consistently everywhere a file enters the picker:acceptattribute is derived from the restriction, pre-filtering the OS dialog (removed entirely when unrestricted, not set to"").upload-restriction.tsmodule (isUploadAllowed,buildUploadAccept,resolveUploadRestrictionLabel) centralizes the matching logic — no filename/extension checks, empty file types are allowed through (server stays the authority), case-insensitive family matching (image/*, etc).#resolveFilesUpload) so every route — Upload button, drag-and-drop, and folders with a pinned default upload type — is covered.dot.asset.picker.upload.*message keys).DotUploadTypeSelectorComponent) gains an optionalrestrictionLabelinput; when set, it swaps in new*.description.scopedcopy instead of the generic description, without removing either storage option.Includes unit tests for the new restriction module, the picker's upload guard across all routes, the selector's scoped/unscoped copy, and the Content Drive regression case, plus the accompanying spec-kit documents (
spec.md,data-model.md, upload-restriction contract).Test plan
upload-restriction.ts(allowed/rejected types, accept string building, label resolution)DotAssetPickerComponentspec: refused drops, refused OS-dialog picks, refused pinned-folder uploads, allowed uploads, unclassifiable file types allowed, accept attribute presence/absenceDotUploadTypeSelectorComponentspec: scoped vs default descriptions, both options always renderedDotContentDriveShellComponentregression spec: restriction label stays emptyThis PR fixes: #37365