From b82544236381095e9ca04936c46d6d5a7471d502 Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 11 Sep 2026 13:10:29 -0500 Subject: [PATCH 01/33] spec(content-drive): Title / All Content scope selector for the search box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec-Kit PR 1 for #37479. Carries spec.md alone. Settles the four decisions the issue left open: All Content stays the default, Title mode matches the contentlet title only, the scope lives in the URL rather than a user preference, and sorting is untouched. Four premises verified against main and corrected in the spec: - Nothing sorts by score today — the default is modDate:desc on both sides, and the only trace of score sorting is a stale comment. Open decision 4 is void. - Folders and links never reach Elasticsearch; they are already matched on name only, in both scopes, so the selector governs the contentlet clause alone. - buildPureESQuery is unreachable under the shipped heuristic, so it stays out of scope. - A scope written into the filters on every selection would light up "Clear all" on an unfiltered drive, because hasNonDefaultFilters counts every key but two. Hence FR-021: the scope counts as filter state only while it differs from the default. Refs #37479 Co-Authored-By: Claude Opus 5 (1M context) --- .../37479-content-drive-search-scope/spec.md | 391 ++++++++++++++++++ 1 file changed, 391 insertions(+) create mode 100644 specs/37479-content-drive-search-scope/spec.md diff --git a/specs/37479-content-drive-search-scope/spec.md b/specs/37479-content-drive-search-scope/spec.md new file mode 100644 index 000000000000..25685d0d565f --- /dev/null +++ b/specs/37479-content-drive-search-scope/spec.md @@ -0,0 +1,391 @@ +# Feature Specification: Content Drive — Title / All Content scope selector for the search box + +**Feature Branch**: `issue-37479-content-drive-search-scope` + +**Created**: 2026-09-11 + +**Status**: Draft + +**Type**: New Feature + +**Related GitHub Issue**: [#37479](https://github.com/dotCMS/core/issues/37479) — related history: [#36688](https://github.com/dotCMS/core/issues/36688) (replaced the broad `catchall:*kw*` wildcard with the current strategy), [#36814](https://github.com/dotCMS/core/issues/36814) (search performance at scale) + +**Input**: User description: "Content Drive: add a Title / All Content scope selector to the search box. Default scope = All Content (no regression); Title mode matches strictly the contentlet title plus folder names, not fileName/metadata.name; scope persists only in the URL filters; sorting unchanged." + +--- + +## Premise Corrections + +Four things the issue takes for granted do not survive verification against `main`. Three of them +narrow the work; the fourth adds a rule the issue's file list has no place for. + +### 1. Results are **not** sorted by score when a term is present + +Open decision 4 asks to *"confirm that score-descending sorting holds in Title mode."* There is no +score sorting to hold. The drive sends whatever sort the grid holds, and its default is +modification date: + +- `core-web/.../dot-content-drive/portlet/src/lib/shared/constants.ts:51-54` — `DEFAULT_SORT = { field: 'modDate', order: DESC }` +- `.../store/dot-content-drive.store.ts:157` — `sortBy: sort()?.field + ':' + sort()?.order`, unconditionally +- `AbstractDriveRequestForm.java:81` — the server default is `SORT_BY = "modDate"` too + +The only trace of score sorting is a **stale comment** at `dot-content-drive.store.ts:481` +("Since we are using scored search for the title we need to sort by score desc") sitting above code +that does nothing of the kind. Nothing in `BrowserAPIImpl` or `ContentDriveHelper` substitutes a +score sort when a filter is set. + +**Consequence for this feature**: open decision 4 is void. Sorting is out of scope entirely — both +scopes keep sending the grid's sort, and neither introduces a scope-dependent sort rule. The stale +comment should be corrected while the file is open (progressive enhancement), not acted upon. + +### 2. Folders and links are already matched on name only, in both scopes + +The issue frames "match against the contentlet title **(and folder/file names)** only" as new Title +behavior. Folders and links never reach Elasticsearch at all — they are loaded from the database and +narrowed in Java by a case-insensitive substring on their own name: + +- `BrowserAPIImpl.java:3026` — `folders.removeIf(f -> !f.getName().toLowerCase().contains(browserQuery.filter.toLowerCase()))`, gated on `filterFolderNames` +- `BrowserAPIImpl.java:2908-2913` — the equivalent for links, on `Link::getTitle`, deliberately **not** gated on that flag + +**Consequence for this feature**: folder and link matching is scope-independent and must not change. +The scope selector governs only the Elasticsearch clause used for **contentlets**. An acceptance +criterion that reads "Title mode returns only rows whose title or folder name matches" is already +half-true today and stays true in All Content mode. + +### 3. `buildPureESQuery` is unreachable for Content Drive under shipped configuration + +The issue flags the older `title:'*x*'^5 OR catchall:*x*^3 OR fileName:*x*^2` shape in +`buildPureESQuery` (~line 610) as *"worth confirming whether the drive reaches it"*. It does not: +`doPureESQuery` is selected only when `BROWSE_API_HEURISTIC_TYPE` is set to `PURE_ES`, and the +shipped default is `HYBRID_SINGLE_CHUNKED_QUERY_ES` (`BrowserAPIImpl.java:701-709`), which routes to +`buildBaseESQuery`. + +**Consequence for this feature**: `buildPureESQuery` is **out of scope**. Changing it would mean +changing a code path no shipped configuration exercises, and doing so unverified is worse than +leaving it consistent with its own heuristic. + +### 4. Storing the scope as a filter would offer "Clear all" on an unfiltered drive + +The issue's file list routes the scope through the drive's filter state — which is what carries it +into the address — but stops there. That state also feeds the filter chip bar, and +`hasNonDefaultFilters` (`utils/functions.ts:334-355`) treats **every** key other than `sharedAssets` +and `languageId` as a non-default filter. That signal is exactly what shows the bar's "Clear all" +(`dot-filter-bar.component.html:7`). + +So a scope written into the filters on every selection would light up "Clear all" the moment someone +picked **All Content** — the default — on a drive where nothing is filtered at all. + +**Consequence for this feature**: FR-021. The scope counts as filter state only while it differs +from the default, mirroring how the search term already deletes its own key when it goes empty +(`dot-content-drive.store.ts:228-234`). + +--- + +--- + +## Decisions (settled 2026-09-11) + +The issue leaves four decisions open and marks them as needing a call before implementation. The +issue owner settled them on 2026-09-11. **Approval of this spec (PR 1) is the record of that +sign-off**; if any of them is reversed, the spec must be re-approved before `/speckit-plan` runs. + +| # | Decision | Settled as | Why | +|---|---|---|---| +| 1 | Default scope | **All Content** | The no-regression choice. Every user who does nothing keeps exactly the results they get today, and the fast path is opt-in. Defaulting to Title would silently change what an existing saved workflow returns. | +| 2 | Field coverage in Title mode | **Contentlet `title` only** — not `fileName`, not `metadata.name` | Keeps the promise the label makes, and keeps the query to a single field so the cost argument holds. File assets are still reachable by name in practice (see [Assumptions](#assumptions)). | +| 3 | Stickiness | **URL only, per search** | The scope behaves like every other Content Drive filter: it survives reload, Back and Forward, and a shared link, and resets to the default on a clean entry. No new per-user preference storage. | +| 4 | Sorting in Title mode | **Unchanged** | Void as asked — see [Premise Correction 1](#1-results-are-not-sorted-by-score-when-a-term-is-present). Both scopes send the grid's current sort. | + +--- + +## Problem Statement + +The Content Drive search box has exactly one behavior: every term runs a global, all-content search. +An author who knows the **name** of what they are looking for has no way to say so. The term is +matched against every indexed field of every document — body copy, Story Block content, metadata — +so a common word returns a large slice of the drive and the one row the author wanted is buried +among documents that merely mention it. + +The same breadth is also the expensive part of the request. The mandatory gate of the all-content +query is `+(catchall:*^10 OR title_dotraw:**^2)` +(`GlobalSearchAttributeStrategy.java:38-40`): `catchall` aggregates every field of the document, so +a common term is cheap to look up and enormous in what it returns, and `title_dotraw:**` is a +leading wildcard, which forces a scan over every distinct raw title rather than a prefix seek. + +In the drive, the matched set is not the end of the work. Matches are fed through database hydration +and per-chunk permission filtering (`BROWSER_CONTENT_CHUNK_SIZE`, default 900) before a page can be +returned, so a broad match multiplies database round trips and permission checks — not just index +time. Narrowing the candidate set at the source makes everything downstream cheaper with it. + +So the scope selector earns its place twice: it is the result quality authors are asking for, and it +gives them a fast path that avoids the most expensive clause in the query. + +## UI Surface + +The ASCII diagram in the issue is the only mock — no image or design file accompanies it. What it +establishes, and all this spec fixes, is which components are on screen: + +- The **search input** — the Content Drive search box as it exists today. +- A **scope dropdown beside it**, offering **Title** and **All Content**. Its label is the active + scope; opening it marks the active option with a check. +- The **placeholder** of the input, which follows the active scope. + +Nothing else about the control's appearance is fixed here. How the two sit together is an +implementation decision. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Find a known item by its name (Priority: P1) + +An author knows the name of the item they want — a page called "Pricing", an image called "hero" — +and types it into the Content Drive search box. Today they get back everything whose body or blocks +happen to contain the word. They open the scope control next to the box, choose **Title**, and the +list narrows to rows whose name actually matches. The placeholder changes to say *Search by title*, +so the box states what it will do before they type again. + +**Why this priority**: This is the whole feature. It delivers the result quality the issue was +raised for and the cheap query path on its own, with nothing else built. Stories 2 and 3 protect it; +neither creates value without it. + +**Independent Test**: Fully testable by selecting **Title** with a term present and confirming that +a document which contains the term only in its body or Story Block — and not in its name — +disappears from the list, while the row whose name matches stays. Delivers the narrowing on its own. + +**Acceptance Scenarios**: + +1. **Given** the Content Drive is open with no search term, **When** the author looks at the search + box, **Then** a scope control is visible next to the input, reading **All Content**, and the + placeholder describes an all-content search. +2. **Given** a term is present in **All Content** scope, **When** the author opens the scope control + and selects **Title**, **Then** the search re-runs immediately with the same term, results are + restricted to name matches, pagination returns to page 1, and the control reads **Title** with a + check mark beside that option. +3. **Given** a document whose title does **not** contain the term but whose body or Story Block + does, **When** the scope is **Title**, **Then** that document is absent from the results. +4. **Given** that same document, **When** the scope is **All Content**, **Then** it is present — + the all-content results are identical to what the drive returns today for that term. +5. **Given** scope **Title** and a term that matches a folder's name, **When** the search runs, + **Then** the folder is listed, exactly as it is in **All Content** scope. +6. **Given** the scope control is open, **When** the author selects the scope that is already + active, **Then** nothing is re-fetched and the current page is preserved. + +--- + +### User Story 2 - The scope travels with the view (Priority: P2) + +An author narrows to **Title**, finds the row, opens it, and comes back with the browser Back +button — the drive returns to the Title-scoped results, not to an all-content list. They copy the +address and send it to a colleague, who opens the same narrowed view. + +**Why this priority**: Without it the scope silently resets on every reload and navigation, and an +author who has narrowed their search loses that narrowing without being told. It is a correctness +guarantee over Story 1 rather than a capability of its own, so it ranks below it. + +**Independent Test**: Fully testable by selecting **Title**, reloading the page, and confirming the +control still reads **Title** and the results are still narrowed — then navigating away and back. + +**Acceptance Scenarios**: + +1. **Given** scope **Title** with a term, **When** the page is reloaded, **Then** the control reads + **Title**, the term is preserved, and the results are the Title-scoped results. +2. **Given** the author switched from **All Content** to **Title**, **When** they press browser + Back, **Then** the view returns to the **All Content** results for that term. +3. **Given** a Content Drive address carrying scope **Title**, **When** a different user opens it, + **Then** they see the same narrowed view, subject to their own permissions. +4. **Given** the author enters Content Drive with no scope in the address, **When** the drive loads, + **Then** the scope is **All Content**. +5. **Given** an address carrying an unrecognized scope value, **When** the drive loads, **Then** the + scope falls back to **All Content** and the drive loads normally, with no error surfaced. +6. **Given** the author clears all filters, **When** the drive reloads its results, **Then** the + scope returns to **All Content** along with the other filter defaults. + +--- + +### User Story 3 - Everything that is not Content Drive is untouched (Priority: P3) + +A developer using the Asset Picker, and an integration calling the Content Drive search endpoint, +see no change at all. The Asset Picker's search box keeps the single all-content behavior it has +today, with no scope control on screen. A request that does not mention the scope behaves exactly as +it does now. + +**Why this priority**: It is a constraint on Stories 1 and 2 rather than a journey of its own, and +it is verified by absence. It still has to be stated and tested, because the search box is shared +and the endpoint is public. + +**Independent Test**: Fully testable by opening the Asset Picker and confirming no scope control +appears and search behaves as before, and by replaying a stored Content Drive search request with no +scope field and comparing the results to the current ones. + +**Acceptance Scenarios**: + +1. **Given** the Asset Picker is open, **When** the author looks at its search box, **Then** no + scope control is present and searching behaves exactly as it does today. +2. **Given** a Content Drive search request that omits the scope entirely, **When** it is processed, + **Then** the results are identical to today's all-content results. +3. **Given** a Content Drive search request that names the all-content scope explicitly, **When** it + is processed, **Then** the results are identical to the request that omits it. +4. **Given** a Content Drive search request naming a scope value the system does not recognize, + **When** it is processed, **Then** it is rejected with a client error that names the offending + value, rather than silently widening or narrowing the results. + +--- + +### Edge Cases + +- **Scope changed with an empty term.** No search is narrowed and nothing is re-fetched beyond the + drive's normal unfiltered listing; the control still records the new scope so the next term uses + it, and the placeholder updates. +- **Scope changed while a search is in flight.** The later request is the one whose results are + shown; an earlier in-flight response never overwrites it. +- **Term matches only folder or link names, in Title scope.** Those rows are listed — folder and + link matching is scope-independent ([Premise Correction 2](#2-folders-and-links-are-already-matched-on-name-only-in-both-scopes)). +- **Multi-word term in Title scope.** The term narrows rather than widens: a row must be a name + match for the phrase as entered, not merely for one of its words. +- **Term containing characters the query syntax treats specially.** Handled the same way in both + scopes; a term is never allowed to alter the structure of the query. +- **Scope combined with the other Content Drive filters** — content type, language, status, + workflow, shared assets, per-field filters. The scope narrows the text match only; every other + filter keeps applying as it does today, and combining them narrows further rather than + conflicting. +- **Scope selected while a folder is selected in the tree.** A new search already resets the folder + scope to the site root; changing the scope of an existing search behaves consistently with that. +- **Scope set explicitly back to All Content.** The drive returns to the state it would have had if + the control had never been touched: nothing recorded in the address, and no "clear all filters" + offered on an otherwise unfiltered drive. +- **A file whose title was edited to something other than its file name**, searched by file name in + Title scope: it does not match. See [Assumptions](#assumptions). + +## Requirements *(mandatory)* + +### Functional Requirements + +**The control** + +- **FR-001**: The Content Drive search box MUST present a scope control adjacent to the search + input, within the same visual container, offering exactly two options: **Title** and + **All Content**. +- **FR-002**: The control MUST display the active scope as its label, and MUST mark the active + option with a check when opened. +- **FR-003**: The search input's placeholder MUST describe the active scope, so the box states what + it will do before the author types. +- **FR-004**: Selecting a scope MUST re-run the current search immediately, without requiring the + author to retype or re-submit the term. +- **FR-005**: Selecting a scope MUST reset pagination to the first page. +- **FR-006**: Re-selecting the already-active scope MUST NOT trigger a new search. +- **FR-007**: The scope control MUST be reachable and operable by keyboard and MUST expose its + current selection to assistive technology. + +**Behavior** + +- **FR-008**: In **Title** scope, a contentlet MUST be returned only when its title matches the + term. A contentlet whose term occurrence is confined to body copy, Story Block content or + metadata MUST NOT be returned. +- **FR-009**: In **All Content** scope, results MUST be identical to what Content Drive search + returns today for the same term and filters — no regression of any kind. +- **FR-010**: In **Title** scope, the query MUST NOT use an all-fields aggregate clause, and MUST + NOT use a leading-wildcard term as its mandatory gate. This is the requirement that makes the + scope a genuine fast path rather than a display filter. +- **FR-011**: Folder and link name matching MUST behave identically in both scopes. +- **FR-012**: The active sort MUST be unaffected by the scope; both scopes MUST apply the sort the + author has chosen, with the existing default. +- **FR-013**: The scope MUST compose with every other Content Drive filter without altering their + behavior. + +**State and contract** + +- **FR-014**: A non-default scope MUST be encoded in the address alongside the other Content Drive + filters, and MUST be restored from it on reload and on browser Back/Forward. +- **FR-015**: An absent or unrecognized scope in the address MUST resolve to **All Content**, + without surfacing an error. +- **FR-016**: The scope MUST NOT be persisted as a per-user preference; a clean entry into Content + Drive MUST start at **All Content**. +- **FR-017**: The Content Drive search request MUST carry the scope as an optional field that + defaults to all-content behavior, so a request that omits it is processed exactly as it is today. +- **FR-018**: A request naming an unrecognized scope value MUST be rejected with a client error + identifying the value, rather than silently defaulting. +- **FR-019**: The shared search box MUST expose the scope control as opt-in. Surfaces that do not + opt in — the Asset Picker today — MUST render and behave exactly as they do now. +- **FR-020**: Clearing all filters MUST return the scope to **All Content**. +- **FR-021**: The scope MUST count as filter state only while it is not the default. Selecting + **All Content** MUST leave the drive in the state it would have been in had the control never been + touched — in particular, it MUST NOT cause a "clear all filters" affordance to be offered on a + drive that is otherwise unfiltered. + +### Key Entities + +- **Search scope**: Which part of a document a Content Drive search term is matched against. Two + values — *Title* and *All Content* — with *All Content* as the default. Lives alongside the search + term as part of the drive's filter state, is carried in the address, and is sent with the search + request. +- **Content Drive search request**: The existing description of what the drive should list — + location, term, content types, languages, status, workflow, per-field criteria, sort, paging. + Gains the scope as one more optional element of the text-matching part. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: On a drive containing a document whose body mentions the search term and a document + whose name is the search term, an author in **Title** scope sees only the second — verified as a + binary pass on a seeded dataset. +- **SC-002**: For any term and filter combination, **All Content** results are byte-identical to + the results the same drive returns before this change — zero regressions across the search cases + covered by the endpoint's test suite. +- **SC-003**: On a large dataset, a **Title** search returns its first page faster than the same + term in **All Content** scope, and the before/after comparison is recorded on the issue. The + target is a measurable reduction, not a fixed threshold; the comparison itself is the deliverable + the issue asks for. +- **SC-004**: A Content Drive address carrying a scope reproduces the same narrowed view for a + second user 100% of the time, and reload and Back/Forward preserve the scope in 100% of attempts. +- **SC-005**: 100% of search requests that omit the scope produce today's results — confirmed by an + explicit endpoint test for the omitted field, not only by the explicit all-content case. +- **SC-006**: The Asset Picker's search box shows no scope control and its search behavior is + unchanged, confirmed by its existing tests passing without modification. +- **SC-007**: An author who knows the name of the item they want reaches it from the search box + without scrolling past unrelated body matches, on a drive where the all-content search for the + same term returns more than one page. + +## Legacy Considerations *(dotCMS-specific — mandatory)* + +- **Existing behavior touched**: Content Drive's keyword search, and the browsing service that + backs it. The browsing service is long-standing, pre-Content-Drive product surface shared with + other file-browsing entry points; Content Drive's search endpoint and its front end are recent. + The shared search box is also used by the Asset Picker, which is explicitly not in scope. +- **Backward-compatibility expectations**: Strict. All-content search must be unchanged for every + existing caller and every existing address; the scope is additive and optional at every layer, and + its absence must be indistinguishable from today. No existing behavior is deprecated. The + all-content query strategy is shared with the Search portlet and the Relationships dialog and must + keep serving them unmodified — the Title scope is a sibling path, not a branch inside the existing + one. +- **Known related decisions**: [#36688](https://github.com/dotCMS/core/issues/36688) deliberately + replaced a broad leading-wildcard all-fields query with the current strategy, for the same cost + reasons argued here — the Title scope must not reintroduce what that issue removed. + [#36814](https://github.com/dotCMS/core/issues/36814) tracks search performance at scale and is + where SC-003's measurement belongs. `/speckit-plan` will formally consult `dotCMS/platform-adrs`. + +## Assumptions + +- **File assets remain findable by name in Title scope, through their title.** Decision 2 excludes + `fileName` and `metadata.name`, but a file asset carries a title field that dotCMS keeps in step + with the file name — it is seeded from it and rewritten on rename + (`FileAssetAPIImpl.java:498`). So searching a file by its name works in Title scope for the + ordinary case. The gap is narrow and deliberate: a file whose title has been edited to something + other than its file name will not match on the file name. If that gap proves to matter in + practice, widening Title scope to cover file names is a follow-up with its own spec, not a + silent change here. +- **The issue's ASCII diagram is the whole design input.** No mock image or design file exists for + this control, and none is being waited on. The spec fixes which components are present and how + they behave; their appearance is settled during implementation. +- **The scope is a front-end-visible concept only for Content Drive.** No other portlet gains it in + this work, and the query strategy shared with the Search portlet and the Relationships dialog is + left as-is. +- **"Title" is the label authors understand**, including for folders and files, and does not need to + read differently per row type. +- **The existing address-encoding scheme for Content Drive filters can carry the scope** without a + new mechanism, and an unknown value degrades to the default the same way an unknown status does + today. +- **The measurement in SC-003 needs a dataset large enough for the difference to exceed noise.** The + issue does not name one; producing it is part of the work, and the comparison is recorded on the + issue rather than in the repository. +- **No database, index-mapping or content-model change is required.** The scope selects between two + ways of querying what is already indexed. From 77abd40b773e88ebc5ebdcbf2654f2e1ed30a455 Mon Sep 17 00:00:00 2001 From: Kevin Date: Sun, 13 Sep 2026 22:14:47 -0500 Subject: [PATCH 02/33] =?UTF-8?q?spec(content-drive):=20address=20review?= =?UTF-8?q?=20round=201=20=E2=80=94=20All=20Fields,=20searchScope,=20ADR-0?= =?UTF-8?q?018?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four asks from @zJaaal and one from @ihoffmann-dot, all settled in a new Review Decisions table (rows 5-8) so PR 1's approval records them. Renamed the wide option "All Content" -> All Fields (values TITLE / ALL_FIELDS). "All Content" described a set of content, which is what #37426's browse scope All genuinely means; this scope widens which *fields* are read, not which content is searched. The issue's original wording stays recorded verbatim in Input. Named the concept "search scope" throughout and the wire field filters.searchScope, inside the existing filters object rather than at the top level. AbstractQueryFilters is {text, filterFolders} today and filterFolders' own Javadoc says "when text is provided" — both members qualify the text search, which is exactly what the search scope does. FR-025 makes a scope with no text a contract error rather than a rule to remember. The browse scope stays top level for the opposite reasons. FR-017 now names the Asset Picker as the caller the optional-field requirement protects, with its actual call site, so a future change to the default has to confront it by name. FR-024 pins the change to the text-search branch and names the three other doors into the same listing (WebAssetHelper, BrowserAjax, DotCMSMacroWebAPI) that would widen the blast radius from two callers to six. SC-008 measures it. ADR-0018's Title -> DB ∪ Index routing gets its own section and an explicit deferral rather than silence. The ADR defers its own union: it states contentlet.title is "not reliably populated" and that fixing that is a separate issue. Verified that no text search consults the column today — Premise Correction 5 shows ContentDriveHelper:180-184 sets useElasticsearchFiltering(true) unconditionally when text is present, so the SQL ILIKE text path is unreachable for Content Drive and contentlet matching is index-only in both scopes. Title scope inherits the existing index-lag exposure rather than creating it, and FR-026 keeps the union additive for when the gated work lands. Also adds FR-022 (the control must explain what each option matches — two labels do not carry the distinction on their own). Refs #37479 Co-Authored-By: Claude Opus 5 (1M context) --- .../37479-content-drive-search-scope/spec.md | 429 ++++++++++++------ 1 file changed, 280 insertions(+), 149 deletions(-) diff --git a/specs/37479-content-drive-search-scope/spec.md b/specs/37479-content-drive-search-scope/spec.md index 25685d0d565f..f19cf7308c7e 100644 --- a/specs/37479-content-drive-search-scope/spec.md +++ b/specs/37479-content-drive-search-scope/spec.md @@ -1,23 +1,28 @@ -# Feature Specification: Content Drive — Title / All Content scope selector for the search box +# Feature Specification: Content Drive — Title / All Fields search scope selector for the search box **Feature Branch**: `issue-37479-content-drive-search-scope` **Created**: 2026-09-11 +**Last revised**: 2026-09-13 — review round 1 (see [Review Decisions](#review-decisions-settled-2026-09-13)) + **Status**: Draft **Type**: New Feature -**Related GitHub Issue**: [#37479](https://github.com/dotCMS/core/issues/37479) — related history: [#36688](https://github.com/dotCMS/core/issues/36688) (replaced the broad `catchall:*kw*` wildcard with the current strategy), [#36814](https://github.com/dotCMS/core/issues/36814) (search performance at scale) +**Related GitHub Issue**: [#37479](https://github.com/dotCMS/core/issues/37479) — related history: [#36688](https://github.com/dotCMS/core/issues/36688) (replaced the broad `catchall:*kw*` wildcard with the current strategy), [#36814](https://github.com/dotCMS/core/issues/36814) (search performance at scale). Sibling in flight: [#37426](https://github.com/dotCMS/core/issues/37426) / [PR #37487](https://github.com/dotCMS/core/pull/37487) (Content Drive **browse** scopes) — the two features land fields on the same request object and are deliberately named apart; see [Review Decision 5](#review-decisions-settled-2026-09-13). **Input**: User description: "Content Drive: add a Title / All Content scope selector to the search box. Default scope = All Content (no regression); Title mode matches strictly the contentlet title plus folder names, not fileName/metadata.name; scope persists only in the URL filters; sorting unchanged." +> The issue's words are recorded verbatim above. The wide option is named **All Fields** in this spec — see [Review Decision 7](#review-decisions-settled-2026-09-13) for why "All Content" was rejected. + --- ## Premise Corrections -Four things the issue takes for granted do not survive verification against `main`. Three of them -narrow the work; the fourth adds a rule the issue's file list has no place for. +Five things the issue takes for granted do not survive verification against `main`. Three of them +narrow the work; the fourth adds a rule the issue's file list has no place for; the fifth settles +which query path the scope actually governs. ### 1. Results are **not** sorted by score when a term is present @@ -48,9 +53,9 @@ narrowed in Java by a case-insensitive substring on their own name: - `BrowserAPIImpl.java:2908-2913` — the equivalent for links, on `Link::getTitle`, deliberately **not** gated on that flag **Consequence for this feature**: folder and link matching is scope-independent and must not change. -The scope selector governs only the Elasticsearch clause used for **contentlets**. An acceptance +The search scope governs only the Elasticsearch clause used for **contentlets**. An acceptance criterion that reads "Title mode returns only rows whose title or folder name matches" is already -half-true today and stays true in All Content mode. +half-true today and stays true in All Fields mode. ### 3. `buildPureESQuery` is unreachable for Content Drive under shipped configuration @@ -73,13 +78,30 @@ and `languageId` as a non-default filter. That signal is exactly what shows the (`dot-filter-bar.component.html:7`). So a scope written into the filters on every selection would light up "Clear all" the moment someone -picked **All Content** — the default — on a drive where nothing is filtered at all. +picked **All Fields** — the default — on a drive where nothing is filtered at all. **Consequence for this feature**: FR-021. The scope counts as filter state only while it differs from the default, mirroring how the search term already deletes its own key when it goes empty (`dot-content-drive.store.ts:228-234`). ---- +### 5. Content Drive's contentlet text search is always index-routed — the SQL text path is unreachable + +The browsing service can match a text filter two different ways. `BrowserAPIImpl` either hands the +term to Elasticsearch or, when `useElasticsearchFiltering` is false, builds a SQL predicate that +runs `contentlet_as_json::text ILIKE '%token%'` over the whole serialized contentlet plus the asset +name (`BrowserAPIImpl.java:2053-2060`, `appendFilterQuery` at `:2222`). The builder's own default is +`false` (`BrowserQuery.java:292`), so the SQL path is the fallback in general. + +Content Drive never takes it. `ContentDriveHelper` sets `useElasticsearchFiltering(true)` +unconditionally whenever the request carries text (`ContentDriveHelper.java:180-184`), and +`isUseElasticSearchForFiltering` then returns true because a text filter is present +(`BrowserAPIImpl.java:1582-1589`). + +**Consequence for this feature**: the search scope has exactly one query path to govern, and +FR-010's "no all-fields aggregate, no leading-wildcard gate" is a statement about the Elasticsearch +clause alone. There is no second, SQL-shaped text match that a Title scope could silently fail to +narrow. This is also what settles the ADR-0018 question — see +[ADR-0018 alignment](#adr-0018-alignment). --- @@ -91,22 +113,61 @@ sign-off**; if any of them is reversed, the spec must be re-approved before `/sp | # | Decision | Settled as | Why | |---|---|---|---| -| 1 | Default scope | **All Content** | The no-regression choice. Every user who does nothing keeps exactly the results they get today, and the fast path is opt-in. Defaulting to Title would silently change what an existing saved workflow returns. | +| 1 | Default scope | **All Fields** | The no-regression choice. Every user who does nothing keeps exactly the results they get today, and the fast path is opt-in. Defaulting to Title would silently change what an existing saved workflow returns. | | 2 | Field coverage in Title mode | **Contentlet `title` only** — not `fileName`, not `metadata.name` | Keeps the promise the label makes, and keeps the query to a single field so the cost argument holds. File assets are still reachable by name in practice (see [Assumptions](#assumptions)). | | 3 | Stickiness | **URL only, per search** | The scope behaves like every other Content Drive filter: it survives reload, Back and Forward, and a shared link, and resets to the default on a clean entry. No new per-user preference storage. | | 4 | Sorting in Title mode | **Unchanged** | Void as asked — see [Premise Correction 1](#1-results-are-not-sorted-by-score-when-a-term-is-present). Both scopes send the grid's current sort. | +## Review Decisions (settled 2026-09-13) + +Review round 1 on [PR #37518](https://github.com/dotCMS/core/pull/37518) raised four further calls. +They are settled here and are part of what PR 1's approval signs off. + +| # | Decision | Settled as | Why | +|---|---|---|---| +| 5 | What the two scopes are called in prose | **"search scope"**, never bare "scope" | Content Drive is simultaneously gaining a **browse** scope (#37426) that says *where* you are browsing. Bare "scope" would name either, and both end up as fields on the same request object, so the ambiguity would outlive the specs. The sibling spec spells "browse scope" throughout for the matching half. | +| 6 | The wire name and its home | **`filters.searchScope`**, inside the existing filters object | `AbstractQueryFilters` is `{ text, filterFolders }` today, and `filterFolders`' Javadoc says "when text is provided". Both members exist to qualify the text search, which is exactly what the search scope does — it says which fields `text` reads and means nothing without `text`. Beside `text`, they travel together and a scope with no text is visibly nonsense rather than a validation rule someone must remember. The browse scope stays top level for the opposite reasons: it qualifies `assetPath`, and "Clear all" resets `filters`, which must never be able to navigate you elsewhere. | +| 7 | The label of the wide option | **All Fields** (values `TITLE` / `ALL_FIELDS`) | "All Content" describes a *set of content*, which is what the browse scope's **All** genuinely means. This scope does not widen which content is searched — it widens which **fields** of each document the term is read against. Naming it "All Content" would have put two different meanings of the same word on the same request. | +| 8 | ADR-0018's `DB ∪ Index` title routing | **Out of scope, explicitly** — Title scope inherits today's index-only routing | See [ADR-0018 alignment](#adr-0018-alignment). The union is gated on title-persistence work the ADR itself defers, and no text search consults the DB title column today. Title scope neither creates nor widens that gap, and it picks the union up for free when the gated work lands. | + +## ADR-0018 alignment + +[ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md) +routes the **Title** criterion to `DB ∪ Index` — the `contentlet.title` column unioned with index +records — so that a just-saved or just-renamed item stays findable while the index catches up. This +feature's Title scope is defined as an Elasticsearch clause and consults no DB column. That is a +deliberate call, not an omission, for three reasons: + +1. **The ADR defers its own union.** It states that `contentlet.title` "is **not reliably + populated**", that the display title is derived and `Contentlet.title()` is `@Nullable`, and that + "populating `contentlet.title` reliably is a known gap to be addressed in a separate issue". The + ADR fixes the routing contract that will *consume* the column once it lands; it does not claim + the column is usable today. +2. **No text search consults it today, in either scope.** Per + [Premise Correction 5](#5-content-drives-contentlet-text-search-is-always-index-routed--the-sql-text-path-is-unreachable), + Content Drive's contentlet text matching is index-only. The pre-existing exposure to index lag is + identical in All Fields scope and would be identical in Title scope. This feature does not + introduce it, and narrowing the clause does not deepen it — an item missing from the index is + missing from both scopes equally. +3. **The union arrives for free.** When the title-persistence issue lands and the text path becomes + `DB-title ∪ index`, Title scope is the scope that benefits most directly, because the DB column + it unions in *is* the title. Nothing in this spec has to be undone for that to happen. + +**What this means for the plan**: the implementation must not make Title scope *harder* to union +later — FR-026. Read-your-writes for the text path stays tracked where the ADR put it, outside this +feature. + --- ## Problem Statement -The Content Drive search box has exactly one behavior: every term runs a global, all-content search. -An author who knows the **name** of what they are looking for has no way to say so. The term is -matched against every indexed field of every document — body copy, Story Block content, metadata — -so a common word returns a large slice of the drive and the one row the author wanted is buried -among documents that merely mention it. +The Content Drive search box has exactly one behavior: every term is matched against every indexed +field of every document. An author who knows the **name** of what they are looking for has no way to +say so. The term hits body copy, Story Block content and metadata alike, so a common word returns a +large slice of the drive and the one row the author wanted is buried among documents that merely +mention it. -The same breadth is also the expensive part of the request. The mandatory gate of the all-content +The same breadth is also the expensive part of the request. The mandatory gate of the all-fields query is `+(catchall:*^10 OR title_dotraw:**^2)` (`GlobalSearchAttributeStrategy.java:38-40`): `catchall` aggregates every field of the document, so a common term is cheap to look up and enormous in what it returns, and `title_dotraw:**` is a @@ -117,8 +178,8 @@ and per-chunk permission filtering (`BROWSER_CONTENT_CHUNK_SIZE`, default 900) b returned, so a broad match multiplies database round trips and permission checks — not just index time. Narrowing the candidate set at the source makes everything downstream cheaper with it. -So the scope selector earns its place twice: it is the result quality authors are asking for, and it -gives them a fast path that avoids the most expensive clause in the query. +So the search scope selector earns its place twice: it is the result quality authors are asking for, +and it gives them a fast path that avoids the most expensive clause in the query. ## UI Surface @@ -126,12 +187,15 @@ The ASCII diagram in the issue is the only mock — no image or design file acco establishes, and all this spec fixes, is which components are on screen: - The **search input** — the Content Drive search box as it exists today. -- A **scope dropdown beside it**, offering **Title** and **All Content**. Its label is the active - scope; opening it marks the active option with a check. +- A **search scope control beside it**, offering **Title** and **All Fields**. Its label is the + active scope; opening it marks the active option with a check. +- A short **explanation of what each option matches**, available from the control. The distinction + between "the item's name" and "anything written anywhere in the item" is not self-evident from two + words, and the control is new (FR-022). - The **placeholder** of the input, which follows the active scope. -Nothing else about the control's appearance is fixed here. How the two sit together is an -implementation decision. +Nothing else about the control's appearance is fixed here. How the two sit together, and whether the +explanation is a tooltip, helper text or per-option description, are implementation decisions. ## User Scenarios & Testing *(mandatory)* @@ -139,9 +203,9 @@ implementation decision. An author knows the name of the item they want — a page called "Pricing", an image called "hero" — and types it into the Content Drive search box. Today they get back everything whose body or blocks -happen to contain the word. They open the scope control next to the box, choose **Title**, and the -list narrows to rows whose name actually matches. The placeholder changes to say *Search by title*, -so the box states what it will do before they type again. +happen to contain the word. They open the search scope control next to the box, choose **Title**, +and the list narrows to rows whose name actually matches. The placeholder changes to say *Search by +title*, so the box states what it will do before they type again. **Why this priority**: This is the whole feature. It delivers the result quality the issue was raised for and the cheap query path on its own, with nothing else built. Stories 2 and 3 protect it; @@ -154,106 +218,120 @@ disappears from the list, while the row whose name matches stays. Delivers the n **Acceptance Scenarios**: 1. **Given** the Content Drive is open with no search term, **When** the author looks at the search - box, **Then** a scope control is visible next to the input, reading **All Content**, and the - placeholder describes an all-content search. -2. **Given** a term is present in **All Content** scope, **When** the author opens the scope control - and selects **Title**, **Then** the search re-runs immediately with the same term, results are - restricted to name matches, pagination returns to page 1, and the control reads **Title** with a - check mark beside that option. + box, **Then** a search scope control is visible next to the input, reading **All Fields**, and + the placeholder describes an all-fields search. +2. **Given** a term is present in **All Fields** scope, **When** the author opens the search scope + control and selects **Title**, **Then** the search re-runs immediately with the same term, + results are restricted to name matches, pagination returns to page 1, and the control reads + **Title** with a check mark beside that option. 3. **Given** a document whose title does **not** contain the term but whose body or Story Block - does, **When** the scope is **Title**, **Then** that document is absent from the results. -4. **Given** that same document, **When** the scope is **All Content**, **Then** it is present — - the all-content results are identical to what the drive returns today for that term. -5. **Given** scope **Title** and a term that matches a folder's name, **When** the search runs, - **Then** the folder is listed, exactly as it is in **All Content** scope. -6. **Given** the scope control is open, **When** the author selects the scope that is already + does, **When** the search scope is **Title**, **Then** that document is absent from the results. +4. **Given** that same document, **When** the search scope is **All Fields**, **Then** it is + present — the all-fields results are identical to what the drive returns today for that term. +5. **Given** search scope **Title** and a term that matches a folder's name, **When** the search + runs, **Then** the folder is listed, exactly as it is in **All Fields** scope. +6. **Given** the search scope control is open, **When** the author selects the scope that is already active, **Then** nothing is re-fetched and the current page is preserved. +7. **Given** the author has not used this control before, **When** they open it, **Then** an + explanation is available that distinguishes matching the item's name from matching anything + written anywhere in the item. --- -### User Story 2 - The scope travels with the view (Priority: P2) +### User Story 2 - The search scope travels with the view (Priority: P2) An author narrows to **Title**, finds the row, opens it, and comes back with the browser Back -button — the drive returns to the Title-scoped results, not to an all-content list. They copy the +button — the drive returns to the Title-scoped results, not to an all-fields list. They copy the address and send it to a colleague, who opens the same narrowed view. -**Why this priority**: Without it the scope silently resets on every reload and navigation, and an -author who has narrowed their search loses that narrowing without being told. It is a correctness -guarantee over Story 1 rather than a capability of its own, so it ranks below it. +**Why this priority**: Without it the search scope silently resets on every reload and navigation, +and an author who has narrowed their search loses that narrowing without being told. It is a +correctness guarantee over Story 1 rather than a capability of its own, so it ranks below it. **Independent Test**: Fully testable by selecting **Title**, reloading the page, and confirming the control still reads **Title** and the results are still narrowed — then navigating away and back. **Acceptance Scenarios**: -1. **Given** scope **Title** with a term, **When** the page is reloaded, **Then** the control reads - **Title**, the term is preserved, and the results are the Title-scoped results. -2. **Given** the author switched from **All Content** to **Title**, **When** they press browser - Back, **Then** the view returns to the **All Content** results for that term. -3. **Given** a Content Drive address carrying scope **Title**, **When** a different user opens it, - **Then** they see the same narrowed view, subject to their own permissions. -4. **Given** the author enters Content Drive with no scope in the address, **When** the drive loads, - **Then** the scope is **All Content**. -5. **Given** an address carrying an unrecognized scope value, **When** the drive loads, **Then** the - scope falls back to **All Content** and the drive loads normally, with no error surfaced. +1. **Given** search scope **Title** with a term, **When** the page is reloaded, **Then** the control + reads **Title**, the term is preserved, and the results are the Title-scoped results. +2. **Given** the author switched from **All Fields** to **Title**, **When** they press browser Back, + **Then** the view returns to the **All Fields** results for that term. +3. **Given** a Content Drive address carrying search scope **Title**, **When** a different user + opens it, **Then** they see the same narrowed view, subject to their own permissions. +4. **Given** the author enters Content Drive with no search scope in the address, **When** the drive + loads, **Then** the search scope is **All Fields**. +5. **Given** an address carrying an unrecognized search scope value, **When** the drive loads, + **Then** the search scope falls back to **All Fields** and the drive loads normally, with no + error surfaced. 6. **Given** the author clears all filters, **When** the drive reloads its results, **Then** the - scope returns to **All Content** along with the other filter defaults. + search scope returns to **All Fields** along with the other filter defaults. --- ### User Story 3 - Everything that is not Content Drive is untouched (Priority: P3) -A developer using the Asset Picker, and an integration calling the Content Drive search endpoint, -see no change at all. The Asset Picker's search box keeps the single all-content behavior it has -today, with no scope control on screen. A request that does not mention the scope behaves exactly as -it does now. +A developer using the **Asset Picker**, and an integration calling the Content Drive search +endpoint, see no change at all. The Asset Picker's search box keeps the single all-fields behavior +it has today, with no search scope control on screen. A request that does not mention the search +scope behaves exactly as it does now. **Why this priority**: It is a constraint on Stories 1 and 2 rather than a journey of its own, and it is verified by absence. It still has to be stated and tested, because the search box is shared and the endpoint is public. -**Independent Test**: Fully testable by opening the Asset Picker and confirming no scope control -appears and search behaves as before, and by replaying a stored Content Drive search request with no -scope field and comparing the results to the current ones. +**Independent Test**: Fully testable by opening the Asset Picker and confirming no search scope +control appears and search behaves as before, and by replaying a stored Content Drive search request +with no `searchScope` field and comparing the results to the current ones. **Acceptance Scenarios**: 1. **Given** the Asset Picker is open, **When** the author looks at its search box, **Then** no - scope control is present and searching behaves exactly as it does today. -2. **Given** a Content Drive search request that omits the scope entirely, **When** it is processed, - **Then** the results are identical to today's all-content results. -3. **Given** a Content Drive search request that names the all-content scope explicitly, **When** it + search scope control is present and searching behaves exactly as it does today. +2. **Given** a Content Drive search request that omits the search scope entirely, **When** it is + processed, **Then** the results are identical to today's all-fields results. +3. **Given** a Content Drive search request that names the all-fields scope explicitly, **When** it is processed, **Then** the results are identical to the request that omits it. -4. **Given** a Content Drive search request naming a scope value the system does not recognize, - **When** it is processed, **Then** it is rejected with a client error that names the offending - value, rather than silently widening or narrowing the results. +4. **Given** a Content Drive search request naming a search scope value the system does not + recognize, **When** it is processed, **Then** it is rejected with a client error that names the + offending value, rather than silently widening or narrowing the results. +5. **Given** any of the other entry points that reach the same content listing — the assets REST + API, the legacy admin file browser, the Velocity macro viewtool — **When** they list content + after this change, **Then** they return exactly what they returned before it. --- ### Edge Cases -- **Scope changed with an empty term.** No search is narrowed and nothing is re-fetched beyond the - drive's normal unfiltered listing; the control still records the new scope so the next term uses - it, and the placeholder updates. -- **Scope changed while a search is in flight.** The later request is the one whose results are - shown; an earlier in-flight response never overwrites it. +- **Search scope changed with an empty term.** No search is narrowed and nothing is re-fetched + beyond the drive's normal unfiltered listing; the control still records the new scope so the next + term uses it, and the placeholder updates. +- **Search scope changed while a search is in flight.** The later request is the one whose results + are shown; an earlier in-flight response never overwrites it. - **Term matches only folder or link names, in Title scope.** Those rows are listed — folder and link matching is scope-independent ([Premise Correction 2](#2-folders-and-links-are-already-matched-on-name-only-in-both-scopes)). - **Multi-word term in Title scope.** The term narrows rather than widens: a row must be a name match for the phrase as entered, not merely for one of its words. - **Term containing characters the query syntax treats specially.** Handled the same way in both scopes; a term is never allowed to alter the structure of the query. -- **Scope combined with the other Content Drive filters** — content type, language, status, - workflow, shared assets, per-field filters. The scope narrows the text match only; every other - filter keeps applying as it does today, and combining them narrows further rather than +- **Search scope combined with the other Content Drive filters** — content type, language, status, + workflow, shared assets, per-field filters. The search scope narrows the text match only; every + other filter keeps applying as it does today, and combining them narrows further rather than conflicting. -- **Scope selected while a folder is selected in the tree.** A new search already resets the folder - scope to the site root; changing the scope of an existing search behaves consistently with that. -- **Scope set explicitly back to All Content.** The drive returns to the state it would have had if - the control had never been touched: nothing recorded in the address, and no "clear all filters" - offered on an otherwise unfiltered drive. +- **Search scope combined with a browse scope** (#37426, in flight). The two are independent: a + browse scope says which slice of content is being listed, a search scope says which fields the + term is read against within it. Neither may change the other's answer. +- **Search scope selected while a folder is selected in the tree.** A new search already resets the + folder scope to the site root; changing the search scope of an existing search behaves + consistently with that. +- **Search scope set explicitly back to All Fields.** The drive returns to the state it would have + had if the control had never been touched: nothing recorded in the address, and no "clear all + filters" offered on an otherwise unfiltered drive. - **A file whose title was edited to something other than its file name**, searched by file name in Title scope: it does not match. See [Assumptions](#assumptions). +- **An item saved or renamed moments before the search, not yet indexed.** It is missing from the + results in **both** scopes, exactly as it is today — Title scope does not make this worse. See + [ADR-0018 alignment](#adr-0018-alignment). ## Requirements *(mandatory)* @@ -261,65 +339,96 @@ scope field and comparing the results to the current ones. **The control** -- **FR-001**: The Content Drive search box MUST present a scope control adjacent to the search - input, within the same visual container, offering exactly two options: **Title** and - **All Content**. -- **FR-002**: The control MUST display the active scope as its label, and MUST mark the active - option with a check when opened. -- **FR-003**: The search input's placeholder MUST describe the active scope, so the box states what - it will do before the author types. -- **FR-004**: Selecting a scope MUST re-run the current search immediately, without requiring the - author to retype or re-submit the term. -- **FR-005**: Selecting a scope MUST reset pagination to the first page. -- **FR-006**: Re-selecting the already-active scope MUST NOT trigger a new search. -- **FR-007**: The scope control MUST be reachable and operable by keyboard and MUST expose its - current selection to assistive technology. +- **FR-001**: The Content Drive search box MUST present a search scope control adjacent to the + search input, within the same visual container, offering exactly two options: **Title** and + **All Fields**. +- **FR-002**: The control MUST display the active search scope as its label, and MUST mark the + active option with a check when opened. +- **FR-003**: The search input's placeholder MUST describe the active search scope, so the box + states what it will do before the author types. +- **FR-004**: Selecting a search scope MUST re-run the current search immediately, without requiring + the author to retype or re-submit the term. +- **FR-005**: Selecting a search scope MUST reset pagination to the first page. +- **FR-006**: Re-selecting the already-active search scope MUST NOT trigger a new search. +- **FR-007**: The search scope control MUST be reachable and operable by keyboard and MUST expose + its current selection to assistive technology. **Behavior** -- **FR-008**: In **Title** scope, a contentlet MUST be returned only when its title matches the - term. A contentlet whose term occurrence is confined to body copy, Story Block content or +- **FR-008**: In **Title** search scope, a contentlet MUST be returned only when its title matches + the term. A contentlet whose term occurrence is confined to body copy, Story Block content or metadata MUST NOT be returned. -- **FR-009**: In **All Content** scope, results MUST be identical to what Content Drive search +- **FR-009**: In **All Fields** search scope, results MUST be identical to what Content Drive search returns today for the same term and filters — no regression of any kind. -- **FR-010**: In **Title** scope, the query MUST NOT use an all-fields aggregate clause, and MUST - NOT use a leading-wildcard term as its mandatory gate. This is the requirement that makes the - scope a genuine fast path rather than a display filter. -- **FR-011**: Folder and link name matching MUST behave identically in both scopes. -- **FR-012**: The active sort MUST be unaffected by the scope; both scopes MUST apply the sort the - author has chosen, with the existing default. -- **FR-013**: The scope MUST compose with every other Content Drive filter without altering their - behavior. +- **FR-010**: In **Title** search scope, the query MUST NOT use an all-fields aggregate clause, and + MUST NOT use a leading-wildcard term as its mandatory gate. This is the requirement that makes the + search scope a genuine fast path rather than a display filter. +- **FR-011**: Folder and link name matching MUST behave identically in both search scopes. +- **FR-012**: The active sort MUST be unaffected by the search scope; both scopes MUST apply the + sort the author has chosen, with the existing default. +- **FR-013**: The search scope MUST compose with every other Content Drive filter without altering + their behavior. **State and contract** -- **FR-014**: A non-default scope MUST be encoded in the address alongside the other Content Drive - filters, and MUST be restored from it on reload and on browser Back/Forward. -- **FR-015**: An absent or unrecognized scope in the address MUST resolve to **All Content**, +- **FR-014**: A non-default search scope MUST be encoded in the address alongside the other Content + Drive filters, and MUST be restored from it on reload and on browser Back/Forward. +- **FR-015**: An absent or unrecognized search scope in the address MUST resolve to **All Fields**, without surfacing an error. -- **FR-016**: The scope MUST NOT be persisted as a per-user preference; a clean entry into Content - Drive MUST start at **All Content**. -- **FR-017**: The Content Drive search request MUST carry the scope as an optional field that - defaults to all-content behavior, so a request that omits it is processed exactly as it is today. -- **FR-018**: A request naming an unrecognized scope value MUST be rejected with a client error - identifying the value, rather than silently defaulting. -- **FR-019**: The shared search box MUST expose the scope control as opt-in. Surfaces that do not - opt in — the Asset Picker today — MUST render and behave exactly as they do now. -- **FR-020**: Clearing all filters MUST return the scope to **All Content**. -- **FR-021**: The scope MUST count as filter state only while it is not the default. Selecting - **All Content** MUST leave the drive in the state it would have been in had the control never been - touched — in particular, it MUST NOT cause a "clear all filters" affordance to be offered on a - drive that is otherwise unfiltered. +- **FR-016**: The search scope MUST NOT be persisted as a per-user preference; a clean entry into + Content Drive MUST start at **All Fields**. +- **FR-017**: The Content Drive search request MUST carry the search scope as an optional field that + defaults to all-fields behavior, so a request that omits it is processed exactly as it is today. + **The Asset Picker is the caller this protects**: it builds its own Content Drive search request + (`with-asset-browse.feature.ts` → `DotContentDriveService.search()`, the same `POST /drive/search` + Content Drive uses) and will never name a search scope. Any future change to the default has to + confront the Asset Picker by name, not merely re-run Content Drive's tests. +- **FR-018**: A request naming an unrecognized search scope value MUST be rejected with a client + error identifying the value, rather than silently defaulting. +- **FR-019**: The shared search box MUST expose the search scope control as opt-in. Surfaces that do + not opt in — the Asset Picker today — MUST render and behave exactly as they do now. +- **FR-020**: Clearing all filters MUST return the search scope to **All Fields**. +- **FR-021**: The search scope MUST count as filter state only while it is not the default. + Selecting **All Fields** MUST leave the drive in the state it would have been in had the control + never been touched — in particular, it MUST NOT cause a "clear all filters" affordance to be + offered on a drive that is otherwise unfiltered. + +**Naming, blast radius and forward compatibility** + +- **FR-022**: The control MUST make available a short explanation of what each option matches, + distinguishing the item's name from anything written anywhere in the item. The two labels alone + MUST NOT be relied on to convey the distinction. +- **FR-023**: The concept MUST be called **search scope** wherever it is named — in the UI copy, in + the address, in the request, and in the code. The request field MUST be `searchScope` and MUST sit + **inside the existing `filters` object**, beside `text`, whose meaning it qualifies and without + which it means nothing. Its values MUST be `TITLE` and `ALL_FIELDS`. Bare "scope" MUST NOT be used + for it, because Content Drive is concurrently gaining a **browse** scope (#37426) that is a + different thing in the same request. +- **FR-024**: The change MUST stay within the text-search branch of the shared content-listing + service. Callers that reach that listing by another door — the assets REST API + (`WebAssetHelper`), the legacy admin file browser (`BrowserAjax`), the Velocity macro viewtool + (`DotCMSMacroWebAPI`) — MUST produce exactly the results they produce today. Widening the change + into shared query building would widen the blast radius from two callers to six. +- **FR-025**: A request carrying a search scope with no text MUST be treated as the contract error + it is, rather than silently ignored — the field qualifies `text` and has no meaning without it. +- **FR-026**: **Title** search scope MUST NOT foreclose ADR-0018's `DB-title ∪ index` routing. When + the deferred title-persistence work lands, adding the DB title column to the Title path MUST be an + additive change to this feature, not a rewrite of it. This feature does not implement that union + and does not widen the index-lag gap that exists today + ([ADR-0018 alignment](#adr-0018-alignment)). ### Key Entities -- **Search scope**: Which part of a document a Content Drive search term is matched against. Two - values — *Title* and *All Content* — with *All Content* as the default. Lives alongside the search +- **Search scope**: Which fields of a document a Content Drive search term is matched against. Two + values — *Title* and *All Fields* — with *All Fields* as the default. Lives alongside the search term as part of the drive's filter state, is carried in the address, and is sent with the search - request. + request as `filters.searchScope`. Named in full throughout: Content Drive is separately gaining a + **browse scope** (#37426) that says *where* you are browsing. The two are independent — a browse + scope says where you are, a search scope says how a search reads what is there. - **Content Drive search request**: The existing description of what the drive should list — - location, term, content types, languages, status, workflow, per-field criteria, sort, paging. - Gains the scope as one more optional element of the text-matching part. + location, term, content types, languages, status, workflow, per-field criteria, sort, paging. Its + `filters` object today holds `text` and `filterFolders`, both of which qualify the text search; + the search scope joins them as a third, equally text-dependent member. ## Success Criteria *(mandatory)* @@ -328,40 +437,57 @@ scope field and comparing the results to the current ones. - **SC-001**: On a drive containing a document whose body mentions the search term and a document whose name is the search term, an author in **Title** scope sees only the second — verified as a binary pass on a seeded dataset. -- **SC-002**: For any term and filter combination, **All Content** results are byte-identical to +- **SC-002**: For any term and filter combination, **All Fields** results are byte-identical to the results the same drive returns before this change — zero regressions across the search cases covered by the endpoint's test suite. - **SC-003**: On a large dataset, a **Title** search returns its first page faster than the same - term in **All Content** scope, and the before/after comparison is recorded on the issue. The + term in **All Fields** scope, and the before/after comparison is recorded on the issue. The target is a measurable reduction, not a fixed threshold; the comparison itself is the deliverable the issue asks for. -- **SC-004**: A Content Drive address carrying a scope reproduces the same narrowed view for a - second user 100% of the time, and reload and Back/Forward preserve the scope in 100% of attempts. -- **SC-005**: 100% of search requests that omit the scope produce today's results — confirmed by an - explicit endpoint test for the omitted field, not only by the explicit all-content case. -- **SC-006**: The Asset Picker's search box shows no scope control and its search behavior is +- **SC-004**: A Content Drive address carrying a search scope reproduces the same narrowed view for + a second user 100% of the time, and reload and Back/Forward preserve the search scope in 100% of + attempts. +- **SC-005**: 100% of search requests that omit `filters.searchScope` produce today's results — + confirmed by an explicit endpoint test for the omitted field, not only by the explicit all-fields + case. +- **SC-006**: The Asset Picker's search box shows no search scope control and its search behavior is unchanged, confirmed by its existing tests passing without modification. - **SC-007**: An author who knows the name of the item they want reaches it from the search box - without scrolling past unrelated body matches, on a drive where the all-content search for the + without scrolling past unrelated body matches, on a drive where the all-fields search for the same term returns more than one page. +- **SC-008**: Every caller of the shared content listing is accounted for by name rather than by a + blanket claim, and the three that are not Content Drive or the Asset Picker return identical + results before and after the change. ## Legacy Considerations *(dotCMS-specific — mandatory)* - **Existing behavior touched**: Content Drive's keyword search, and the browsing service that backs it. The browsing service is long-standing, pre-Content-Drive product surface shared with other file-browsing entry points; Content Drive's search endpoint and its front end are recent. - The shared search box is also used by the Asset Picker, which is explicitly not in scope. -- **Backward-compatibility expectations**: Strict. All-content search must be unchanged for every - existing caller and every existing address; the scope is additive and optional at every layer, and - its absence must be indistinguishable from today. No existing behavior is deprecated. The - all-content query strategy is shared with the Search portlet and the Relationships dialog and must - keep serving them unmodified — the Title scope is a sibling path, not a branch inside the existing - one. + The shared search box is also used by the Asset Picker, which is explicitly not in scope. Two + callers reach the Content Drive search endpoint (Content Drive and the Asset Picker); three more + reach the same underlying listing by other doors (FR-024). +- **Backward-compatibility expectations**: Strict. All-fields search must be unchanged for every + existing caller and every existing address; the search scope is additive and optional at every + layer, and its absence must be indistinguishable from today. No existing behavior is deprecated. + The all-fields query strategy is shared with the Search portlet and the Relationships dialog and + must keep serving them unmodified — the Title scope is a sibling path, not a branch inside the + existing one. - **Known related decisions**: [#36688](https://github.com/dotCMS/core/issues/36688) deliberately replaced a broad leading-wildcard all-fields query with the current strategy, for the same cost reasons argued here — the Title scope must not reintroduce what that issue removed. [#36814](https://github.com/dotCMS/core/issues/36814) tracks search performance at scale and is - where SC-003's measurement belongs. `/speckit-plan` will formally consult `dotCMS/platform-adrs`. + where SC-003's measurement belongs. **ADR-0018** (Database-First Content Drive Search) is the one + ADR this feature touches; its `DB ∪ Index` title routing is addressed explicitly in + [ADR-0018 alignment](#adr-0018-alignment) and deferred with reasons, not by omission. + [#37426](https://github.com/dotCMS/core/issues/37426) lands a **browse** scope on the same request + object; the naming split in FR-023 is agreed between the two specs. +- **Contract debt deliberately not taken on**: the Content Drive request also lets a caller say the + same thing twice (`archived: true` alongside `status: ["ARCHIVED"]`, reconciled in + `ContentDriveHelper`), and carries `live` as a boolean that selects a *version* rather than + filtering anything; six fields that genuinely are filter-bar chips sit at the top level rather + than in `filters`. That cleanup is worth doing while the endpoint still has two callers, but it is + a contract change of its own and neither this feature nor #37426 should carry it. ## Assumptions @@ -373,19 +499,24 @@ scope field and comparing the results to the current ones. other than its file name will not match on the file name. If that gap proves to matter in practice, widening Title scope to cover file names is a follow-up with its own spec, not a silent change here. +- **Read-your-writes for text search stays where ADR-0018 put it.** The `contentlet.title` column + is not reliably populated today and the ADR defers the work that would make it so. This feature + assumes it will remain unpopulated for the life of this implementation, and that Title scope's + index-only matching is therefore no worse than the all-fields search it sits beside. - **The issue's ASCII diagram is the whole design input.** No mock image or design file exists for this control, and none is being waited on. The spec fixes which components are present and how they behave; their appearance is settled during implementation. -- **The scope is a front-end-visible concept only for Content Drive.** No other portlet gains it in - this work, and the query strategy shared with the Search portlet and the Relationships dialog is - left as-is. +- **The search scope is a front-end-visible concept only for Content Drive.** No other portlet gains + it in this work, and the query strategy shared with the Search portlet and the Relationships + dialog is left as-is. - **"Title" is the label authors understand**, including for folders and files, and does not need to - read differently per row type. -- **The existing address-encoding scheme for Content Drive filters can carry the scope** without a - new mechanism, and an unknown value degrades to the default the same way an unknown status does - today. + read differently per row type. **"All Fields"** is understood with the help of FR-022's + explanation rather than on its own. +- **The existing address-encoding scheme for Content Drive filters can carry the search scope** + without a new mechanism, and an unknown value degrades to the default the same way an unknown + status does today. - **The measurement in SC-003 needs a dataset large enough for the difference to exceed noise.** The issue does not name one; producing it is part of the work, and the comparison is recorded on the issue rather than in the repository. -- **No database, index-mapping or content-model change is required.** The scope selects between two - ways of querying what is already indexed. +- **No database, index-mapping or content-model change is required.** The search scope selects + between two ways of querying what is already indexed. From 1ce8cdd1bfa6f6acf1341ede092c7c3579a8b112 Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 14 Sep 2026 11:51:27 -0500 Subject: [PATCH 03/33] =?UTF-8?q?spec(content-drive):=20fold=20#37532=20in?= =?UTF-8?q?to=20the=20spec=20=E2=80=94=20literal-text=20search=20terms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #37532 (High, customer ticket 39185) reports that content type field filters in the Content Search portlet return "no results found" for values containing Lucene query-syntax characters. Verified that the same defect lives in Content Drive's own search box, in the exact clause FR-010 rewrites, so the two issues are resolved together. Premise Correction 6 records what GlobalSearchAttributeStrategy actually does. Escaping is applied to its final clause only (:46-48); the mandatory gate at :37-38 is built from the raw value, so a term like "ABC (XETRA: DB)" yields invalid query_string syntax. Its private SPECIAL_CHARS_TO_ESCAPE regex (:20) is missing "/", which is #37532's fifth acceptance criterion verbatim, while LuceneQueryUtils.escape — vendor-neutral, documented, already used by TextFieldStrategy — covers the full reserved set. The split at :40-45 has no empty-token filter, so consecutive separators emit a term-less title:^5 clause. And a query that fails to parse is swallowed at BrowserAPIImpl:893-895, returning an empty set, which is why the user is told their content does not exist. Nothing sanitizes upstream: ContentDriveHelper:183 passes text() raw. The parallel field-filter path is already correct — TextFieldStrategy escapes and filters empty tokens — so #37532's Content Drive field-filter criterion is verification, not implementation. FR-030 and SC-012 pin it. FR-009 and SC-002 had to yield. Escaping the gate changes All Fields results for affected terms, which contradicts the no-regression promise as approved, so the carve-out is written down and bounded rather than smuggled in: reserved characters and consecutive separators only, every other term unchanged, and the tests that change enumerated in the PR. Adds User Story 4 (P1, listed fourth), FR-027 to FR-031 and SC-009 to SC-012. FR-031 keeps the fix in the shared strategy rather than a Content-Drive-only branch, so the Search portlet and the Relationships dialog stop mis-parsing reserved characters too. What this deliberately does not do is stated in Legacy Considerations and in "Why #37532 lands here": the Content Search portlet keeps its current behaviour. #37532 itself directs the improved behaviour to Content Drive rather than to the legacy construction, but the customer on ticket 39185 is using the portlet today, so closing #37532 on this work is the issue owner's call and is flagged rather than assumed. Refs #37479, #37532 Co-Authored-By: Claude Opus 5 (1M context) --- .../37479-content-drive-search-scope/spec.md | 228 ++++++++++++++++-- 1 file changed, 214 insertions(+), 14 deletions(-) diff --git a/specs/37479-content-drive-search-scope/spec.md b/specs/37479-content-drive-search-scope/spec.md index f19cf7308c7e..b28f08e23a9e 100644 --- a/specs/37479-content-drive-search-scope/spec.md +++ b/specs/37479-content-drive-search-scope/spec.md @@ -1,18 +1,27 @@ -# Feature Specification: Content Drive — Title / All Fields search scope selector for the search box +# Feature Specification: Content Drive — Title / All Fields search scope, with literal-text search terms **Feature Branch**: `issue-37479-content-drive-search-scope` **Created**: 2026-09-11 -**Last revised**: 2026-09-13 — review round 1 (see [Review Decisions](#review-decisions-settled-2026-09-13)) +**Last revised**: 2026-09-14 — review round 1, then merged with #37532 (see +[Review Decisions](#review-decisions-settled-2026-09-13) and +[Merge of #37532](#merge-of-37532-settled-2026-09-14)) **Status**: Draft **Type**: New Feature -**Related GitHub Issue**: [#37479](https://github.com/dotCMS/core/issues/37479) — related history: [#36688](https://github.com/dotCMS/core/issues/36688) (replaced the broad `catchall:*kw*` wildcard with the current strategy), [#36814](https://github.com/dotCMS/core/issues/36814) (search performance at scale). Sibling in flight: [#37426](https://github.com/dotCMS/core/issues/37426) / [PR #37487](https://github.com/dotCMS/core/pull/37487) (Content Drive **browse** scopes) — the two features land fields on the same request object and are deliberately named apart; see [Review Decision 5](#review-decisions-settled-2026-09-13). +**Resolves**: [#37479](https://github.com/dotCMS/core/issues/37479) (search scope selector) **and** +[#37532](https://github.com/dotCMS/core/issues/37532) (Lucene query-syntax characters in search +terms — High severity, customer ticket +[39185](https://helpdesk.dotcms.com/a/tickets/39185)). The two were specified separately and merged +on 2026-09-14 once #37532's defect was found to live in the very clause this feature rewrites — see +[Why #37532 lands here](#why-37532-lands-here). -**Input**: User description: "Content Drive: add a Title / All Content scope selector to the search box. Default scope = All Content (no regression); Title mode matches strictly the contentlet title plus folder names, not fileName/metadata.name; scope persists only in the URL filters; sorting unchanged." +**Related history**: [#36688](https://github.com/dotCMS/core/issues/36688) (replaced the broad `catchall:*kw*` wildcard with the current strategy), [#36814](https://github.com/dotCMS/core/issues/36814) (search performance at scale). Sibling in flight: [#37426](https://github.com/dotCMS/core/issues/37426) / [PR #37487](https://github.com/dotCMS/core/pull/37487) (Content Drive **browse** scopes) — the two features land fields on the same request object and are deliberately named apart; see [Review Decision 5](#review-decisions-settled-2026-09-13). + +**Input (#37479)**: User description: "Content Drive: add a Title / All Content scope selector to the search box. Default scope = All Content (no regression); Title mode matches strictly the contentlet title plus folder names, not fileName/metadata.name; scope persists only in the URL filters; sorting unchanged." > The issue's words are recorded verbatim above. The wide option is named **All Fields** in this spec — see [Review Decision 7](#review-decisions-settled-2026-09-13) for why "All Content" was rejected. @@ -20,9 +29,10 @@ ## Premise Corrections -Five things the issue takes for granted do not survive verification against `main`. Three of them -narrow the work; the fourth adds a rule the issue's file list has no place for; the fifth settles -which query path the scope actually governs. +Six things do not survive verification against `main`. Three of them narrow the work; the fourth +adds a rule the issue's file list has no place for; the fifth settles which query path the search +scope actually governs; the sixth is a live customer defect sitting in the clause this feature +rewrites. ### 1. Results are **not** sorted by score when a term is present @@ -103,6 +113,81 @@ clause alone. There is no second, SQL-shaped text match that a Title scope could narrow. This is also what settles the ADR-0018 question — see [ADR-0018 alignment](#adr-0018-alignment). +### 6. The search box does not treat the term as literal text today + +The edge case "a term is never allowed to alter the structure of the query" was written into this +spec as a property to preserve. It is not a property the search box has. `GlobalSearchAttributeStrategy` +escapes **only its final clause**: + +```java +// :37-38 — the MANDATORY gate, built from the RAW value +luceneQuery.append("+(catchall:").append(value).append("*^10 OR ") + .append(fieldName).append("_dotraw:*").append(value).append("*^2)"); +// :46-48 — escaping happens here, and applies to this clause alone +value = value.replaceAll(SPECIAL_CHARS_TO_ESCAPE, "\\\\$1"); +luceneQuery.append("title:").append(value).append("*"); +``` + +Three distinct defects follow, and nothing sanitizes the value upstream — `ContentDriveHelper:183` +passes `filters().text()` straight into `withFilter`, which reaches the strategy raw: + +1. **The gate is built from unescaped input.** A term such as `ABC (XETRA: DB)` produces + `+(catchall:ABC (XETRA: DB)*^10 OR ...)`, which is not valid `query_string` syntax. +2. **The escape set is incomplete.** `SPECIAL_CHARS_TO_ESCAPE` (`:20`) is a private regex missing + `/`, while `LuceneQueryUtils.LUCENE_SPECIAL_CHARS` — vendor-neutral, documented, already used by + `TextFieldStrategy` — covers the full reserved set including `/`. +3. **Consecutive separators emit empty clauses.** `:40-45` splits on `[,|\s+]` with no empty-token + filter, so `"a b"` yields a `title:^5` clause with no term. + +A query that fails to parse is then **swallowed**: `BrowserAPIImpl:893-895` logs and returns an +empty set, and the multi-query path does the same at every level. The user sees "no results" for +content they know exists — the exact symptom of customer ticket 39185. + +**Consequence for this feature**: FR-027 to FR-030, and a carve-out in FR-009/SC-002, because +fixing this **changes All Fields results** for affected terms. The parallel path is already correct +and is the model to copy: Content Drive's *field* filters go through `TextFieldStrategy:45-46,53-54`, +which escapes via `LuceneQueryUtils.escape` and filters empty tokens. + +--- + +## Why #37532 lands here + +[#37532](https://github.com/dotCMS/core/issues/37532) was raised against the **Content Search +portlet**, where `ContentletAjax.searchContentletsByUser()` builds field-filter clauses without +escaping and swallows the resulting parse failure at `ContentletAjax.java:1058`. It is merged into +this spec rather than kept separate for two reasons. + +**The issue asks for it.** It is explicitly filed as an enhancement rather than a defect fix, and +says so in its own words: the legacy field-level query generation should be reworked rather than +patched, and *"Content Drive is intended to replace the Content Search portlet, so the improved +field-level search behaviour should land there."* This spec is that landing. + +**The defect is already here.** Premise Correction 6 shows all three failure modes — unescaped +input, incomplete escape set, empty clauses — plus the silent swallow, living in Content Drive's +own search box. Fixing #37479 without them would mean rewriting the exact clause that carries the +bug and leaving the bug in it. + +**What this does NOT do, and it matters**: the Content Search portlet keeps its current behavior. +A user filtering on a content type field in that portlet still gets a silent empty result set for a +value containing `:` or `(`. Closing #37532 on this work is only honest if the team accepts that +trade — it is the direction the issue itself sets, but the customer on ticket 39185 is using the +portlet today, not Content Drive. If that is not acceptable, the portlet needs its own fix and +#37532 should stay open against it. Flagged rather than assumed. + +Scope split, explicitly: + +| #37532 acceptance criterion | Where it lands | +|---|---| +| Values treated as literal text, not query syntax | **Here** — FR-027 (search box). Already satisfied for Content Drive field filters. | +| Full reserved set `\ + - ! ( ) : ^ [ ] " { } ~ * ? \| & /` verified | **Here** — SC-010 | +| Consecutive whitespace no longer emits `**` clauses | **Here** — FR-028 | +| Failed query surfaces an error instead of a silent zero result | **Here** — FR-029 | +| `/` added to the escape set of the global catchall path | **Here** — FR-027, by adopting `LuceneQueryUtils.escape` | +| Regression test covering the reported headline value | **Here** — SC-009 | +| Equivalent field-filter behaviour confirmed in Content Drive | **Here** — FR-030, as verification; the code already does it | +| Rework of `ContentletAjax` field-filter construction | **Not here** — legacy portlet, slated for replacement | +| Error state in the Content Search portlet's own UI | **Not here** — same reason | + --- ## Decisions (settled 2026-09-11) @@ -130,6 +215,13 @@ They are settled here and are part of what PR 1's approval signs off. | 7 | The label of the wide option | **All Fields** (values `TITLE` / `ALL_FIELDS`) | "All Content" describes a *set of content*, which is what the browse scope's **All** genuinely means. This scope does not widen which content is searched — it widens which **fields** of each document the term is read against. Naming it "All Content" would have put two different meanings of the same word on the same request. | | 8 | ADR-0018's `DB ∪ Index` title routing | **Out of scope, explicitly** — Title scope inherits today's index-only routing | See [ADR-0018 alignment](#adr-0018-alignment). The union is gated on title-persistence work the ADR itself defers, and no text search consults the DB title column today. Title scope neither creates nor widens that gap, and it picks the union up for free when the gated work lands. | +### Merge of #37532 (settled 2026-09-14) + +| # | Decision | Settled as | Why | +|---|---|---|---| +| 9 | Whether #37532 (literal-text search terms) joins this spec | **Merged in**, for the search box only | The defect lives in the clause FR-010 rewrites, and #37532 itself directs the improved behaviour to land in Content Drive. Keeping them apart would mean rewriting the buggy clause and leaving the bug in it. See [Why #37532 lands here](#why-37532-lands-here). | +| 10 | FR-009's "no regression" promise vs. fixing the defect | **FR-009 yields** — a carve-out, written down | Escaping the gate *changes* All Fields results for terms containing reserved characters. That is the point, but it contradicts FR-009/SC-002 as originally approved, so the exception is stated rather than smuggled in. | + ## ADR-0018 alignment [ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md) @@ -301,6 +393,42 @@ with no `searchScope` field and comparing the results to the current ones. --- +### User Story 4 - A name that contains punctuation is found (Priority: P1) + +An author searches for `ABC Bank (XETRA: DBKGn.DB / NYSE: DB)` — the headline of a piece of content +they are looking at in another tab. Today the drive reports "no results found". The colons, +parentheses and slash are read as query syntax rather than as part of the name, the query fails to +parse, the failure is logged and discarded, and the author is told their content does not exist. +After this change the term is matched as the literal text it is, and the item is returned — in both +search scopes. + +**Why this priority**: It is listed fourth but ranks P1. This is the live customer defect behind +[#37532](https://github.com/dotCMS/core/issues/37532) (High severity, ticket 39185), and it is a +silent wrong answer rather than a missing capability — the worst failure mode a search box has, +because the author has no way to tell a broken query from an empty drive. Story 1 builds on the same +clause, so the two are implemented together. + +**Independent Test**: Fully testable by seeding content whose title carries each character in the +reserved set, searching for it verbatim, and confirming it is returned. Delivers the fix on its own, +with or without the search scope control on screen. + +**Acceptance Scenarios**: + +1. **Given** content whose title is `ABC Bank (XETRA: DBKGn.DB / NYSE: DB)`, **When** the author + pastes that title into the search box in **All Fields** scope, **Then** the item is returned. +2. **Given** that same content, **When** the author searches for it in **Title** scope, **Then** the + item is returned — the fix applies to the new clause as well as the existing one. +3. **Given** a term containing any character of the reserved set + `\ + - ! ( ) : ^ [ ] " { } ~ * ? | & /`, **When** the search runs, **Then** the character is + matched as literal text and never alters the structure of the query. +4. **Given** a term containing consecutive spaces or separators, **When** the search runs, **Then** + no empty clause is emitted and the results are the same as for the single-separator form. +5. **Given** a search request that nonetheless fails to execute, **When** the drive renders the + response, **Then** the author is shown an error state, **not** an empty result list presented as + a successful search. + +--- + ### Edge Cases - **Search scope changed with an empty term.** No search is narrowed and nothing is re-fetched @@ -312,8 +440,12 @@ with no `searchScope` field and comparing the results to the current ones. link matching is scope-independent ([Premise Correction 2](#2-folders-and-links-are-already-matched-on-name-only-in-both-scopes)). - **Multi-word term in Title scope.** The term narrows rather than widens: a row must be a name match for the phrase as entered, not merely for one of its words. -- **Term containing characters the query syntax treats specially.** Handled the same way in both - scopes; a term is never allowed to alter the structure of the query. +- **Term containing characters the query syntax treats specially.** Matched as literal text in both + scopes; a term is never allowed to alter the structure of the query. This does **not** hold today + ([Premise Correction 6](#6-the-search-box-does-not-treat-the-term-as-literal-text-today)) and is + made true by FR-027. +- **Term that is only separators**, or that reduces to no usable token after splitting. It yields no + text clause at all rather than an empty one, and the drive lists as it would with no term. - **Search scope combined with the other Content Drive filters** — content type, language, status, workflow, shared assets, per-field filters. The search scope narrows the text match only; every other filter keeps applying as it does today, and combining them narrows further rather than @@ -359,7 +491,11 @@ with no `searchScope` field and comparing the results to the current ones. the term. A contentlet whose term occurrence is confined to body copy, Story Block content or metadata MUST NOT be returned. - **FR-009**: In **All Fields** search scope, results MUST be identical to what Content Drive search - returns today for the same term and filters — no regression of any kind. + returns today for the same term and filters — no regression of any kind. **One carve-out, and only + one**: terms containing Lucene reserved characters or consecutive separators, whose results change + by design under FR-027 and FR-028 because today's behaviour for them is a malformed query and a + silent empty result set. Every term outside that carve-out MUST be unchanged, and the carve-out + MUST NOT be used to justify any other difference. - **FR-010**: In **Title** search scope, the query MUST NOT use an all-fields aggregate clause, and MUST NOT use a leading-wildcard term as its mandatory gate. This is the requirement that makes the search scope a genuine fast path rather than a display filter. @@ -417,6 +553,29 @@ with no `searchScope` field and comparing the results to the current ones. and does not widen the index-lag gap that exists today ([ADR-0018 alignment](#adr-0018-alignment)). +**Literal-text search terms (#37532)** + +- **FR-027**: A search term MUST be matched as literal text. **Every** clause built from the term + MUST escape the full Lucene `query_string` reserved set — `\ + - ! ( ) : ^ [ ] " { } ~ * ? | & /` + — not only the last one, and the mandatory gate in particular. The escaping MUST use the shared, + vendor-neutral utility rather than a strategy-private character set, so the reserved set has one + definition and `/` is covered. Wildcards the system adds itself MUST be applied **after** escaping + so they are not themselves escaped. This applies to **both** search scopes: the Title clause + introduced by FR-010 is subject to it exactly as the existing all-fields clause is. +- **FR-028**: Consecutive separators in a term MUST NOT produce empty clauses. A term that reduces + to no usable token MUST produce no text clause at all, rather than a clause with an empty value. +- **FR-029**: A search request whose query fails to execute MUST surface an error state to the + author. It MUST NOT be reported as a successful search that found nothing. The failure MUST + remain logged, but logging MUST NOT be the only response to it. +- **FR-030**: Content Drive's per-field filters MUST match terms literally on the same reserved set + as FR-027, and this MUST be confirmed by test rather than by inspection. The implementation + already satisfies this; the requirement exists so a regression would be caught and so #37532's + corresponding criterion is demonstrably met. +- **FR-031**: The all-fields search behaviour MUST remain shared with the Search portlet and the + Relationships dialog. FR-027's escaping corrects a defect in that shared path and therefore + benefits them too; it MUST NOT be implemented as a Content-Drive-only branch that leaves the + shared strategy broken for its other consumers. + ### Key Entities - **Search scope**: Which fields of a document a Content Drive search term is matched against. Two @@ -437,9 +596,11 @@ with no `searchScope` field and comparing the results to the current ones. - **SC-001**: On a drive containing a document whose body mentions the search term and a document whose name is the search term, an author in **Title** scope sees only the second — verified as a binary pass on a seeded dataset. -- **SC-002**: For any term and filter combination, **All Fields** results are byte-identical to - the results the same drive returns before this change — zero regressions across the search cases - covered by the endpoint's test suite. +- **SC-002**: For any term and filter combination **that contains no Lucene reserved character and + no consecutive separator**, All Fields results are byte-identical to the results the same drive + returns before this change — zero regressions across the search cases covered by the endpoint's + test suite. Terms inside the FR-009 carve-out are measured by SC-009 and SC-010 instead, and the + set of tests that change is enumerated in the PR rather than left implicit. - **SC-003**: On a large dataset, a **Title** search returns its first page faster than the same term in **All Fields** scope, and the before/after comparison is recorded on the issue. The target is a measurable reduction, not a fixed threshold; the comparison itself is the deliverable @@ -458,6 +619,17 @@ with no `searchScope` field and comparing the results to the current ones. - **SC-008**: Every caller of the shared content listing is accounted for by name rather than by a blanket claim, and the three that are not Content Drive or the Asset Picker return identical results before and after the change. +- **SC-009**: Searching the exact headline reported on customer ticket 39185 — + `ABC Bank (XETRA: DBKGn.DB / NYSE: DB) and PSL Launch independent European CLO Total Return Indices` + — returns the item, in both search scopes, as a regression test that fails before the change and + passes after it. +- **SC-010**: Every character in the reserved set `\ + - ! ( ) : ^ [ ] " { } ~ * ? | & /` is + covered by a test that seeds a title containing it and finds that title by searching for it + verbatim — the full set, not a sample, in both search scopes. +- **SC-011**: A search whose query fails to execute produces a visible error state in 100% of + attempts, and zero of those attempts render as a successful empty result list. +- **SC-012**: Content Drive's per-field filters pass the same reserved-set coverage as SC-010, + confirming #37532's field-filter criterion by test rather than by inspection. ## Legacy Considerations *(dotCMS-specific — mandatory)* @@ -473,6 +645,20 @@ with no `searchScope` field and comparing the results to the current ones. The all-fields query strategy is shared with the Search portlet and the Relationships dialog and must keep serving them unmodified — the Title scope is a sibling path, not a branch inside the existing one. +- **Legacy surface deliberately left as-is**: the **Content Search portlet** keeps the behaviour + #37532 reports. Its `ContentletAjax.searchContentletsByUser()` field-filter branch + (`ContentletAjax.java:874-906`) still builds unescaped per-token clauses and still swallows the + parse failure at `:1058`. #37532 frames the legacy construction as something to replace rather + than patch, and names Content Drive as where the improved behaviour should land — so this spec + improves the replacement, not the thing being replaced. **Consequence to accept consciously**: a + user filtering in the Content Search portlet still sees a silent empty result set until Content + Drive replaces it. See [Why #37532 lands here](#why-37532-lands-here). +- **Shared-path defect, shared-path fix**: FR-027 corrects `GlobalSearchAttributeStrategy`, which + also serves the Search portlet and the Relationships dialog. Unlike the Title scope — a sibling + path deliberately kept out of the shared strategy — the escaping defect is *in* the shared + strategy and is fixed there, so every consumer stops mis-parsing reserved characters. This is the + one place where this feature intentionally changes behaviour beyond Content Drive, and FR-031 + states it so it is not mistaken for scope creep. - **Known related decisions**: [#36688](https://github.com/dotCMS/core/issues/36688) deliberately replaced a broad leading-wildcard all-fields query with the current strategy, for the same cost reasons argued here — the Title scope must not reintroduce what that issue removed. @@ -519,4 +705,18 @@ with no `searchScope` field and comparing the results to the current ones. issue does not name one; producing it is part of the work, and the comparison is recorded on the issue rather than in the repository. - **No database, index-mapping or content-model change is required.** The search scope selects - between two ways of querying what is already indexed. + between two ways of querying what is already indexed, and the escaping fix is a pure string + transform over the term before it reaches the index. +- **Escaping the reserved set does not degrade the matching #36688 tuned.** The characters are made + literal, not removed, so the mandatory catchall prefix gate and the boost structure keep the shape + that issue settled. Terms free of reserved characters produce a byte-identical query, which is + what makes SC-002's narrowed promise measurable. +- **A visible error state is reachable from where the failure happens.** FR-029 assumes the drive + can distinguish "query failed" from "nothing matched" and carry that to the UI. Today the + distinction is destroyed at `BrowserAPIImpl:893-895`, which returns an empty set for both; how the + signal is carried out is an implementation decision, but the two cases must stop being the same + value. +- **Closing #37532 on this work is the team's call, not this spec's.** The spec delivers every one + of its acceptance criteria that applies to Content Drive, and states plainly which two do not. + Whether that is enough to close the issue against the customer ticket is a decision for the issue + owner. From 012f5d8f925eb73109b1ea81da4cf60c4028781b Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 14 Sep 2026 13:54:25 -0500 Subject: [PATCH 04/33] fix(content-drive): match search terms as literal text, not query syntax MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the Content Drive half of #37532 (High, customer ticket 39185): filtering on a value containing ":", "(" or "/" returned "No results found" for content the user was looking at. GlobalSearchAttributeStrategy escaped only its final clause, so the mandatory gate — the clause that decides whether a document matches at all — was built from raw user input. A term like "ABC Bank (XETRA: DB)" produced a query Elasticsearch could not parse; the failure was logged and discarded, and the caller received an empty result indistinguishable from a genuine miss. Three defects, one cause: - The gate carried the raw term. Now the term is escaped once, up front, and every clause uses the escaped value. The "*" wildcards the strategy appends are added after escaping so they stay live. - The private SPECIAL_CHARS_TO_ESCAPE regex omitted "/" entirely. Replaced with LuceneQueryUtils.escape — the helper TextFieldStrategy already uses, so the two strategies no longer disagree about the reserved set, and a character walk rather than a regex, which ADR-0009 calls for ahead of the ES→OpenSearch move. - Consecutive separators emitted a term-less "title:^5" clause. Empty tokens are now dropped, matching TextFieldStrategy. A query that fails to execute is no longer reported as a search that found nothing — but only for callers that ask. BrowserQuery gains surfaceQueryFailures, off by default, and Content Drive is the only caller that opts in. The assets REST API, the legacy admin browser and the Velocity viewtool keep receiving today's empty result. The failure is still logged either way. Also lays the contract groundwork for #37479: SearchScope, and filters.searchScope defaulting to ALL_FIELDS so a request omitting it is processed exactly as before. Nothing reads the scope yet. Evidence, against real PostgreSQL and OpenSearch: - ContentDriveLiteralTextSearchTest 3/3. The ticket 39185 headline is found when searched verbatim, all 19 reserved characters are findable by their own text, and field filters match literally. The first two failed before this change — that Red is what proved the defect reported against the Content Search portlet also reaches Content Drive. - 21 unit assertions green. GlobalSearchAttributeStrategyBaselineTest pins the boundary: two terms without reserved characters produce byte-identical queries, and the three that changed carry their before/after in-line. Those three are the complete list of pre-existing expectations this commit changes. The Content Search portlet's own ContentletAjax path is deliberately untouched — #37532 directs the improved behaviour to Content Drive rather than to the legacy construction. Refs #37532, #37479 Co-Authored-By: Claude Opus 5 (1M context) --- .../com/dotcms/browser/BrowserAPIImpl.java | 10 + .../java/com/dotcms/browser/BrowserQuery.java | 47 ++++ .../GlobalSearchAttributeStrategy.java | 32 ++- .../api/v1/drive/AbstractQueryFilters.java | 17 ++ .../rest/api/v1/drive/ContentDriveHelper.java | 13 + .../dotcms/rest/api/v1/drive/SearchScope.java | 33 +++ ...alSearchAttributeStrategyBaselineTest.java | 116 +++++++++ .../GlobalSearchAttributeStrategyTest.java | 153 ++++++++++++ .../src/test/java/com/dotcms/MainSuite3a.java | 2 + .../ContentDriveLiteralTextSearchTest.java | 235 ++++++++++++++++++ 10 files changed, 652 insertions(+), 6 deletions(-) create mode 100644 dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/SearchScope.java create mode 100644 dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyBaselineTest.java create mode 100644 dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyTest.java create mode 100644 dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveLiteralTextSearchTest.java diff --git a/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java b/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java index cec5b06df168..5c2bf7cf0ec8 100644 --- a/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java @@ -891,7 +891,17 @@ private Set processSingleESQuery(final BrowserQuery browserQuery, final inodes.size(), collectedInodes.size(), duration)); } catch (final Exception e) { + // Log first, always — this is the only record for callers that do not opt in. Logger.error(this, String.format("Single ES query failed for %d inodes: %s", inodes.size(), getErrorMessage(e)), e); + // Then, only if the caller asked for it, stop pretending the search found nothing. + // Collapsing "the query failed" into an empty result is what let a malformed query + // reach a user as "No results found" for content they were looking at (issue #37532). + // Off by default, so the assets REST API, the legacy admin browser and the Velocity + // viewtool keep behaving exactly as they do today. + if (browserQuery.surfaceQueryFailures) { + throw new DotRuntimeException( + "Content search query failed to execute: " + getErrorMessage(e), e); + } } return new LinkedHashSet<>(collectedInodes); diff --git a/dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java b/dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java index d18b7accf677..d6723f35fce1 100644 --- a/dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java +++ b/dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java @@ -5,6 +5,7 @@ import com.dotcms.contenttype.model.type.BaseContentType; import com.dotmarketing.beans.Host; import com.dotmarketing.beans.Identifier; +import com.dotcms.rest.api.v1.drive.SearchScope; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.Role; import com.dotmarketing.business.Theme; @@ -63,6 +64,8 @@ public class BrowserQuery { final boolean showDefaultLangItems; final boolean useElasticsearchFiltering; final boolean filterFolderNames; + final SearchScope searchScope; + final boolean surfaceQueryFailures; final Set languageIds; final String luceneQuery; final Set baseTypes; @@ -155,6 +158,8 @@ private BrowserQuery(final Builder builder) { final Tuple2 siteAndFolder = getParents(builder.hostFolderId,this.user, builder.hostIdSystemFolder); this.filter = builder.filter; this.useElasticsearchFiltering = builder.useElasticsearchFiltering; + this.searchScope = builder.searchScope; + this.surfaceQueryFailures = builder.surfaceQueryFailures; this.skipFolder = builder.skipFolder; this.ignoreSiteForFolders = builder.ignoreSiteForFolders; this.filterFolderNames = builder.filterFolderNames; @@ -291,6 +296,13 @@ public static final class Builder { private User user; private boolean useElasticsearchFiltering = false; private boolean filterFolderNames = false; + // Defaults to ALL_FIELDS so the callers that never set it — the assets REST API, the legacy + // admin browser, the Velocity viewtool and the File Asset API — keep producing exactly the + // results they produced before this field existed. + private SearchScope searchScope = SearchScope.ALL_FIELDS; + // Defaults to false so every existing caller keeps today's behavior exactly: a query that + // fails to execute is logged and yields an empty result. Only Content Drive opts in. + private boolean surfaceQueryFailures = false; private String filter = null; private String fileName = null; private String sortBy = "moddate"; @@ -333,6 +345,8 @@ private Builder(BrowserQuery browserQuery) { ? browserQuery.site.getIdentifier() : browserQuery.folder.getInode(); this.useElasticsearchFiltering = browserQuery.useElasticsearchFiltering; + this.searchScope = browserQuery.searchScope; + this.surfaceQueryFailures = browserQuery.surfaceQueryFailures; this.forceSystemHost = browserQuery.forceSystemHost; this.skipFolder = browserQuery.skipFolder; this.ignoreSiteForFolders = browserQuery.ignoreSiteForFolders; @@ -457,6 +471,39 @@ public Builder useElasticsearchFiltering(boolean useElasticsearchFiltering) { return this; } + /** + * Which fields the text filter is matched against. Only Content Drive sets this; every + * other caller leaves it at {@link SearchScope#ALL_FIELDS} and is therefore unaffected. + * + * @param searchScope the {@link SearchScope} + * @return this + */ + public Builder searchScope(final SearchScope searchScope) { + this.searchScope = null == searchScope ? SearchScope.ALL_FIELDS : searchScope; + return this; + } + + /** + * Whether a query that fails to execute should be surfaced to the caller instead of being + * reported as a search that found nothing. + * + *

