Skip to content

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

Open
nicobytes wants to merge 2 commits intomainfrom
34588-defect-host-or-folder-field-not-picking-up-current-location-in-site-browser
Open

feat(dot-custom-event-handler): enhance contentlet creation with folder#35322
nicobytes wants to merge 2 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

…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 1m 50s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze implementation
  • Post review feedback

The overall approach is sound, but there are a few real issues worth addressing.


1. File assets get no folderPath — incomplete fix

getSelectedfileAsset calls createContentlet without a third argument, so the Host/Folder field is never pre-filled when creating file assets from a folder context. The issue title says "Host or Folder field not picking up current location" — it applies to both page and file assets.

showFileAssetPopUp receives a folderMap parameter but never stores it anywhere (contrast with showPageAssetPopUp which sets currentPageAssetFolderMap). The same logic for extracting hostName + folderPath needs to be applied here.

Fix this →


2. folderPath format is inconsistent at root

In getSelectedpageAsset (view_browser_js_inc.jsp:1636–1640):

var hostName = currentPageAssetFolderMap.fullPath.split(':')[0];
var cmsFolderPath = currentPageAssetFolderMap.folderPath || '/';
folderPath = cmsFolderPath === '/' ? hostName : hostName + cmsFolderPath;
  • Subfolder: cmsFolderPath = "/level1/""demo.dotcms.com/level1/" (includes trailing slash)
  • Root: cmsFolderPath = "/""demo.dotcms.com" (no slash at all)

The Host/Folder widget may or may not handle "demo.dotcms.com" vs "demo.dotcms.com/" the same way. Tests only cover the subfolder case ("default/level1/level2/"). This should be tested/confirmed against the component's actual parsing behavior.


3. currentPageAssetFolderMap is never cleared

After createContentlet fires and the dialog closes, currentPageAssetFolderMap still holds the last folder map. If the user later creates a page asset from a different entry point that doesn't call showPageAssetPopUp (or the variable is read before the new popup opens), a stale folder could be used silently. Consider clearing it at the start of getSelectedpageAsset after reading it, or resetting it when the dialog is destroyed.


4. selected.item can still be null after the null check

getSelectedpageAsset now correctly returns early when !selected (line 1631–1633). However, selected.item is still accessed without a check at line 1645 (selected.item.velocityVarName). A widget object can be truthy while its item is null (nothing selected in the FilteringSelect). This is a pre-existing issue but was not fixed as part of this PR, and the early return addition draws attention to it.


5. Copilot's patchState concern is already handled

Copilot flagged that patchState might not reset queryParams when no supported params are present. Looking at the code in edit-content.store.ts:341–346:

const supportedQueryParams: EditContentQueryParams = {};
if (queryParams?.['folderPath']) {
    supportedQueryParams.folderPath = queryParams['folderPath'];
}
patchState(store, { queryParams: supportedQueryParams });

patchState is always called (even with an empty {}), so stale state is properly reset on each initializeAsPortlet call. The Copilot suggestion is a no-op.


6. Minor: no folderPath format validation

folderPath flows from the URL query param into the form field without any validation. A crafted URL could inject an arbitrary string. Since this only pre-fills a form field (not directly sent to the API), the blast radius is low, but it's worth noting if the field value is ever used in navigation or API calls downstream without further validation.

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.
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