Skip to content

feat(content-drive): prompt Asset vs File on upload (#35575)#36243

Open
zJaaal wants to merge 9 commits into
mainfrom
35575-task-content-drive-upload-button-asset-vs-file-type-selection
Open

feat(content-drive): prompt Asset vs File on upload (#35575)#36243
zJaaal wants to merge 9 commits into
mainfrom
35575-task-content-drive-upload-button-asset-vs-file-type-selection

Conversation

@zJaaal

@zJaaal zJaaal commented Jun 19, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Closes #35575.

When uploading to a folder with no preference set, Content Drive now prompts the user to choose between Asset (dotAsset) and File (FileAsset) before the upload runs, and sends the selected base type to the upload endpoint instead of the hardcoded dotAsset. This applies to both upload entry points: the Upload button and OS drag-and-drop.

Behavior

  • Upload button: choose a type → the OS file picker opens → the file uploads as the chosen type.
  • Drag-and-drop: drop files → choose a type → the dropped files upload as the chosen type.
  • The selector defaults to Assets (marked Recommended); Files maps to FileAsset.
  • Backend resolves the type from the contentlet contentType field; FileAsset is the File Asset content-type variable (matches DEFAULT_FILE_ASSET_TYPES).
Screen.Recording.2026-06-18.at.2.34.07.PM-1.mov

Changes

  • New dot-content-drive-dialog-upload-selector dialog, wired through the shell's UPLOAD_SELECTOR dialog. It emits the full selection (target folder + content type + files) so a future per-folder "remember" preference ([TASK] Content Drive: Persist default upload base type on folder (remember preference + edit folder fields) #35578) can reuse it without rework.
  • DotUploadFileService.uploadDotAsset takes an optional contentType (defaults to dotAsset; backward compatible with existing callers).
  • Renamed onAddNewDotAsset / addNewDotAssetonUpload / upload (no more hardcoded dotAsset in handler/event names).
  • Fixed two pre-existing layout bugs noticed during review: dropzone overlay icon centering, and the locale chip wrapping at narrow widths.

Out of scope (tracked in epic #35436)

Folder-level preferences/inheritance (#35577), the "Remember for this folder" checkbox (#35578), dynamic button label, MIME-type routing, and multi-file upload.

Testing

  • New spec for the upload selector dialog (option rendering, Recommended chip, default selection, emitted selection for Asset/File, Cancel).
  • Shell spec reworked to the dialog flow (driven via UI events): opening the selector from button/drag-drop/sidebar, drag-drop upload as dotAsset/FileAsset, root upload, message handling, multi-file warning, and the button flow (pick → picker → upload).
  • Upload service spec covers the default and explicit contentType.
  • portlets-content-drive: 666 passing; data-access: 690 passing. Lint clean.

Manual UI verification of the live upload + i18n copy is pending a backend message-cache reload.

🤖 Generated with Claude Code

Uploading to a folder with no preference now prompts the user to choose
between Asset (dotAsset) and File (FileAsset) before the upload runs,
for both the Upload button and OS drag-and-drop. The selected base type
is sent to the upload endpoint instead of the hardcoded dotAsset.

- Parameterize DotUploadFileService.uploadDotAsset with a contentType
  (defaults to dotAsset; backward compatible)
- Add the dot-content-drive-dialog-upload-selector dialog (Assets
  recommended + selected by default, Files), wired through the shell's
  UPLOAD_SELECTOR dialog and emitting the full selection (folder + type
  + files) so a future per-folder "remember" can reuse it
- Rename onAddNewDotAsset/addNewDotAsset to onUpload/upload
- Fix pre-existing dropzone overlay icon centering and the locale chip
  wrapping at narrow widths
- Tests for the dialog, both upload flows, and the service param

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Claude finished @zJaaal's task in 1m 10s —— View job


Rollback Safety Analysis

  • Read rollback-unsafe categories reference
  • Get full PR diff
  • Analyze diff against all unsafe categories
  • Applied label: AI: Safe To Rollback

Result: ✅ Safe to Roll Back

All 16 changed files are purely frontend (Angular TypeScript + HTML + i18n) with no backend, database, Elasticsearch, or API contract changes. Checked against every category in the reference:

Category Assessment
C-1 Structural Data Model ✅ No DB schema changes
C-2 ES Mapping Change ✅ No ES mapping changes
C-3 Content JSON Model Version ✅ No ImmutableContentlet or CURRENT_MODEL_VERSION changes
C-4 DROP TABLE/COLUMN ✅ No SQL DDL
H-1 One-Way Data Migration ✅ No data migrations
H-2 RENAME TABLE/COLUMN ✅ No renames
H-3 PK Restructuring ✅ No PK changes
H-4 New ContentType Field Type ✅ No new field type registrations
H-5 Storage Provider Change ✅ No storage provider changes
H-6 DROP PROCEDURE/FUNCTION ✅ No procedure drops
H-7 NOT NULL Column Without Default ✅ No column additions
H-8 VTL Viewtool Contract Change ✅ No viewtool changes
M-1 Column Type Change ✅ No SQL
M-2 Push Publishing Bundle Format ✅ No bundler changes
M-3 REST/GraphQL API Contract uploadDotAsset signature change is internal (Angular service), not a REST endpoint — DotUploadFileService is a client-side Angular service, not a backend REST API
M-4 OSGi Plugin API ✅ No Java changes

Summary: The PR adds a new Angular dialog component (dot-content-drive-dialog-upload-selector) and routes upload calls through a type-selection prompt. The only backend-touching change is 7 new i18n keys in Language.properties (purely additive). Rolling back to N-1 simply removes the dialog and reverts to hardcoded dotAsset — no data, schema, or index state is affected.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — deepseek.v3.2

[🟡 Medium] core-web/libs/data-access/src/lib/dot-upload-file/dot-upload-file.service.ts:124 — Missing baseType property in extraData for FileAsset upload. The uploadDotAsset method removes baseType from the extraData object when calling the service, but the backend may require it for FileAsset content type creation. This could cause upload failures for non-dotAsset types.

[🟡 Medium] core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts:531 — Potential null/undefined access in error detail. The uploadDotAsset error handler uses uploadedContentType variable which may be undefined if the response doesn't contain it, causing the error message to show "undefined".

[🟡 Medium] core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts:548 — Missing null check for hostFolder?.id. When hostFolder is undefined, hostFolder?.id ?? '' returns empty string, but this might not be the intended behavior for root folder uploads. Should verify this aligns with backend expectations.

[🟡 Medium] core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-upload-selector/dot-content-drive-dialog-upload-selector.component.ts:62 — Unnecessary null check. The $canContinue computed property checks !!this.$selectedType() but $selectedType always has a default value from UPLOAD_SELECTOR_OPTIONS[0].contentType, so it will never be falsy. This creates dead code.

[🟠 High] core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts:409-410 — Missing cleanup of file input value in error path. When onFileChange is called but selection is undefined or files length is 0, the code resets $activeSelection and input.value, but if there's an error in the upload flow, the file input might retain its value, causing issues with subsequent uploads.

[🟡 Medium] core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts:647-649 — Test coverage gap for error case where uploadedContentType is undefined. The test mocks a successful upload response but doesn't test the error handling path where the response might not contain the contentType property.


Run: #27977708548 · tokens: in: 17851 · out: 595 · total: 18446

- Type the upload selection's contentType as a union derived from
  UPLOAD_SELECTOR_OPTIONS so it can't drift or accept arbitrary values
- Rename the shadowed contentType in the upload success handler

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zJaaal

zJaaal commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

Thanks for the review — went through each item.

Addressed (e097bfa):

  • 🟡 models.tsDotContentDriveUploadSelection.contentType is now a union (DotContentDriveUploadContentType) derived from UPLOAD_SELECTOR_OPTIONS, so it can't accept arbitrary strings and stays in sync with the rendered choices. This also covers the "validate the contentType param" point — the only producer is the typed selector.
  • 🟡 shell.component.ts — renamed the shadowed contentType in the upload success handler (contentType: uploadedContentType) for clarity.

Intentional / no change (with rationale):

  • 🟡 hostFolder ?? '' — empty string is the established "site root" convention here (same as DotUploadFileService.publishContent); the backend resolves it to the current host.
  • 🟡 $canContinue — kept as a defensive guard so onContinue and the disabled state stay correct if the default selection ever changes; harmless given a radio can't be deselected today.
  • 🟡 onFileChange reset ordering — the reset and input.value = '' are adjacent synchronous statements (no await/async between them), so a finally block isn't needed; resetting before the guard is deliberate so a re-opened picker can't reuse a stale selection.
  • 🟡 duplicated !files?.length checks — each guard serves a distinct purpose: resolveFilesUpload needs it before reading files.length, and uploadFile needs it to narrow files before files[0]. They're type-narrowing guards, not redundant validation.
  • 🟡 hardcoded dotAsset/FileAsset in UPLOAD_SELECTOR_OPTIONS — these are dotCMS's canonical base content-type variables (matches DEFAULT_FILE_ASSET_TYPES); they're the single source the union type now derives from.
  • 🟠 createFileList test helper — test-only; it replicates the length/indexed-access/item() surface the implementation actually uses. Fine for these specs.
  • 🟡 jest not imported in the dialog spec — jest/afterEach are ambient globals here, consistent with the sibling dot-content-drive-dialog-content-type-selector.component.spec.ts. Tests pass.

— Reply drafted by Claude (Claude Code) on behalf of @zJaaal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Content Drive: Upload button — Asset vs File type selection

1 participant