Off by default, and deliberately so. "The query failed" and "nothing matched" have + * been the same empty result for every caller of this API; turning that into an error + * unconditionally would change behavior for the assets REST API, the legacy admin browser + * and the Velocity viewtool, none of which asked for it. Content Drive opts in because it + * has a user to tell — reporting a parse failure as "no results found" is what made a + * customer believe their content had vanished (issue #37532).

+ * + *

The failure is logged either way; this only controls whether it is also raised.

+ * + * @param surfaceQueryFailures flag + * @return this + */ + public Builder surfaceQueryFailures(final boolean surfaceQueryFailures) { + this.surfaceQueryFailures = surfaceQueryFailures; + return this; + } + /** * if we want to filter folder names when searching with Text filters * @param filterFolderNames flag diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategy.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategy.java index ddd6c7c11d46..cae9559c89c4 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategy.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategy.java @@ -1,6 +1,7 @@ package com.dotcms.rest.api.v1.content.search.strategies; import com.dotcms.rest.api.v1.content.search.handlers.FieldContext; +import com.dotmarketing.util.LuceneQueryUtils; /** * This Field Strategy implementation specifies the correct syntax for querying a Global Search @@ -17,12 +18,26 @@ public class GlobalSearchAttributeStrategy implements FieldStrategy { /** This is the RegEx used to split the values of the field into tokens */ private static final String VALUE_SPLIT_REGEX = "[,|\\s+]"; - private static final String SPECIAL_CHARS_TO_ESCAPE = "([+\\-!\\(\\){}\\[\\]^\"~*?:\\\\]|[&\\|]{2})"; @Override public String generateQuery(final FieldContext fieldContext) { final String fieldName = fieldContext.fieldName(); - String value = fieldContext.fieldValue().toString(); + final String rawValue = fieldContext.fieldValue().toString(); + // Escape ONCE, up front, and use the escaped value in EVERY clause below — including the + // mandatory gate. Previously only the trailing clause was escaped, so a term containing a + // reserved character (a colon, a parenthesis, a slash) produced a gate that Elasticsearch + // could not parse; the failure was logged and discarded and the user was told their content + // did not exist (issue #37532, customer ticket 39185). + // + // LuceneQueryUtils.escape replaces a private regex that was missing "/" entirely. It is the + // same helper TextFieldStrategy uses, so the two strategies no longer disagree about what + // the reserved set is, and it is a vendor-neutral character walk rather than a regex — + // which matters for the ES→OpenSearch migration (ADR-0009 flags Lucene 10 changes to + // special-character handling). + // + // The "*" wildcards appended below are syntax this strategy adds itself, so they are + // appended AFTER escaping and stay live rather than becoming literal asterisks. + final String value = LuceneQueryUtils.escape(rawValue); final StringBuilder luceneQuery = new StringBuilder(); // Mandatory gate: match either a catchall token PREFIX (fast, existing behavior) OR the // fieldName_dotraw raw value via wildcard. Unlike catchall (which aggregates every field @@ -37,14 +52,19 @@ public String generateQuery(final FieldContext fieldContext) { luceneQuery.append("+(catchall:").append(value).append("*^10 OR ") .append(fieldName).append("_dotraw:*").append(value).append("*^2)").append(" "); luceneQuery.append(fieldName).append(":'").append(value).append("'^15").append(" "); - final String[] titleSplit = value.split(VALUE_SPLIT_REGEX); + // Tokenize the RAW value so the split sees the user's real separators, then escape each + // token individually. Empty tokens are dropped: consecutive separators used to emit a + // term-less "title:^5" clause that could not parse (FR-028). + final String[] titleSplit = rawValue.split(VALUE_SPLIT_REGEX); if (titleSplit.length > 1) { for (final String term : titleSplit) { - luceneQuery.append(fieldName).append(":").append(term).append("^5").append(" "); + if (term.isEmpty()) { + continue; + } + luceneQuery.append(fieldName).append(":") + .append(LuceneQueryUtils.escape(term)).append("^5").append(" "); } } - value = value.replaceAll("\\*", ""); - value = value.replaceAll(SPECIAL_CHARS_TO_ESCAPE, "\\\\$1"); luceneQuery.append("title:").append(value).append("*"); return luceneQuery.toString(); } diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/AbstractQueryFilters.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/AbstractQueryFilters.java index 5a2833404225..2d66b7bb490c 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/AbstractQueryFilters.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/AbstractQueryFilters.java @@ -26,4 +26,21 @@ public interface AbstractQueryFilters { @JsonProperty("text") String text(); + /** + * Which fields {@link #text()} is matched against. + * + *

Sits here rather than at the top level of the request because it qualifies {@code text} + * and means nothing without it — the same reason {@link #filterFolders()} lives here. A request + * carrying a search scope with no text is rejected as the contract error it is, rather than + * being silently ignored.

+ * + *

Defaults to {@link SearchScope#ALL_FIELDS}, so a request that omits this field is + * processed exactly as it was before the field existed.

+ * + * @return the {@link SearchScope}, never {@code null} + */ + @JsonProperty("searchScope") + @Value.Default + default SearchScope searchScope() { return SearchScope.ALL_FIELDS; } + } diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveHelper.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveHelper.java index e4518dfc03a1..0761ec46262e 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveHelper.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveHelper.java @@ -180,9 +180,22 @@ public PaginatedContents driveSearch(final DriveRequestForm requestForm, final U if (null != requestForm.filters() && UtilMethods.isSet(requestForm.filters().text())) { builder.useElasticsearchFiltering(true) // Rely on ES for enhanced text filtering .filterFolderNames(requestForm.filters().filterFolders()) + .searchScope(requestForm.filters().searchScope()) .withFilter(requestForm.filters().text()); + } else if (null != requestForm.filters() + && SearchScope.ALL_FIELDS != requestForm.filters().searchScope()) { + // The search scope says which fields the TEXT is read against, so it is meaningless + // without text. Rejecting it makes the nonsense visible at the contract boundary + // instead of leaving it as a rule someone has to remember. + throw new BadRequestException( + "'filters.searchScope' qualifies 'filters.text' and cannot be used without it."); } + // Content Drive is the one caller with a user to tell when a query fails to execute. + // Every other consumer of this API keeps receiving today's empty result (see + // BrowserQuery.Builder#surfaceQueryFailures). + builder.surfaceQueryFailures(true); + // Per-field value filters (Content Drive). Field types are resolved against a single // content type; index-routed criteria also flip on ES filtering, while DB-routed criteria // (Tag) are resolved in the DB path. diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/SearchScope.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/SearchScope.java new file mode 100644 index 000000000000..095dca809282 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/SearchScope.java @@ -0,0 +1,33 @@ +package com.dotcms.rest.api.v1.drive; + +/** + * Which fields of a document a Content Drive search term is matched against. + * + *

Not to be confused with a browse scope, which says where you are browsing. The + * two are independent: a browse scope says where you are, a search scope says how a search reads + * what is there. Both travel on the same request, which is why neither is called simply + * "scope".

+ * + *

{@link #ALL_FIELDS} is the default, so a request that omits the scope is processed exactly as + * it was before this type existed.

+ * + * @see AbstractQueryFilters#searchScope() + */ +public enum SearchScope { + + /** + * The term is matched against every indexed field of the document — body copy, Story Block + * content and metadata included. The historical behavior of the Content Drive search box. + */ + ALL_FIELDS, + + /** + * The term is matched against the contentlet title only. A contentlet whose term occurrence is + * confined to body copy, Story Block content or metadata is not returned. + * + *

Folder and link name matching is unaffected: those never reach the search index and are + * narrowed on their own name in both scopes.

+ */ + TITLE + +} diff --git a/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyBaselineTest.java b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyBaselineTest.java new file mode 100644 index 000000000000..f5e1e90a578b --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyBaselineTest.java @@ -0,0 +1,116 @@ +package com.dotcms.rest.api.v1.content.search.strategies; + +import static org.junit.Assert.assertEquals; + +import com.dotcms.rest.api.v1.content.search.handlers.FieldContext; +import org.junit.Test; + +/** + * Characterization tests pinning the query {@link GlobalSearchAttributeStrategy} produces + * before the escaping fix of issue #37532, so that "All Fields results are unchanged" can be + * proved rather than asserted. + * + *

The spec's FR-009 promises no regression for All Fields search, with exactly one + * written carve-out: terms containing Lucene reserved characters or consecutive separators, whose + * results change by design because today's behavior for them is a malformed query. This class + * draws that line concretely:

+ * + *
    + *
  • Invariant cases — a term with no reserved character must produce a byte-identical + * query after the fix. These assertions must never be edited.
    • + *
  • Carve-out cases — a term with a reserved character produced a malformed query + * before the fix. These assertions were updated on 2026-09-14 when the fix landed, each + * carrying its before/after in a comment. They are the complete list of pre-existing test + * expectations this work changed (SC-002).
    • + *
+ * + * @see #37532 + */ +public class GlobalSearchAttributeStrategyBaselineTest { + + private static String query(final String value) { + return new GlobalSearchAttributeStrategy().generateQuery( + new FieldContext.Builder().withFieldName("title").withFieldValue(value).build()); + } + + // --------------------------------------------------------------------------------------- + // Invariant: no reserved characters. These MUST stay byte-identical after the fix (FR-009). + // --------------------------------------------------------------------------------------- + + /** A single plain word — the most common search there is. */ + @Test + public void baseline_plainWord() { + assertEquals( + "+(catchall:pricing*^10 OR title_dotraw:*pricing*^2) " + + "title:'pricing'^15 " + + "title:pricing*", + query("pricing")); + } + + /** Multiple words: a space is not a Lucene reserved character, so this is invariant too. */ + @Test + public void baseline_multipleWords() { + assertEquals( + "+(catchall:hello world*^10 OR title_dotraw:*hello world*^2) " + + "title:'hello world'^15 " + + "title:hello^5 title:world^5 " + + "title:hello world*", + query("hello world")); + } + + // --------------------------------------------------------------------------------------- + // Carve-out: reserved characters. These record TODAY'S BROKEN OUTPUT and are expected to + // change when #37532 is fixed. Each one is a query Elasticsearch cannot parse. + // --------------------------------------------------------------------------------------- + + /** + * A hyphen is reserved. Note the asymmetry that is the whole defect: the mandatory gate carries + * the RAW {@code angular-cms} while only the trailing clause is escaped to + * {@code angular\-cms}. After the fix every clause must be escaped. + */ + @Test + public void baseline_reservedCharacter_hyphen_isEscapedInEveryClause() { + // CHANGED by the #37532 fix. Before, the gate carried the RAW term and only the trailing + // clause was escaped: + // +(catchall:angular-cms*^10 OR title_dotraw:*angular-cms*^2) title:'angular-cms'^15 ... + assertEquals( + "+(catchall:angular\\-cms*^10 OR title_dotraw:*angular\\-cms*^2) " + + "title:'angular\\-cms'^15 " + + "title:angular\\-cms*", + query("angular-cms")); + } + + /** + * A forward slash is reserved by the {@code query_string} syntax but is absent from this + * strategy's private escape set, so it is not escaped anywhere — not even in the final + * clause. This is #37532's fifth acceptance criterion, reproduced. + */ + @Test + public void baseline_forwardSlash_isEscaped() { + // CHANGED by the #37532 fix. Before, "/" was absent from the private escape set so it was + // escaped NOWHERE — the issue's fifth acceptance criterion: + // +(catchall:a/b*^10 OR title_dotraw:*a/b*^2) title:'a/b'^15 title:a/b* + assertEquals( + "+(catchall:a\\/b*^10 OR title_dotraw:*a\\/b*^2) " + + "title:'a\\/b'^15 " + + "title:a\\/b*", + query("a/b")); + } + + /** + * Consecutive separators split into an empty token, emitting a term-less {@code title:^5} + * clause that cannot parse (FR-028). + */ + @Test + public void baseline_consecutiveSpaces_emitNoTermlessClause() { + // CHANGED by the #37532 fix (FR-028). Before, the empty token between the two spaces + // produced a term-less clause that could not parse: + // ... title:a^5 title:^5 title:b^5 ... + assertEquals( + "+(catchall:a b*^10 OR title_dotraw:*a b*^2) " + + "title:'a b'^15 " + + "title:a^5 title:b^5 " + + "title:a b*", + query("a b")); + } +} diff --git a/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyTest.java b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyTest.java new file mode 100644 index 000000000000..3907f4aab398 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/search/strategies/GlobalSearchAttributeStrategyTest.java @@ -0,0 +1,153 @@ +package com.dotcms.rest.api.v1.content.search.strategies; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.dotcms.rest.api.v1.content.search.handlers.FieldContext; +import org.junit.Test; + +/** + * Tests for {@link GlobalSearchAttributeStrategy} covering issue #37532: a search term must be + * matched as literal text, never as query syntax. + * + *

The defect these tests pin down is an asymmetry. The strategy escapes only its final clause, + * so the mandatory gate — the clause that decides whether a document matches at all — is built from + * the raw user input. A title containing {@code :} or {@code (} therefore produces a query + * Elasticsearch cannot parse, the failure is swallowed, and the author is told their content does + * not exist. See customer ticket 39185.

+ * + *

Today's (broken) output is pinned separately in + * {@link GlobalSearchAttributeStrategyBaselineTest}, which is what makes the change visible in + * review rather than implicit.

+ * + * @see #37532 + */ +public class GlobalSearchAttributeStrategyTest { + + /** The Lucene {@code query_string} reserved set, as documented on {@code LuceneQueryUtils}. */ + private static final char[] RESERVED = { + '\\', '+', '-', '!', '(', ')', ':', '^', '[', ']', '"', '{', '}', '~', '*', '?', '|', + '&', '/' + }; + + private static String query(final String value) { + return new GlobalSearchAttributeStrategy().generateQuery( + new FieldContext.Builder().withFieldName("title").withFieldValue(value).build()); + } + + /** The mandatory gate — everything up to the first {@code )} — is where matching is decided. */ + private static String gateOf(final String query) { + return query.substring(0, query.indexOf(')') + 1); + } + + // --------------------------------------------------------------------------------------- + // FR-027 — every clause is escaped, not only the last one. + // --------------------------------------------------------------------------------------- + + /** + * The defect in one assertion: the gate must not carry a raw reserved character. This is what + * makes the ticket 39185 headline unfindable today. + */ + @Test + public void mandatoryGate_escapesReservedCharacters() { + final String gate = gateOf(query("angular-cms")); + assertTrue("The mandatory gate must carry the ESCAPED term, not the raw one: " + gate, + gate.contains("angular\\-cms")); + assertFalse("The gate must not contain the unescaped hyphen: " + gate, + gate.contains("catchall:angular-cms")); + } + + /** + * Every character of the reserved set must be escaped, in every clause. A single unescaped + * occurrence anywhere is enough to break parsing of the whole query. + */ + @Test + public void everyReservedCharacter_isEscapedEverywhere() { + for (final char c : RESERVED) { + final String term = "a" + c + "b"; + final String result = query(term); + final String unescaped = "a" + c + "b"; + // The escaped form is what must appear; the bare form must not survive anywhere + // except as part of the escaped sequence. + assertTrue("Reserved character '" + c + "' must be escaped somewhere in: " + result, + result.contains("a\\" + c + "b")); + assertFalse("Reserved character '" + c + "' left unescaped in the gate: " + result, + gateOf(result).contains("catchall:" + unescaped)); + } + } + + /** + * A forward slash is reserved by the {@code query_string} syntax but is absent from the + * strategy's historical private escape set — #37532's fifth acceptance criterion, stated as a + * test of its own because it is the one character the old set silently omitted. + */ + @Test + public void forwardSlash_isEscaped() { + final String result = query("a/b"); + assertTrue("A forward slash must be escaped: " + result, result.contains("a\\/b")); + assertFalse("A raw forward slash must not survive: " + result, result.contains("a/b")); + } + + /** + * The {@code *} wildcards the strategy appends are syntax it adds itself, so they must sit + * OUTSIDE the escaped token. Escaping them would turn a prefix search into a literal search + * for an asterisk. + */ + @Test + public void appendedWildcards_areNotThemselvesEscaped() { + final String result = query("pricing"); + assertTrue("The catchall prefix wildcard must remain live syntax: " + result, + result.contains("catchall:pricing*")); + assertFalse("The appended wildcard must not be escaped: " + result, + result.contains("pricing\\*")); + } + + // --------------------------------------------------------------------------------------- + // FR-028 — consecutive separators must not emit empty clauses. + // --------------------------------------------------------------------------------------- + + /** {@code "a b"} must not produce a term-less {@code title:^5} clause. */ + @Test + public void consecutiveSeparators_emitNoEmptyClause() { + final String result = query("a b"); + assertFalse("An empty token produced a term-less clause: " + result, + result.contains("title:^5")); + } + + /** A term made only of separators yields no boost clauses at all rather than empty ones. */ + @Test + public void separatorsOnlyTerm_emitsNoEmptyClause() { + final String result = query(" , "); + assertFalse("A separators-only term produced a term-less clause: " + result, + result.contains("title:^5")); + } + + // --------------------------------------------------------------------------------------- + // FR-009 — terms with no reserved character must be untouched by this change. + // --------------------------------------------------------------------------------------- + + /** + * The carve-out has a hard edge: an ordinary term must produce the byte-identical query it + * produced before the fix. This is the assertion that keeps "no regression" honest. + */ + @Test + public void ordinaryTerm_isByteIdenticalToTheBaseline() { + assertEquals( + "+(catchall:pricing*^10 OR title_dotraw:*pricing*^2) " + + "title:'pricing'^15 " + + "title:pricing*", + query("pricing")); + } + + /** Multi-word ordinary terms keep their per-token boosts exactly as before. */ + @Test + public void ordinaryMultiWordTerm_isByteIdenticalToTheBaseline() { + assertEquals( + "+(catchall:hello world*^10 OR title_dotraw:*hello world*^2) " + + "title:'hello world'^15 " + + "title:hello^5 title:world^5 " + + "title:hello world*", + query("hello world")); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java b/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java index 542329b14ce7..fc6360f0ae0f 100644 --- a/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java +++ b/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java @@ -11,6 +11,7 @@ import com.dotcms.rest.api.v1.drive.ContentDriveFieldFilterTest; import com.dotcms.rest.api.v1.drive.ContentDriveHelperContentletAPIComparisonTest; import com.dotcms.rest.api.v1.drive.ContentDriveKeywordSearchTest; +import com.dotcms.rest.api.v1.drive.ContentDriveLiteralTextSearchTest; import com.dotcms.rest.api.v1.drive.ContentDriveLinksTest; import com.dotcms.rest.api.v1.drive.ContentDriveWorkflowArchiveStepTest; import com.dotcms.rest.api.v1.drive.ContentDriveWorkflowFilterTest; @@ -82,6 +83,7 @@ ContentDriveFieldFilterTest.class, ContentDriveHelperContentletAPIComparisonTest.class, ContentDriveKeywordSearchTest.class, + ContentDriveLiteralTextSearchTest.class, ContentDriveLinksTest.class, ContentDriveWorkflowArchiveStepTest.class, ContentDriveWorkflowFilterTest.class, diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveLiteralTextSearchTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveLiteralTextSearchTest.java new file mode 100644 index 000000000000..372b73ccf576 --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveLiteralTextSearchTest.java @@ -0,0 +1,235 @@ +package com.dotcms.rest.api.v1.drive; + +import com.dotcms.DataProviderWeldRunner; +import com.dotcms.IntegrationTestBase; +import com.dotcms.browser.BrowserAPIImpl.PaginatedContents; +import com.dotcms.contenttype.model.type.BaseContentType; +import com.dotcms.contenttype.model.field.TextField; +import com.dotcms.datagen.ContentTypeDataGen; +import com.dotcms.datagen.FieldDataGen; +import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.FolderDataGen; +import com.dotcms.datagen.SiteDataGen; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.contentlet.model.IndexPolicy; +import com.dotmarketing.portlets.folders.model.Folder; +import com.dotmarketing.util.Logger; +import com.liferay.portal.model.User; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.enterprise.context.ApplicationScoped; +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.junit.Assert.assertTrue; + +/** + * Regression test for issue #37532 — a Content Drive search term must be matched as literal + * text, never as Lucene {@code query_string} syntax. + * + *

Reported through customer ticket 39185: filtering on a value containing {@code :}, {@code (} + * or {@code /} returns "No results found" for content the user is looking at. The term is read as + * query syntax, the query fails to parse, the failure is logged and discarded, and the caller + * receives an empty result set indistinguishable from a genuine miss.

+ * + *

The issue was raised against the Content Search portlet, but the same defect reaches Content + * Drive: {@code GlobalSearchAttributeStrategy} escapes only its final clause, leaving the + * mandatory gate — the clause that decides whether a document matches at all — built from + * raw user input.

+ * + *

These tests must FAIL before the fix. A passing run against unmodified code would mean + * the defect does not reach Content Drive, and the premise of the spec would need revisiting.

+ * + * @see #37532 + */ +@ApplicationScoped +@RunWith(DataProviderWeldRunner.class) +public class ContentDriveLiteralTextSearchTest extends IntegrationTestBase { + + private static final ContentDriveHelper contentDriveHelper = new ContentDriveHelper(); + + /** The exact headline reported on customer ticket 39185. */ + private static final String TICKET_39185_HEADLINE = + "ABC Bank (XETRA: DBKGn.DB / NYSE: DB) and PSL Launch independent European CLO " + + "Total Return Indices"; + + /** + * The Lucene {@code query_string} reserved set. Each character gets its own seeded title so a + * failure names exactly which one is still being read as syntax (SC-010). + */ + private static final char[] RESERVED = { + '\\', '+', '-', '!', '(', ')', ':', '^', '[', ']', '"', '{', '}', '~', '*', '?', '|', + '&', '/' + }; + + /** A searchable text field, so the same reserved set can be exercised through field filters. */ + private static final String TOPIC_VAR = "topic"; + + private static User systemUser; + private static String assetPath; + private static String fieldFilterInode; + private static Host testSite; + private static ContentType testType; + + /** Seeded title → inode, so an assertion can prove the right row came back. */ + private static final Map seeded = new LinkedHashMap<>(); + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + systemUser = APILocator.getUserAPI().getSystemUser(); + final long languageId = APILocator.getLanguageAPI().getDefaultLanguage().getId(); + + final String uniqueId = System.currentTimeMillis() + ""; + testSite = new SiteDataGen().name("literal-text-" + uniqueId + ".local").nextPersisted(); + final Folder folder = + new FolderDataGen().name("literalFolder_" + uniqueId).site(testSite).nextPersisted(); + assetPath = "//" + testSite.getHostname() + folder.getPath(); + + testType = new ContentTypeDataGen() + .name("LiteralTextType_" + uniqueId) + .velocityVarName("literalTextType_" + uniqueId) + .baseContentType(BaseContentType.CONTENT) + .host(testSite) + .nextPersisted(); + + new FieldDataGen().type(TextField.class).name(TOPIC_VAR).velocityVarName(TOPIC_VAR) + .contentTypeId(testType.id()).searchable(true).indexed(true).nextPersisted(); + + seed(TICKET_39185_HEADLINE, folder, languageId); + + // For the field-filter half of #37532: a searchable field value carrying reserved + // characters, filtered through userSearchable rather than the search box. + fieldFilterInode = new ContentletDataGen(testType.id()) + .setProperty("title", "fieldfilter" + uniqueId) + .setProperty(TOPIC_VAR, "ABC (XETRA: DB) / topic" + uniqueId) + .folder(folder) + .languageId(languageId) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted() + .getInode(); + for (final char c : RESERVED) { + // A distinct, searchable title per reserved character. The marker keeps the titles + // unique to this run so a stray match from elsewhere cannot make the test pass. + seed("reserved" + uniqueId + c + "marker", folder, languageId); + } + + Logger.info(ContentDriveLiteralTextSearchTest.class, + String.format("Seeded %d titles under %s", seeded.size(), assetPath)); + } + + private static void seed(final String title, final Folder folder, final long languageId) { + final Contentlet item = new ContentletDataGen(testType.id()) + .setProperty("title", title) + .folder(folder) + .languageId(languageId) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + seeded.put(title, item.getInode()); + } + + @AfterClass + public static void cleanup() { + try { + if (null != testType) { + APILocator.getContentTypeAPI(systemUser).delete(testType); + } + } catch (final Exception e) { + Logger.warn(ContentDriveLiteralTextSearchTest.class, + "Could not delete test content type: " + e.getMessage()); + } + try { + if (null != testSite) { + APILocator.getHostAPI().archive(testSite, systemUser, false); + APILocator.getHostAPI().delete(testSite, systemUser, false); + } + } catch (final Exception e) { + Logger.warn(ContentDriveLiteralTextSearchTest.class, + "Could not delete test site: " + e.getMessage()); + } + } + + private PaginatedContents search(final String term) + throws DotDataException, DotSecurityException { + return contentDriveHelper.driveSearch(DriveRequestForm.builder() + .assetPath(assetPath) + .showFolders(false) + .live(false) + .archived(false) + .offset(0) + .maxResults(100) + .filters(QueryFilters.builder().text(term).build()) + .build(), systemUser); + } + + private static boolean contains(final PaginatedContents results, final String inode) { + return results.list.stream() + .map(item -> (String) item.get("inode")) + .anyMatch(inode::equals); + } + + /** + * #37532 also asks for equivalent behavior in Content Drive's field filters. Those route + * through {@code TextFieldStrategy}, which already escapes via {@code LuceneQueryUtils} and + * already drops empty tokens — so this test is expected to pass before the search-box fix + * as well as after. It exists so a regression there would be caught, and so the issue's + * corresponding criterion is met by evidence rather than by inspection. + */ + @Test + public void fieldFilterValue_withReservedCharacters_matchesLiterally() throws Exception { + final String uniquePart = assetPath.substring(assetPath.indexOf("literalFolder_") + 14) + .replace("/", ""); + final PaginatedContents results = contentDriveHelper.driveSearch(DriveRequestForm.builder() + .assetPath(assetPath) + .contentTypes(java.util.List.of(testType.variable())) + .showFolders(false).live(false).archived(false).offset(0).maxResults(100) + .userSearchable(java.util.Map.of(TOPIC_VAR, "ABC (XETRA: DB) / topic" + uniquePart)) + .build(), systemUser); + + assertTrue("A field-filter value containing reserved characters must match literally", + contains(results, fieldFilterInode)); + } + + /** + * The customer's case, end to end. Searching for the exact headline of a piece of content must + * return that content. + */ + @Test + public void ticket39185Headline_isFoundBySearchingItVerbatim() throws Exception { + final PaginatedContents results = search(TICKET_39185_HEADLINE); + assertTrue( + "The ticket 39185 headline was not returned when searched verbatim. This is the " + + "customer-reported defect: the colons, parentheses and slash are being " + + "read as query syntax instead of as part of the name.", + contains(results, seeded.get(TICKET_39185_HEADLINE))); + } + + /** + * Every character of the reserved set, one seeded title each, so a failure names the offending + * character rather than reporting a generic miss (SC-010). + */ + @Test + public void everyReservedCharacterInATitle_isFoundBySearchingItVerbatim() throws Exception { + final StringBuilder failures = new StringBuilder(); + for (final Map.Entry entry : seeded.entrySet()) { + if (entry.getKey().equals(TICKET_39185_HEADLINE)) { + continue; + } + final PaginatedContents results = search(entry.getKey()); + if (!contains(results, entry.getValue())) { + failures.append("\n - not found: '").append(entry.getKey()).append('\''); + } + } + assertTrue("Titles containing reserved characters were not findable by their own text:" + + failures, failures.length() == 0); + } +} From 6a90973df4d9d28517e662005703dafa81f8026f Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 14 Sep 2026 14:50:49 -0500 Subject: [PATCH 05/33] fix(content-drive): show a failed search as an error, not an empty grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes FR-029 for #37532. The backend now distinguishes "the query failed" from "nothing matched"; this puts that on screen. The store already set status ERROR on a failed search, but nothing rendered it: the grid simply went empty. That is the defect the customer reported — content they were looking at appeared not to exist. A banner now sits above the listing when the last search failed, with a retry. Above rather than instead of: replacing the grid would hide the thing the message explains, and it would also make the listing unreachable to component queries, which broke seven existing specs on the first attempt. Also removes a stale comment in the store claiming score-descending sorting for title search. Nothing has sorted by score since #36688 — the default is modDate on both sides — so the comment described behaviour the code does not have (progressive enhancement; spec Premise Correction 1). Four specs cover the banner: it appears, it stays alongside the listing it explains, it carries role="alert", and the retry re-runs the search. All 162 specs in the shell suite pass. Refs #37532, #37479 Co-Authored-By: Claude Opus 5 (1M context) --- .../dot-content-drive-shell.component.html | 18 +++++++ .../dot-content-drive-shell.component.spec.ts | 51 +++++++++++++++++++ .../dot-content-drive-shell.component.ts | 19 +++++++ .../src/lib/store/dot-content-drive.store.ts | 1 - .../WEB-INF/messages/Language.properties | 3 ++ 5 files changed, 91 insertions(+), 1 deletion(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html index 652ce320a2f8..2b64ad4e3d57 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html @@ -37,6 +37,24 @@ (uploadFiles)="onRequestUpload($event)" (dragEnter)="onDropzoneDragEnter()" class="col-start-2 row-start-3 overflow-auto"> + + @if ($searchFailed()) { + + } { }); }); + // A search that failed to run must not be presented as a search that found nothing. The two + // were the same empty grid before issue #37532, which is how a customer came to believe content + // they were looking at had vanished. + // + // The status is set per test and restored afterwards: it is shared across this file, and + // leaving it on ERROR hides the listing from every test that follows. + describe('when a search fails to run', () => { + afterEach(() => { + statusSignal.set(DotContentDriveStatus.LOADING); + spectator.detectChanges(); + }); + + const failSearch = () => { + statusSignal.set(DotContentDriveStatus.ERROR); + spectator.detectChanges(); + }; + + it('should show an error state instead of the listing', () => { + failSearch(); + + expect(spectator.query(byTestId('search-error-state'))).toBeTruthy(); + }); + + it('should keep the error visible alongside the listing it explains', () => { + failSearch(); + + // The banner sits above the grid rather than replacing it, so the user sees WHY the + // results are incomplete instead of an unexplained empty table. + expect(spectator.query(byTestId('search-error-state'))).toBeTruthy(); + expect(spectator.query('dot-folder-list-view')).toBeTruthy(); + }); + + it('should announce the failure to assistive technology', () => { + failSearch(); + + expect(spectator.query(byTestId('search-error-state'))?.getAttribute('role')).toBe( + 'alert' + ); + }); + + it('should re-run the search when the user retries', () => { + failSearch(); + + // The testid lands on the host; the clickable element is the + + + + + + + {{ item.label | dm }} + + + + `, changeDetection: ChangeDetectionStrategy.OnPush, imports: [ DotSearchInputComponent, - SelectButtonModule, + ListboxModule, + PopoverModule, TooltipModule, DotMessagePipe, FormsModule @@ -109,6 +138,16 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { DEFAULT_SEARCH_SCOPE ); + /** Whether the panel is open, so the chevron can point the right way. */ + protected readonly $panelOpen = signal(false); + + /** The trigger shows the active scope, as the mock does. */ + protected readonly $activeScopeLabel = computed( + () => + this.scopeOptions.find((option) => option.value === this.$searchScope())?.label ?? + 'content-drive.search.scope.all-fields' + ); + /** The box says what it will do before the user types again. */ protected readonly $placeholder = computed(() => this.$searchScope() === DOT_CONTENT_DRIVE_SEARCH_SCOPE.TITLE From a00fb2a3018daaf8aed6e69c5b8629a758ebdee3 Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 14 Sep 2026 23:28:13 -0500 Subject: [PATCH 13/33] refactor(content-drive): join the scope control to the input with p-inputgroup The previous attempt put a border around a component that already had one, so the control rendered as a box inside a box with a button bolted on. It did not belong on the page. p-inputgroup is the component for this: it owns the seam, the rounding and the shared border, which a hand-rolled wrapper could only approximate. The scope now sits in an addon, flush and borderless, following PrimeNG's own input-group-with-select recipe. The trigger is a p-select rather than a button driving a popover and a listbox. A dropdown that shows its selection and opens a panel of options is exactly what p-select is, and it brings the panel, the chevron, its open state, keyboard handling and the checkmark on the active option without any of it being reimplemented here. One override remains: the shared search box brings its own border and rounding, so its right edge is flattened and handed to the group. ::ng-deep because that markup belongs to @dotcms/ui rather than to this template, matching how the sidebar already reaches into p-tree. 30 of 30 portlet spec files green. PrimeNG usage validated against the component API. Refs #37479 Co-Authored-By: Claude Opus 5 (1M context) --- ...ntent-drive-search-input.component.spec.ts | 12 +- ...ot-content-drive-search-input.component.ts | 109 +++++++++--------- 2 files changed, 59 insertions(+), 62 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts index 64bc0f89c2a0..e53e931e8fa6 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts @@ -101,10 +101,10 @@ describe('DotContentDriveSearchInputComponent', () => { key === 'searchScope' ? scope : undefined ); - it('should render the scope trigger next to the search input', () => { + it('should render the scope control next to the search input', () => { spectator.detectChanges(); - expect(spectator.query(byTestId('search-scope-trigger'))).toBeTruthy(); + expect(spectator.query(byTestId('search-scope'))).toBeTruthy(); expect(searchInput()).toBeTruthy(); }); @@ -172,12 +172,10 @@ describe('DotContentDriveSearchInputComponent', () => { it('should name the control for assistive technology', () => { spectator.detectChanges(); - const trigger = spectator.query(byTestId('search-scope-trigger')); - expect(trigger?.getAttribute('aria-label')).toBeTruthy(); - // The trigger opens a listbox panel, and a screen reader has to be told so. - expect(trigger?.getAttribute('aria-haspopup')).toBe('listbox'); - expect(trigger?.getAttribute('aria-expanded')).toBe('false'); + expect( + spectator.query(byTestId('search-scope'))?.getAttribute('aria-label') + ).toBeTruthy(); }); it('should offer an explanation of what each option matches', () => { diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index e98f7ddce9ec..393956d9448a 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -3,14 +3,14 @@ import { Component, computed, inject, - signal, OnDestroy, viewChild } from '@angular/core'; import { FormsModule } from '@angular/forms'; -import { ListboxModule } from 'primeng/listbox'; -import { PopoverModule } from 'primeng/popover'; +import { InputGroupModule } from 'primeng/inputgroup'; +import { InputGroupAddonModule } from 'primeng/inputgroupaddon'; +import { SelectModule } from 'primeng/select'; import { TooltipModule } from 'primeng/tooltip'; import { @@ -36,62 +36,64 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' @Component({ selector: 'dot-content-drive-search-input', template: ` - -
+ + - - -
- - - - - - - {{ item.label | dm }} - - - - + (search)="onSearch($event)" /> + + + + + {{ item.label | dm }} + + + + {{ item.label | dm }} + + + + + `, + styles: [ + ` + /* The shared search box brings its own border and rounding. Inside an input group that + reads as a box inside a box, so its right edge is flattened and handed to the group. + ::ng-deep because the markup belongs to @dotcms/ui, not to this template. */ + :host ::ng-deep .p-inputgroup dot-search-input { + flex: 1 1 auto; + min-width: 0; + } + + :host ::ng-deep .p-inputgroup dot-search-input .p-inputtext { + border-top-right-radius: 0; + border-bottom-right-radius: 0; + border-right: 0; + } + ` + ], changeDetection: ChangeDetectionStrategy.OnPush, imports: [ DotSearchInputComponent, - ListboxModule, - PopoverModule, + InputGroupModule, + InputGroupAddonModule, + SelectModule, TooltipModule, DotMessagePipe, FormsModule @@ -138,9 +140,6 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { DEFAULT_SEARCH_SCOPE ); - /** Whether the panel is open, so the chevron can point the right way. */ - protected readonly $panelOpen = signal(false); - /** The trigger shows the active scope, as the mock does. */ protected readonly $activeScopeLabel = computed( () => From 40013537605666be292296d4aeb93484af5307e1 Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 14 Sep 2026 23:37:00 -0500 Subject: [PATCH 14/33] fix(content-drive): pin the scope control's width and match the chip palette MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three problems visible as soon as the control rendered on a real page. The width moved. p-select sizes itself to its selected label, so "Title" and "All Fields" produced two different widths — and since the input took whatever space was left, choosing a scope resized the text field under the user's cursor. The addon is now a fixed 140px, so neither the white field nor the group as a whole ever moves. Neither label was readable. Both rendered truncated — "T..." and "All ..." — because the select was sized by a layout that had not left it room. The fixed width fixes this too; 140px is what the filter chips below already use and it fits the longer label whole. The colours belonged to the default input theme rather than to this page. The border and label colour now match dot-chip-filter's own border-slate-200 and text-slate-600, so the search box and the chip row under it read as one family. 140px is not an invented number: the chips carry min-w-[140px] for the same reason, having solved the same jitter first. Taken as a fixed width rather than a minimum, because a minimum still lets the content push it. 30 of 30 portlet spec files green. Refs #37479 Co-Authored-By: Claude Opus 5 (1M context) --- ...ot-content-drive-search-input.component.ts | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index 393956d9448a..1433a67121d2 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -55,7 +55,7 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' [pTooltip]="'content-drive.search.scope.help' | dm" optionValue="value" appendTo="body" - class="dot-search-scope border-none! bg-transparent! shadow-none!" + class="dot-search-scope" data-testid="search-scope" tooltipPosition="bottom" (ngModelChange)="onScopeChange($event)"> @@ -86,6 +86,42 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' border-bottom-right-radius: 0; border-right: 0; } + + /* A FIXED width, not a minimum. + p-select sizes itself to its selected label, so "Title" and "All Fields" produced + two different widths — and because the input takes the remaining space, choosing a + scope resized the text field under the user's cursor. Pinning the addon means + neither the white field nor the whole group ever moves. 140px is what the filter + chips below already use, and it fits the longer label without truncating it. */ + :host ::ng-deep .p-inputgroup .p-inputgroup-addon { + width: 140px; + flex: 0 0 140px; + padding: 0; + } + + :host ::ng-deep .dot-search-scope { + width: 100%; + border: none; + background: transparent; + box-shadow: none; + } + + /* Let the label use the width it now has. */ + :host ::ng-deep .dot-search-scope .p-select-label { + overflow: visible; + text-overflow: clip; + } + + /* Match the filter chips rather than the default input theme: same border, same text + colour, so the search box and the chip row under it read as one family. */ + :host ::ng-deep .p-inputgroup dot-search-input .p-inputtext, + :host ::ng-deep .p-inputgroup .p-inputgroup-addon { + border-color: var(--color-slate-200, #e2e8f0); + } + + :host ::ng-deep .dot-search-scope .p-select-label { + color: var(--color-slate-600, #475569); + } ` ], changeDetection: ChangeDetectionStrategy.OnPush, From 40726d93ff31a541b6612a6a10d69427797cd451 Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 15 Sep 2026 00:10:51 -0500 Subject: [PATCH 15/33] refactor(content-drive): bring the scope control in line with the frontend standards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My changes broke two of them, and the second is why the last round of CSS had no visible effect anyway. The search input component carried an inline template and an inline `styles` block. ANGULAR_STANDARDS requires logic, template and styles in separate files; the template now lives in its own .html and the styles block is gone entirely rather than moved to a .scss. Gone rather than moved because the styling belongs in PrimeNG's PassThrough. `pt` is the component's supported way into its own internals, so the addon becomes transparent and the label gets its padding without any stylesheet and without ::ng-deep reaching past a component's API into its markup. STYLING_STANDARDS puts PrimeNG and Tailwind first and custom CSS last; this had it backwards. The failed-search banner had the same problem in a smaller way: a hand-rolled div with a heading, a paragraph and a button, in a shell that already imports MessageModule and already uses `p-message` with `pt` three lines further up. It is now a p-message with severity="error", which brings the styling, the icon and the semantics. Its heading key is dropped from Language.properties — the severity says what the heading was saying. No .scss file was added, no ::ng-deep survives in the working tree, and neither touched component carries an inline template or styles. 30 of 30 portlet spec files green. Refs #37479, #37532 Co-Authored-By: Claude Opus 5 (1M context) --- ...-content-drive-search-input.component.html | 46 ++++++++ ...ot-content-drive-search-input.component.ts | 107 +++--------------- .../dot-content-drive-shell.component.html | 29 +++-- .../WEB-INF/messages/Language.properties | 1 - 4 files changed, 81 insertions(+), 102 deletions(-) create mode 100644 core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html new file mode 100644 index 000000000000..fe3997a57bd0 --- /dev/null +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html @@ -0,0 +1,46 @@ + + + + + + + + + {{ item.label | dm }} + + + + {{ item.label | dm }} + + + + + diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index 1433a67121d2..c0202e467261 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -35,95 +35,7 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' */ @Component({ selector: 'dot-content-drive-search-input', - template: ` - - - - - - - - {{ item.label | dm }} - - - - {{ item.label | dm }} - - - - - - `, - styles: [ - ` - /* The shared search box brings its own border and rounding. Inside an input group that - reads as a box inside a box, so its right edge is flattened and handed to the group. - ::ng-deep because the markup belongs to @dotcms/ui, not to this template. */ - :host ::ng-deep .p-inputgroup dot-search-input { - flex: 1 1 auto; - min-width: 0; - } - - :host ::ng-deep .p-inputgroup dot-search-input .p-inputtext { - border-top-right-radius: 0; - border-bottom-right-radius: 0; - border-right: 0; - } - - /* A FIXED width, not a minimum. - p-select sizes itself to its selected label, so "Title" and "All Fields" produced - two different widths — and because the input takes the remaining space, choosing a - scope resized the text field under the user's cursor. Pinning the addon means - neither the white field nor the whole group ever moves. 140px is what the filter - chips below already use, and it fits the longer label without truncating it. */ - :host ::ng-deep .p-inputgroup .p-inputgroup-addon { - width: 140px; - flex: 0 0 140px; - padding: 0; - } - - :host ::ng-deep .dot-search-scope { - width: 100%; - border: none; - background: transparent; - box-shadow: none; - } - - /* Let the label use the width it now has. */ - :host ::ng-deep .dot-search-scope .p-select-label { - overflow: visible; - text-overflow: clip; - } - - /* Match the filter chips rather than the default input theme: same border, same text - colour, so the search box and the chip row under it read as one family. */ - :host ::ng-deep .p-inputgroup dot-search-input .p-inputtext, - :host ::ng-deep .p-inputgroup .p-inputgroup-addon { - border-color: var(--color-slate-200, #e2e8f0); - } - - :host ::ng-deep .dot-search-scope .p-select-label { - color: var(--color-slate-600, #475569); - } - ` - ], + templateUrl: './dot-content-drive-search-input.component.html', changeDetection: ChangeDetectionStrategy.OnPush, imports: [ DotSearchInputComponent, @@ -169,6 +81,23 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { } ]; + /** + * PrimeNG PassThrough: Tailwind classes handed to the select's own internals. + * + * This is how the component is styled without a stylesheet — `pt` is PrimeNG's supported way in, + * so nothing here reaches past the component's API into its markup. + * + * Each entry earns its place. The root sheds the chrome the input group already drew; the label + * gets the padding that otherwise leaves the chevron flush against the text; the dropdown gets + * its own column so it stays right-aligned. Colours match `dot-chip-filter`, so the search box + * and the chip row beneath it read as one family. + */ + protected readonly scopePt = { + root: 'h-full w-full border-0 bg-transparent shadow-none rounded-none', + label: 'flex items-center pl-3 pr-1 text-slate-600 whitespace-nowrap overflow-visible', + dropdown: 'w-8 shrink-0 grow-0 text-slate-500' + }; + /** Absent from the filters means the default — the scope is only stored when it differs. */ protected readonly $searchScope = computed( () => diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html index 2b64ad4e3d57..de5b8e7ece8d 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html @@ -39,22 +39,27 @@ class="col-start-2 row-start-3 overflow-auto"> @if ($searchFailed()) { - + role="alert" + class="block p-4"> +
+ {{ 'content-drive.search.error.message' | dm }} + +
+ } + Date: Tue, 15 Sep 2026 00:36:37 -0500 Subject: [PATCH 16/33] fix(content-drive): make the joined field actually join, with Tailwind `!` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read PrimeNG's own compiled CSS to find the real cause rather than guessing again. Two separate bugs, both hiding behind the same symptom (two disconnected boxes with a visible gap): 1. PrimeNG injects its component styles at runtime, after Tailwind's compiled stylesheet has already loaded. A plain Tailwind class and a PrimeNG rule targeting the same element (e.g. `.p-select { background: ...; border: ... }`) have equal specificity, and on a tie the later-inserted stylesheet wins — PrimeNG's. Every override in this component now carries Tailwind v4's `!` (important) modifier, which is the exact pattern PrimeNG's own inputgroup+select demo uses (`border-none! shadow-none! bg-transparent!` in their docs). 2. The addon side (`p-inputgroup-addon`) is a direct child of `p-inputgroup` and gets its own connecting rules for free — top/bottom border, right border and right-side rounding, all correct by default, confirmed straight from @primeuix/styles' inputgroup CSS. The search INPUT side never got the matching treatment, because `dot-search-input` is a wrapper component sitting between `p-inputgroup` and the actual ``, and PrimeNG's connecting CSS only rewires DIRECT `.p-iconfield`/`.p-component` children — the wrapper breaks that chain, so the input kept its own default border and full rounding on every side, rendering as its own separate box no matter what was applied to `p-inputgroup` itself. The second bug needed an actual capability, not a workaround: added an optional `inputClass` to the shared `DotSearchInputComponent` so a host that nests it in an input group can flatten its connecting edge from outside. Empty by default, so every other consumer (AssetPicker included, five usages) is unaffected — verified all still pass. Also switched from the `pTooltip` directive to Select's own `tooltip` / `tooltipPosition` inputs, dropping the now-unused TooltipModule import; Select implements them with the same underlying Tooltip directive, so the existing directive-instance assertion still holds. 390+ tests green: 30 portlet spec files, 13 ui spec files (394 tests, covering the AssetPicker and the shared search input), both libraries typecheck clean. Refs #37479 Co-Authored-By: Claude Sonnet 5 --- ...-content-drive-search-input.component.html | 26 ++++++++++++++----- ...ot-content-drive-search-input.component.ts | 2 -- .../dot-search-input.component.html | 2 +- .../dot-search-input.component.ts | 14 ++++++++++ 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html index fe3997a57bd0..dbfea567a1fd 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html @@ -2,13 +2,20 @@ border. The scope sits in an addon so the pair reads as a single control rather than a text box with a dropdown parked beside it. - Styling goes through PrimeNG's PassThrough (`pt`) rather than a stylesheet: it is the component's - own way of reaching its internals, so no CSS file is needed to make the addon transparent or to - give the label room. --> + Styling goes through PrimeNG's own PassThrough (`pt`) and Tailwind's `!` (important) modifier, + never a stylesheet. `!` is required, not decorative: PrimeNG injects its component styles at + runtime, after Tailwind's compiled stylesheet has already loaded, so a plain utility class of + equal specificity loses the tie. PrimeNG's own docs style a Select nested in an input group the + same way (`border-none! shadow-none! bg-transparent!`). --> + @@ -16,20 +23,25 @@ "All Fields" produced two different widths — and because the input takes the remaining space, choosing a scope resized the text field under the user's cursor. 140px is what the filter chips below already use, and it fits the longer label whole. - Transparent background and a left-only border: the addon has to read as part of the field - it sits in, with the border acting as the divider between the two halves. --> + + White background and a left-only border: the addon must read as part of the same field, + with that left edge as the one hairline divider between the two halves — not as its own + boxed panel. `p-inputgroupaddon` draws top/bottom/right borders on its own by default (it is + the group's last child), which is exactly the outer edge that is wanted; only the + background and the left divider need to be added explicitly. --> + class="w-[140px]! shrink-0 grow-0 border-y-0! border-r-0! border-l! border-slate-200! bg-white! p-0!"> diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index c0202e467261..3ffb6d0239ed 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -11,7 +11,6 @@ import { FormsModule } from '@angular/forms'; import { InputGroupModule } from 'primeng/inputgroup'; import { InputGroupAddonModule } from 'primeng/inputgroupaddon'; import { SelectModule } from 'primeng/select'; -import { TooltipModule } from 'primeng/tooltip'; import { DotKeyboardShortcutService, @@ -42,7 +41,6 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' InputGroupModule, InputGroupAddonModule, SelectModule, - TooltipModule, DotMessagePipe, FormsModule ], diff --git a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html index 30f9274e11c8..13a08d5d4d71 100644 --- a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html +++ b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html @@ -8,7 +8,7 @@ [placeholder]="$placeholder() | dm" [attr.aria-label]="$placeholder() | dm" autocomplete="off" - class="w-full" + [class]="'w-full ' + $inputClass()" [attr.data-testId]="$testId()" [formControl]="searchControl" /> @if ($text().length) { diff --git a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts index fedb62af7d4b..0fbd4fd3073e 100644 --- a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts +++ b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts @@ -67,6 +67,20 @@ export class DotSearchInputComponent { */ readonly $testId = input('search-input-field', { alias: 'testId' }); + /** + * Extra classes for the actual `` element, additive to its own `w-full`. + * + * Exists for one reason: a host that nests this component inside a `p-inputgroup` cannot reach + * the `` from outside to flatten its connecting edge, because PrimeNG's own + * inputgroup CSS only rewires direct structural relationships (`.p-inputgroup > .p-component`, + * `.p-inputgroup > .p-iconfield > .p-component`) — and this component's own host element sits + * between the group and that input, breaking the chain. Optional and empty by default, so + * every existing consumer (the AssetPicker included) is unaffected. + * + * @alias inputClass + */ + readonly $inputClass = input('', { alias: 'inputClass' }); + /** Emits the trimmed term once the debounce window closes. */ readonly search = output(); From 97e2c7f34ae57bd3d5d24de39d883eab99cc6a22 Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 15 Sep 2026 00:46:36 -0500 Subject: [PATCH 17/33] refactor(content-drive): rebuild the scope control on PrimeNG's own recipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Threw out the `p-select` + Tailwind `!important` approach entirely and rebuilt on https://v21.primeng.org/inputgroup#button, the pattern PrimeNG documents for exactly this: a trigger inside an input-group addon. The trigger is now a plain `pButton`, directly inside `p-inputgroup-addon` with no wrapper and no override classes. Confirmed straight from @primeuix/styles' own inputgroup CSS that this combination is self-styling: `.p-inputgroupaddon:has(.p-button)` zeroes the addon's padding and `.p-inputgroupaddon .p-button` zeroes the button's radius, which is exactly why the official demo needs no custom CSS either. No `!important` was ever going to be necessary here — the double-boxed look came from fighting a component (`p-select`) that isn't part of that recipe, not from insufficient specificity. The trigger opens a `p-popover` + `p-listbox`, styled with `CHIP_FILTER_POPOVER_PT` / `CHIP_FILTER_LISTBOX_PT` / `CHIP_FILTER_SCROLL_HEIGHT` — the exact constants a sibling in this same portlet (`dot-content-drive-workflow-filter`) already uses for its own dropdown. The panel now matches every other filter in the toolbar instead of inventing its own look. The one real gap — the search input's connecting edge, since its host component sits between `p-inputgroup` and the actual `` and breaks PrimeNG's structural CSS — is closed without `!important` or a stylesheet. `DotSearchInputComponent` gains an optional `inputDt` input, forwarded as `[dt]` onto the real ``. A design-token override sets the CSS custom property the component's own stylesheet already reads, so it applies by redefinition rather than by winning a specificity fight — which is why no `!` is needed there either. The override value itself reuses `{form.field.border.radius}`, the same token reference the active theme's own preset uses internally, rather than a hardcoded pixel guess. Empty by default; the five other consumers of the shared component are unaffected. 1398 tests green in the portlet, 394 in `ui` (13 files, the AssetPicker included), both libraries typecheck clean. Zero `!important`, zero extra wrapper divs, zero custom stylesheets. Refs #37479 Co-Authored-By: Claude Sonnet 5 --- ...-content-drive-search-input.component.html | 91 +++++++++---------- ...ntent-drive-search-input.component.spec.ts | 4 +- ...ot-content-drive-search-input.component.ts | 33 +++---- .../dot-search-input.component.html | 3 +- .../dot-search-input.component.ts | 21 +++-- 5 files changed, 79 insertions(+), 73 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html index dbfea567a1fd..3095665e1d24 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html @@ -1,58 +1,57 @@ - + The search input's connecting edge needs one, because this component's own host sits between + `p-inputgroup` and the real ``, breaking PrimeNG's structural CSS. `inputDt` reaches + through with a design-token override rather than a class, so it applies by redefining the + value the component's own stylesheet reads instead of competing with it for specificity. + + The panel reuses the same popover + listbox pass-through every other Content Drive filter + dropdown already uses (see dot-content-drive-workflow-filter), so this control matches the + rest of the toolbar instead of introducing a styling idiom of its own. --> - - - - + + + + + + + {{ item.label | dm }} + + + + diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts index e53e931e8fa6..aa099145f15f 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.spec.ts @@ -104,7 +104,7 @@ describe('DotContentDriveSearchInputComponent', () => { it('should render the scope control next to the search input', () => { spectator.detectChanges(); - expect(spectator.query(byTestId('search-scope'))).toBeTruthy(); + expect(spectator.query(byTestId('search-scope-trigger'))).toBeTruthy(); expect(searchInput()).toBeTruthy(); }); @@ -174,7 +174,7 @@ describe('DotContentDriveSearchInputComponent', () => { spectator.detectChanges(); expect( - spectator.query(byTestId('search-scope'))?.getAttribute('aria-label') + spectator.query(byTestId('search-scope-trigger'))?.getAttribute('aria-label') ).toBeTruthy(); }); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index 3ffb6d0239ed..567d24bc669e 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -8,11 +8,17 @@ import { } from '@angular/core'; import { FormsModule } from '@angular/forms'; +import { ButtonModule } from 'primeng/button'; import { InputGroupModule } from 'primeng/inputgroup'; import { InputGroupAddonModule } from 'primeng/inputgroupaddon'; -import { SelectModule } from 'primeng/select'; +import { ListboxModule } from 'primeng/listbox'; +import { PopoverModule } from 'primeng/popover'; +import { TooltipModule } from 'primeng/tooltip'; import { + CHIP_FILTER_LISTBOX_PT, + CHIP_FILTER_POPOVER_PT, + CHIP_FILTER_SCROLL_HEIGHT, DotKeyboardShortcutService, DotKeyboardShortcutUnregister, DotMessagePipe, @@ -38,9 +44,12 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' changeDetection: ChangeDetectionStrategy.OnPush, imports: [ DotSearchInputComponent, + ButtonModule, InputGroupModule, InputGroupAddonModule, - SelectModule, + ListboxModule, + PopoverModule, + TooltipModule, DotMessagePipe, FormsModule ], @@ -80,21 +89,13 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { ]; /** - * PrimeNG PassThrough: Tailwind classes handed to the select's own internals. - * - * This is how the component is styled without a stylesheet — `pt` is PrimeNG's supported way in, - * so nothing here reaches past the component's API into its markup. - * - * Each entry earns its place. The root sheds the chrome the input group already drew; the label - * gets the padding that otherwise leaves the chevron flush against the text; the dropdown gets - * its own column so it stays right-aligned. Colours match `dot-chip-filter`, so the search box - * and the chip row beneath it read as one family. + * The same popover/listbox pass-through every other Content Drive filter dropdown already uses + * (`dot-content-drive-workflow-filter`, `dot-status-filter`, …), so this panel matches the rest + * of the toolbar instead of introducing its own styling. */ - protected readonly scopePt = { - root: 'h-full w-full border-0 bg-transparent shadow-none rounded-none', - label: 'flex items-center pl-3 pr-1 text-slate-600 whitespace-nowrap overflow-visible', - dropdown: 'w-8 shrink-0 grow-0 text-slate-500' - }; + protected readonly POPOVER_PT = CHIP_FILTER_POPOVER_PT; + protected readonly LISTBOX_PT = CHIP_FILTER_LISTBOX_PT; + protected readonly SCROLL_HEIGHT = CHIP_FILTER_SCROLL_HEIGHT; /** Absent from the filters means the default — the scope is only stored when it differs. */ protected readonly $searchScope = computed( diff --git a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html index 13a08d5d4d71..6cfeaa6514ac 100644 --- a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html +++ b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.html @@ -8,7 +8,8 @@ [placeholder]="$placeholder() | dm" [attr.aria-label]="$placeholder() | dm" autocomplete="off" - [class]="'w-full ' + $inputClass()" + class="w-full" + [dt]="$inputDt()" [attr.data-testId]="$testId()" [formControl]="searchControl" /> @if ($text().length) { diff --git a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts index 0fbd4fd3073e..b5e9f69d9ebb 100644 --- a/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts +++ b/core-web/libs/ui/src/lib/components/dot-search-input/dot-search-input.component.ts @@ -68,18 +68,23 @@ export class DotSearchInputComponent { readonly $testId = input('search-input-field', { alias: 'testId' }); /** - * Extra classes for the actual `` element, additive to its own `w-full`. + * PrimeNG design-token overrides for the actual ``, e.g. to flatten one side's radius + * when this component sits inside a `p-inputgroup`. * - * Exists for one reason: a host that nests this component inside a `p-inputgroup` cannot reach - * the `` from outside to flatten its connecting edge, because PrimeNG's own - * inputgroup CSS only rewires direct structural relationships (`.p-inputgroup > .p-component`, + * Exists for one reason: a host that nests this component inside an input group cannot reach + * the `` from outside, because PrimeNG's own inputgroup CSS only rewires direct + * structural relationships (`.p-inputgroup > .p-component`, * `.p-inputgroup > .p-iconfield > .p-component`) — and this component's own host element sits - * between the group and that input, breaking the chain. Optional and empty by default, so - * every existing consumer (the AssetPicker included) is unaffected. + * between the group and that input, breaking the chain. * - * @alias inputClass + * `dt`, not a class: a design-token override sets the CSS custom property the component's own + * stylesheet already reads, so it applies unconditionally rather than fighting PrimeNG's + * dynamically-injected styles for specificity. Empty by default, so every existing consumer + * (the AssetPicker included) is unaffected. + * + * @alias inputDt */ - readonly $inputClass = input('', { alias: 'inputClass' }); + readonly $inputDt = input>({}, { alias: 'inputDt' }); /** Emits the trimmed term once the debounce window closes. */ readonly search = output(); From 9c9c47d03f712777441205df4fab956ca117f32f Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 15 Sep 2026 00:51:17 -0500 Subject: [PATCH 18/33] fix(content-drive): pin the trigger's width and match its border to the input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two mismatches, both traced to a specific cause rather than patched by guessing. The button resized with its label because a p-button has no width rule of its own — width is purely content-driven, so "Title" and "All Fields" produced two different button sizes and the whole group visibly moved. Fixed with a plain Tailwind `w-[140px]`, the same width the filter chips below already use; there is nothing in Button's own stylesheet to compete with on this property, so no override mechanic was needed at all. The border colours never matched because Button and InputText draw from different token families in this theme. `button.secondary.border.color` resolves to `{surface.700}`; `inputtext.border.color` resolves to `{form.field.border.color}`. Confirmed straight from the Lara preset source, not guessed from how the colours looked. `TRIGGER_DT` repoints the button's border at the same token the input already uses, so the two stay identical if the theme itself changes rather than silently drifting apart again. Also truncates the label and shrinks the chevron so the longer "All Fields" text doesn't wrap or push the icon around inside the now-fixed width. 1398 tests still green across all 30 portlet spec files. Zero `!important`, zero extra divs — the width is Tailwind because it's layout, the border is `dt` because it's a PrimeNG token PrimeNG itself needed to be told about. Refs #37479 Co-Authored-By: Claude Sonnet 5 --- ...-content-drive-search-input.component.html | 25 ++++++++++++++++--- ...ot-content-drive-search-input.component.ts | 13 ++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html index 3095665e1d24..64f79ba2a67d 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html @@ -16,23 +16,42 @@ + diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index 567d24bc669e..f24ef510094d 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -88,6 +88,19 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { } ]; + /** + * Repoints the trigger's border at the exact token the search input already uses. + * + * Button and InputText are different PrimeNG components with separate token families — this + * theme's `button.secondary.border.color` resolves to `{surface.700}` while + * `inputtext.border.color` resolves to `{form.field.border.color}`, so a plain `severity` + * button never matched the field it sits beside. A design-token override, not a literal colour: + * if the active theme's form-field border ever changes, this stays in step with it. + */ + protected readonly TRIGGER_DT = { + secondary: { borderColor: '{form.field.border.color}' } + }; + /** * The same popover/listbox pass-through every other Content Drive filter dropdown already uses * (`dot-content-drive-workflow-filter`, `dot-status-filter`, …), so this panel matches the rest From 3fb5e6cdbee9592b45d503ff314faf6c2cc0cffb Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 15 Sep 2026 00:54:16 -0500 Subject: [PATCH 19/33] fix(content-drive): match the trigger's background and text colour too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix only repointed the border and made things worse: this theme's `button.secondary` sets `borderColor: {surface.100}`, identical to its own `background: {surface.100}` — the default secondary button's border is invisible by design, matching its own fill. Overriding only the border color left a visibly grey-filled button sitting next to a white field, a bigger mismatch than the one being fixed. `TRIGGER_DT` now repoints all three: background, border colour and text colour, each to the exact token InputText already uses (`form.field.background`, `form.field.border.color`, `form.field.color`). Button and InputText are simply different PrimeNG components with separate token families end to end, not a single misaligned property, so partial repointing was always going to leave a mismatch somewhere. 1398 tests still green, typecheck clean. Refs #37479 Co-Authored-By: Claude Sonnet 5 --- ...ot-content-drive-search-input.component.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts index f24ef510094d..138ce38703d8 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.ts @@ -89,16 +89,24 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { ]; /** - * Repoints the trigger's border at the exact token the search input already uses. + * Repoints the trigger at the exact tokens the search input already uses, so the two read as + * one field rather than a white box beside a grey one. * - * Button and InputText are different PrimeNG components with separate token families — this - * theme's `button.secondary.border.color` resolves to `{surface.700}` while - * `inputtext.border.color` resolves to `{form.field.border.color}`, so a plain `severity` - * button never matched the field it sits beside. A design-token override, not a literal colour: - * if the active theme's form-field border ever changes, this stays in step with it. + * Button and InputText are different PrimeNG components with entirely separate token families. + * In this theme `button.secondary` resolves to `background: {surface.100}`, + * `borderColor: {surface.100}` (identical to its own background — invisible by design) and + * `color: {surface.600}`, while `inputtext` resolves to `{form.field.background}`, + * `{form.field.border.color}` and `{form.field.color}`. Overriding the border alone left a + * visible grey-filled button next to a white field; all three have to move together for the + * pair to read as one control. Each is a design-token override, not a literal colour, so the + * two stay identical if the active theme itself changes. */ protected readonly TRIGGER_DT = { - secondary: { borderColor: '{form.field.border.color}' } + secondary: { + background: '{form.field.background}', + borderColor: '{form.field.border.color}', + color: '{form.field.color}' + } }; /** From 1f995a5e3db4a4de2b23a21ab94716484a8d27ba Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 15 Sep 2026 01:19:44 -0500 Subject: [PATCH 20/33] fix(content-drive): replace the important-modifier with pt; freeze the placeholder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the manually-applied `border-none!` with `[pButtonPT]`'s `root.style`. `pButtonPT`'s value reaches the host through the `[style]`/`[class]` HOST BINDINGS `Bind` (the directive backing `pButton`) already declares — the same mechanism as any `[style]` binding, applied as a real inline style — so it wins the cascade over PrimeNG's own injected `.p-button-secondary` rule unconditionally, the same guarantee `!important` gave without reaching for it. Also freezes the placeholder. It no longer describes the active scope: the `[placeholder]` binding is removed entirely so the shared `DotSearchInputComponent`'s own default ("Search") always applies, regardless of Title vs All Fields. The `$placeholder` computed is deleted rather than left unused. Spec note: this narrows FR-003 ("The search input's placeholder MUST describe the active scope"), decided directly against the approved spec rather than derived from it. Recorded here as a marker; the amendment itself belongs in tasks.md alongside the FR-029 one already pending re-approval before PR 2. 1398 tests still green, typecheck clean, zero `!important` in the component's own code (two mentions left are prose, inside a comment explaining why one is no longer needed). Refs #37479 Co-Authored-By: Claude Sonnet 5 --- ...-content-drive-search-input.component.html | 30 +++++------ ...ntent-drive-search-input.component.spec.ts | 17 +++---- ...ot-content-drive-search-input.component.ts | 50 ++++++++++--------- 3 files changed, 49 insertions(+), 48 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html index 64f79ba2a67d..4ccd1c840afa 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/components/dot-content-drive-search-input/dot-content-drive-search-input.component.html @@ -13,40 +13,40 @@ dropdown already uses (see dot-content-drive-workflow-filter), so this control matches the rest of the toolbar instead of introducing a styling idiom of its own. --> + - + The button's own border is removed via `[pButtonPT]`'s `root.style`, not a `!important` + class: PT's `style` slot is applied through the directive's own `[style]` host binding (see + `TRIGGER_PT`), which is a real inline style and wins the cascade unconditionally, the same + guarantee `!important` gives without needing it. What is left showing is the addon's own + border, which already equals the input's — `inputgroup.addon.borderColor` resolves to the + same `{form.field.border.color}` alias `inputtext` uses, confirmed straight from the Lara + preset source. -->