Skip to content

feat(dot-custom-event-handler): enhance contentlet creation with folder#35322

Merged
nicobytes merged 5 commits intomainfrom
34588-defect-host-or-folder-field-not-picking-up-current-location-in-site-browser
Apr 17, 2026
Merged

feat(dot-custom-event-handler): enhance contentlet creation with folder#35322
nicobytes merged 5 commits intomainfrom
34588-defect-host-or-folder-field-not-picking-up-current-location-in-site-browser

Conversation

@nicobytes
Copy link
Copy Markdown
Member

@nicobytes nicobytes commented Apr 15, 2026

This pull request introduces support for pre-filling the Host/Folder field when creating new contentlets, by passing a folderPath parameter through the UI and Angular application layers. It ensures that if a folder is selected in the legacy browser, the corresponding path is propagated and used to initialize the field value in the Angular edit content form. The changes include capturing the query param on the JavaScript side, updating Angular routing and state management, and enhancing field resolution logic and tests.

End-to-end support for pre-filling Host/Folder field:

  • The legacy browser JavaScript (view_browser_js_inc.jsp) now passes a folderPath parameter as part of the custom event when creating a contentlet, based on the selected folder. [1] [2] [3]
  • The Angular service (dot-custom-event-handler.service.ts) and its tests are updated to extract and pass folderPath as a query param when navigating to the new content route. [1] [2] [3] [4] [5]

State management and field resolution updates:

  • The edit content store (edit-content.store.ts) captures supported query params (currently folderPath) from the route and makes them available for form initialization, with corresponding tests to verify behavior. [1] [2] [3] [4]
  • The Host/Folder field resolution logic now prefers folderPath from query params when available, and the component passes these params to the resolution function. Tests were added to verify all scenarios. [1] [2] [3] [4] [5]

These changes ensure that when a user creates content from a specific folder context in the legacy browser, the new content form will be pre-filled with the correct Host/Folder value, improving usability and consistency across the application.

This PR fixes: #34588

This PR fixes: #34588

…erPath query param

- Updated `DotCustomEventHandlerService` to include `folderPath` in the navigation query parameters when creating contentlets.
- Added tests to verify that the `folderPath` is correctly passed during contentlet creation.
- Enhanced existing tests to ensure proper handling of query parameters in navigation calls.

This change improves the flexibility of contentlet creation by allowing the specification of a folder path directly through the event handler, facilitating better content organization.
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Claude finished @nicobytes's task in 3m 46s —— View job


PR Review

  • Gather context
  • Read legacy JSP changes
  • Read Angular service changes
  • Read store changes
  • Read field resolution changes
  • Read component changes
  • Post review findings

Issues

1. getSelectedfileAsset doesn't pass folderPath — missing scope

view_browser_js_inc.jsp:1659

createContentlet(loc, selected.item.velocityVarName);  // no folderPath

Only getSelectedpageAsset sends the folder context. Users creating file assets from a selected folder won't get the Host/Folder field pre-filled. If this is intentional (FILEASSET goes through the legacy path), it needs a comment. If it's an oversight, getSelectedfileAsset needs the same folderPath logic as getSelectedpageAsset. Fix this →


2. folderPath silently dropped for legacy-editor content types

dot-custom-event-handler.service.ts:119-121

if (this.shouldRedirectToOldContentEditor(contentType)) {
    return this.createContentletLegacy($event);  // folderPath is discarded here
}

When a content type's metadata flags it for the old editor, folderPath is silently ignored. The legacy path uses folderInode from the URL, not folderPath. This is probably fine, but it means partial scenarios exist — if someone switches a content type back to the new editor later, it'll suddenly work. Either document the limitation or pass folderPath through some mechanism the legacy editor understands.


3. Weak test assertion for the "not create" case

dot-custom-event-handler.service.spec.ts:570

expect(router.navigate).not.toHaveBeenCalled();

