modernization(query-tool) #35709: migrate Query Tool portlet from Dojo JSP to Angular

[hmoreras]

Summary

Migrates the Query Tool portlet from the legacy Dojo/Dijit JSP to a new Angular + PrimeNG implementation, with a query-tool-legacy rollback entry preserved in portlet.xml.

Resolves #35709 (sub-task of epic #34735).

Screenshare.-.2026-05-22.8_33_31.AM.mp4

What changed

New Angular portlet — core-web/libs/portlets/dot-query-tool/

• Shell + page component + DotQueryToolStore (NgRx SignalStore) + data-access service
• PrimeNG layout: two-pane splitter, Monaco editor (plain-text Lucene), collapsible Parameters panel, stats bar + Results / Raw tabs
• Help popover with four canonical Lucene examples
• URL query-param state sync (q, offset, limit, sort, userId) replacing the legacy rememberQuery session flag — implemented as a reactive effect() using Router.createUrlTree + Location.go (canonical dot-content-drive-shell pattern) so address-bar updates do not re-mount the component
• MAX_RESULTS = 1000 cap mirroring ES Search's MAX_HITS, with a <p-message> warn when the user-supplied limit is capped
• Result row click opens the target contentlet in a new tab:
  • HTML pages → /dotAdmin/#/edit-page/content?url=…&language_id=…
  • New editor (CONTENT_EDITOR2_ENABLED=true) → /dotAdmin/#/content/{inode}
  • Legacy editor → /dotAdmin/#/c/content/{inode}
  • about:blank is opened synchronously inside the click handler to avoid popup-blocker rejection, then location.href is assigned once the async getContentType() lookup resolves
• Consumes POST /api/v1/content/_search (v1 wrapper, preferred per ContentResource JavaDoc)

Cross-portlet shared helpers (consumed by both Query Tool and ES Search)

• libs/utils: buildCurlSnippet, buildFetchSnippet (POSIX-shell-safe) + unit tests
• libs/ui: DOT_MONACO_BASE_OPTIONS, DOT_MONACO_RAW_OPTIONS
• Store: apiRequestBody computed — single source of truth for the search payload so runSearch and the curl/fetch snippets cannot drift
• DotClipboardUtil replaces hand-rolled navigator.clipboard.writeText; getDownloadLink replaces hand-rolled <a> download anchors

ES Search alignment

• Stats bar format harmonized: Showing X of Y results · N ms
• Share / Export button parity
• Tab count-badges removed from both portlets; Raw editor gains bottom padding

Backend wiring

• portlet.xml: query-tool now points at PortletController (Angular shell); query-tool-legacy entry preserved as live-toggle rollback to the existing JSP without redeploy
• PortletID: QUERY_TOOL_LEGACY added (mirrors TAGS_LEGACY precedent)
• Language.properties: com.dotcms.repackage.javax.portlet.title.query-tool-legacy = Query Tool Legacy so the legacy twin shows up under Roles → Tools (parity with es-search-legacy / tags-legacy)
• queryTool.action.run: Submit → Run (button label + empty-state hint copy); help tooltip / aria-label: Click for query examples

Tests

• Jest + Spectator coverage for shell, page, store, and service (27 passing) — limit-cap behavior, Location.go URL-sync assertions, menu-driven export tests via exportItems[i].command() + clipboard spy, error paths through httpErrorManager.handle
• New unit tests for buildCurlSnippet / buildFetchSnippet in libs/utils

Definition of Done (from epic #34735)

• Admin runs a valid Lucene query — matching contentlets render with contentTook / queryTook timings
• Admin runs an invalid Lucene query — inline error surfaced via httpErrorManager; results table unchanged
• Non-admin role blocked (server-side gate; userId forwarded only when set)
• Unlicensed instance blocked (existing license gate preserved)
• Rollback to legacy JSP without redeploy — query-tool-legacy portlet entry
• Pagination across limit-size pages with URL-synced offset

🤖 Generated with Claude Code

This PR fixes: #35709

[claude]

Claude finished @hmoreras's task in 3m 52s —— View job

---

PR Review

Solid migration overall. The store/component split is clean, the URL sync via effect() + Location.replaceState is the right pattern, and the popup-blocker workaround on result clicks is thoughtful. A few real issues and a handful of nits below.

---

Bugs / Risks

1. Unreachable error handler in onResultClick — dot-query-tool-page.component.ts:244-247

DotContentletEditUrlService.resolveEditUrl() swallows content-type lookup failures via catchError(() => of(null)) and always emits a URL. The synchronous HTMLPAGE branch (of(pageUrl)) never errors either. So the error callback that closes the placeholder tab and shows the global error toast is dead code — when the API actually fails, the user gets the legacy editor URL silently (which may or may not work). Two options:

• Stop swallowing in the service (let the caller decide), OR
• Remove the dead error branch and accept the silent legacy fallback as the contract.

Either way the current code documents an error path that cannot fire.

2. userId URL param is not gated by admin status — dot-query-tool-page.component.ts:202 + dot-query-tool.store.ts:100-109

ngOnInit reads userId from query params and stores it before any admin check. apiRequestBody then includes it in every search payload as long as it's non-empty. A non-admin who pastes (or is sent) a link like /query-tool?q=...&userId=admin@dotcms.com will issue searches with userId set. The PR description says the server enforces this, and that's the right primary defense — but the client should at minimum:

• Not hydrate userId from the URL when !isAdmin, and
• Not write userId into the URL via the sync effect for non-admins.

Otherwise the shareable URL leaks impersonation intent in both directions.

3. urlMap / url may not be strings — dot-contentlet-edit-url.service.ts:125

const url = (contentlet['urlMap'] as string) || (contentlet['url'] as string);

Both fields are accessed as unknown and cast. If a contentlet has a non-string urlMap (object, array, number from a misconfigured field), URLSearchParams will stringify it to [object Object] and the page editor opens with a broken URL. Add typeof === 'string' guards, or null-coalesce to null and fall through to the inode-based resolver.

4. url === this.#router.url short-circuit is misleading — dot-query-tool-page.component.ts:130

Location.replaceState intentionally does not update Angular Router.url (that's the whole point of using Location instead of Router.navigate — preserve the component instance). So after the first sync, this.#router.url keeps reflecting the original URL, never the synced one. The comparison is effectively dead after the first run; #lastSyncedUrl alone is doing the work. Either drop the router.url comparison or comment why it exists (handles the "first run already matches the URL we'd write" case). Right now it reads like a load-bearing check and isn't.

---

Smaller issues

5. Endpoint string duplicated — SEARCH_ENDPOINT = '/api/v1/content/_search' is hardcoded in both dot-query-tool-page.component.ts:60 (for snippet building) and dot-query-tool.service.ts:20 (actual call). If one ever moves, the copy/cURL snippets silently lie. Export from the service or a small constants file.

6. Missing test for the limit-cap input UX described in the doc — the store comment at dot-query-tool.store.ts:139-155 makes a careful UX claim ("user sees the cap explanation alongside the capped results instead of an input value silently changing mid-loading"). The store tests cover this. The component tests do not assert the p-message warning renders when limitWasCapped() is true — that's the user-visible piece of the contract. Worth a quick byTestId('query-tool-size-cap-warning') assertion.

7. No test for the error branch of onResultClick — even if you keep the dead handler (see #1), there's no test exercising it. Conversely if you remove it, no test needs updating. This is a tell that the branch isn't real.

8. Help examples assume htmlpageasset / fileAsset exist — dot-query-tool-page.component.ts:177,182 hard-codes content type variables. Customer instances that have renamed or removed these defaults will see help examples that return zero results. Not a blocker; consider a comment that flags it for future i18n/customization.

9. setLimit(NaN) keeps NaN — dot-query-tool.store.ts:127: Math.max(1, NaN) is NaN. In practice the p-inputNumber won't send NaN, but the helper #parseInt already protects URL hydration. Belt-and-suspenders: Math.max(1, Number.isFinite(limit) ? limit : DEFAULT_LIMIT).

10. ES Search copyAs becomes private while Query Tool's becomes #private — for consistency with the recently-applied review (using # prefix in the new portlet), the same convention should apply to the older ES Search component (dot-es-search-page.component.ts:280,286,337,349,357). Since this PR is already touching that file, worth aligning. Not blocking.

---

Things I checked and they're fine

• Backend SearchForm uses camelCase userId — matches frontend payload (the ES Search lowercase userid is a different endpoint convention; no drift here).
• URLSearchParams properly encodes the url parameter into the page edit hash route — no XSS via the page URL.
• inode is interpolated into hash routes only, not document URL — even a malicious inode can't escape the hash.
• effect() reads status() tracked, everything else untracked() — no infinite-loop risk.
• switchMap correctly cancels prior in-flight searches when Run is clicked twice.
• tapResponse resolves the EMPTY observable test case as expected (status stays LOADING).
• Portlet count assertion in SerializationHelperTest (52 → 53) matches the one new query-tool-legacy entry.

---

Suggested fixes

For the top two issues, here are direct hooks for Claude:

Fix #1 (dead error path) →

Fix #2 (userId admin gating) →

---

Net: one real bug (#1 dead error path produces silent failures), one security-shaped item (#2 client-side admin gating is missing), and a handful of polish items. Nothing that blocks the migration concept.
· branch: issue-35709-query-tool-angular