This passes even if router.navigate is never called for any reason (service error, wrong handler, etc.). It doesn't verify the expected fallback path (dotContentletEditorService.create) was actually taken. Fix this →


4. folderPath exposed as a plain URL query param

After router.navigate([...], { queryParams: { folderPath: '...' } }), the value is visible in the URL and can be crafted by anyone. The hostFolderResolutionFn takes it at face value. There's no validation that the path refers to a folder the user actually has permission to access — that validation is deferred entirely to the Host/Folder field's store load. This is low risk (the backend enforces permissions when saving), but a malformed path will silently produce an empty/mis-resolved field with no user feedback. Worth a console.warn or fallback to field.defaultValue if the path lookup fails.


Looks good:

  • Stale queryParams state issue is correctly handled: patchState(store, { queryParams: supportedQueryParams }) is always called (even with empty object), so a previous folderPath doesn't persist across navigations.
  • hostFolderResolutionFn only uses folderPath for new content (!contentlet?.hostName || !contentlet?.url), so editing existing content is unaffected.
  • getSelectedpageAsset now has an early return after the error message (the Copilot concern was already addressed in this code).
  • Store tests cover: folderPath present, folderPath absent, ordering of patchState vs initializeNewContent.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end support for pre-filling the Host/Folder field when creating new content from the legacy browser by propagating a folderPath value through the legacy JS → custom event → Angular navigation/query params → edit-content store → field resolution.

Changes:

  • Emit folderPath in the legacy browser’s create-contentlet custom event (when available).
  • Pass folderPath as an Angular route query param and capture it in DotEditContentStore.
  • Prefer folderPath (from query params) when resolving the Host/Folder field initial value, with new/updated unit tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
dotCMS/src/main/webapp/html/portlet/ext/browser/view_browser_js_inc.jsp Adds folderPath to the custom event payload when creating content from a selected folder context.
core-web/apps/dotcms-ui/src/app/api/services/dot-custom-event-handler/dot-custom-event-handler.service.ts Reads folderPath from the event and navigates to the new-content route with queryParams.
core-web/apps/dotcms-ui/src/app/api/services/dot-custom-event-handler/dot-custom-event-handler.service.spec.ts Updates/extends tests to cover navigation with folderPath.
core-web/libs/edit-content/src/lib/store/edit-content.store.ts Captures supported query params (currently folderPath) into store state during route initialization.
core-web/libs/edit-content/src/lib/store/edit-content.store.spec.ts Adds tests validating query param capture behavior.
core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts Passes store-captured query params into field resolution.
core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form-resolutions.ts Extends resolution signatures and prefers folderPath for Host/Folder when initializing new content.
core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form-resolutions.spec.ts Adds test coverage for Host/Folder resolution behavior with query params.

Comment thread core-web/libs/edit-content/src/lib/store/edit-content.store.ts Outdated
- Updated the test for `DotCustomEventHandlerService` to assert that the router's `navigate` method is not called, streamlining the test logic.
- Refactored the `edit-content` store to always patch the state with query parameters, removing unnecessary checks for empty parameters.
- Added a return statement in the JSP file to prevent further execution when no valid HTML page asset type is selected.

These changes enhance test clarity and ensure consistent state management in the application.
- Introduced a new fixture for the host-folder field to facilitate testing.
- Implemented multiple tests to validate the default selection, nested folder context pre-fill, and persistence of selections in the new content form.
- Added helper functions for interacting with the host-folder field in tests.
- Updated the content type and folder request files to support new functionalities.
- Enhanced the newEditContentForm.page.ts to navigate with folderPath query parameters.
@nicobytes nicobytes added this pull request to the merge queue Apr 17, 2026
Merged via the queue into main with commit 4494b5f Apr 17, 2026
50 checks passed
@nicobytes nicobytes deleted the 34588-defect-host-or-folder-field-not-picking-up-current-location-in-site-browser branch April 17, 2026 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[DEFECT] Host or Folder field not picking up current location in Site Browser

3 participants