diff --git a/core-web/apps/dotcms-ui-e2e/src/pages/contentDrive.page.ts b/core-web/apps/dotcms-ui-e2e/src/pages/contentDrive.page.ts index 121d7e5bb0cd..852a674f81e0 100644 --- a/core-web/apps/dotcms-ui-e2e/src/pages/contentDrive.page.ts +++ b/core-web/apps/dotcms-ui-e2e/src/pages/contentDrive.page.ts @@ -21,6 +21,18 @@ import { Portlet } from '@utils/portlets'; */ const OUTCOME_TIMEOUT = 60000; +/** + * The part of the `/api/v1/drive/search` request body the scope tests assert on. Deliberately + * partial — the full form is the backend's business — so a new server-side field does not break + * these tests, while the two that matter (`text`, `searchScope`) are named. + */ +interface DriveSearchPayload { + filters: { + text?: string; + searchScope?: string; + }; +} + export class ContentDrivePage { readonly toolbar: Locator; readonly treeSelector: Locator; @@ -136,6 +148,89 @@ export class ContentDrivePage { }); } + /** + * Runs `action` and returns the JSON request body of the drive search it triggers. + * + * The wait is armed BEFORE `action` runs — the only order that works — and the predicate also + * pins the method and status, so a failing search fails here at the capture rather than + * downstream as a missing row. + * + * `matches` guards against a race the portlet's own startup creates: it can still be settling + * its initial listing when this is called, and those searches carry their own (usually empty) + * text. Every attempt arms before anything pending can slip past, so `action`'s own request + * is never missed — a settling search only costs one round of the loop. + */ + async captureSearchPayload( + action: () => Promise, + matches?: (payload: DriveSearchPayload) => boolean + ): Promise { + for (let attempt = 0; attempt < 6; attempt++) { + const search = this.page.waitForResponse( + (r) => + r.url().includes('/api/v1/drive/search') && + r.request().method() === 'POST' && + r.status() === 200, + { timeout: 15000 } + ); + + if (attempt === 0) { + await action(); + } + + const response = await search; + const payload = (await response.request().postDataJSON()) as DriveSearchPayload; + + if (!matches || matches(payload)) { + return payload; + } + } + + throw new Error('no drive search matching the expected payload was captured'); + } + + /** + * Runs `body` and fails if a drive search that matches `matches` was submitted while it did. + * + * The evidence a negative needs: "re-selecting the active scope must not re-search" (FR-006) + * cannot be asserted on copy or on rows that did not appear — only watching the wire can. The + * same pattern `expectNothingUploadedWhile` uses for uploads. + * + * The payloads are inspected rather than the URLs: without `matches` any drive search in the + * window is a violation, but with it a caller can name precisely what a violation would look + * like. That is what lets the scope test watch a term-carrying window and ignore a leftover + * startup search (an empty `text`) instead of draining it with extra round trips first — a + * re-search the click caused would carry the same term, so the predicate catches it all the + * same while the window stays open. The same evidence pattern, one request fewer to prove it. + */ + async expectNoSearchWhile( + body: () => Promise, + matches?: (payload: DriveSearchPayload) => boolean + ) { + const offenders: DriveSearchPayload[] = []; + const record = (request: Request) => { + if (request.url().includes('/api/v1/drive/search') && request.method() === 'POST') { + const payload = request.postDataJSON() as DriveSearchPayload; + + if (!matches || matches(payload)) { + offenders.push(payload); + } + } + }; + + this.page.on('request', record); + + try { + await body(); + } finally { + this.page.off('request', record); + } + + expect( + offenders, + 'a drive search fired, so the scope re-selection was not ignored' + ).toEqual([]); + } + /** Navigates into a folder by clicking its row in the tree. */ async openFolder(name: string) { await this.treeNodeLabels.filter({ hasText: name }).first().click(); diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/content-drive/content-drive-search-scope.spec.ts b/core-web/apps/dotcms-ui-e2e/src/tests/content-drive/content-drive-search-scope.spec.ts new file mode 100644 index 000000000000..f3a41187e905 --- /dev/null +++ b/core-web/apps/dotcms-ui-e2e/src/tests/content-drive/content-drive-search-scope.spec.ts @@ -0,0 +1,441 @@ +import { ContentDrivePage } from '@pages'; +import { type APIRequestContext } from '@playwright/test'; +import { createFakePayloadTextField } from '@utils/dot-content-types.mock'; +import { Portlet } from '@utils/portlets'; + +import { ContentDriveSearchScope } from './helpers/content-drive-search'; +import { ContentDriveTree } from './helpers/content-drive-tree'; + +import { type ContentDriveApiHelpers, expect, test } from '../../fixtures/content-drive.fixture'; +import { createContentlet, deleteContentlets } from '../../requests/contentlets'; +import { createFakeContentType, deleteContentType } from '../../requests/contentType'; + +/** + * Journey: Content Drive search scope — searching by Title or by All Fields (issue #37479) + * + * The query construction itself is the backend's half, covered by the Java integration suite. + * What a browser adds is the wiring no lower suite sees end to end: the control driving real + * `/api/v1/drive/search` requests, the rows those requests return, the address-bar round trip, + * and the p-listbox toggle that emits `null` when the active option is re-clicked — the exact + * kind of synthesized-event gap the keyboard spec exists for. + */ + +/** + * What one test's seeding produces. The term is the 8-character per-test suffix behind a `zq` + * prefix — rare enough that the only matches in the drive are the rows this test planted. + */ +interface SeededWorld { + term: string; + /** A folder whose NAME contains the term, so folder rows match it (FR-011). */ + folderName: string; + /** The default site's hostname, captured once so the teardown does not re-resolve it. */ + siteHostname: string; + /** A contentlet the term reaches through its TITLE — it is the title's first token. */ + titleRowTitle: string; + /** A contentlet the term reaches ONLY through its body field — its title never contains it. */ + fieldRowTitle: string; + contentletIds: string[]; + contentTypeId: string; +} + +/** + * Seeds one test's search world through the REST API. + * + * The two contentlets are the pair that tells the scopes apart: in All Fields the term is found + * via the catchall (the body token), so both rows come back; in Title scope the prefix match only + * reaches the row whose title starts with the term, so the field row disappears. Both sit at the + * site root, which is what makes the drive-wide search test readable without a second folder. + */ +async function seedSearchWorld( + request: APIRequestContext, + apiHelpers: ContentDriveApiHelpers, + testSuffix: string +): Promise { + const site = await apiHelpers.getDefaultSite(); + const term = `zq${testSuffix}`; + const folderName = `${term}-folder`; + + await apiHelpers.createFolders(site.hostname, [`/${folderName}`]); + + const contentType = await createFakeContentType(request, { + name: `ZqSearch${testSuffix}`, + fields: [ + createFakePayloadTextField({ name: 'Title', variable: 'title', sortOrder: 1 }), + createFakePayloadTextField({ + name: 'Search body', + variable: 'searchBody', + sortOrder: 2, + // dotCMS does not index a field unless told to, and the whole point of this + // field is to be the one the term reaches through the all-fields catchall. An + // unindexed field makes the body row invisible in BOTH scopes, which fails the + // test for the wrong reason. + indexed: true + }) + ] + }); + + const titleRow = await createContentlet(request, { + contentType: contentType.variable, + title: `${term} hero asset`, + searchBody: 'a text no search term reaches' + }); + const fieldRow = await createContentlet(request, { + contentType: contentType.variable, + title: `Herd notes ${testSuffix}`, + searchBody: term + }); + + return { + term, + folderName, + siteHostname: site.hostname, + titleRowTitle: `${term} hero asset`, + fieldRowTitle: `Herd notes ${testSuffix}`, + contentletIds: [titleRow.identifier, fieldRow.identifier], + contentTypeId: contentType.id + }; +} + +test.describe('Content Drive Search Scope', () => { + // These tests watch real round trips — each capture waits out the search debounce plus the + // backend, and the teardown closes a recorded context and fires the API deletes — all inside + // the default 60s budget. On a loaded CI runner (8 workers over a shared Docker backend, per + // the pom's own notes) five of them finished their bodies right at the edge and ran out of + // budget at teardown, three retries in a row. The doubled budget is not covering a broken + // assertion — none fired — it sizes the budget to what watching the wire honestly costs; a + // genuinely hung test still fails here, just with room to show which step hung. + test.setTimeout(120_000); + + // Set by every test through `seed`, read by the teardown. Deliberately a describe-level + // `let` used only for cleanup — each test seeds its own world before asserting on it, so + // nothing is shared and `fullyParallel` stays honest. + let world: SeededWorld | undefined; + + const seed = async ( + request: APIRequestContext, + apiHelpers: ContentDriveApiHelpers, + testSuffix: string + ) => { + world = await seedSearchWorld(request, apiHelpers, testSuffix); + + return world; + }; + + test.afterEach(async ({ request, apiHelpers }) => { + if (world) { + await deleteContentlets(request, world.contentletIds); + + // The site is already known — the seeding fetched it — so the teardown does not spend + // a round trip re-resolving it. Every call here rides on the test's own timeout + // budget; the leaner this stays, the more of that budget the browser teardown keeps. + await apiHelpers.deleteFolders(world.siteHostname, [`/${world.folderName}`]); + + await deleteContentType(request, world.contentTypeId); + world = undefined; + } + }); + + test('presents the scope control on the all-fields default with a plain placeholder', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + + await search.expectActive('ALL_FIELDS'); + // The control names itself for assistive technology (FR-023), and the placeholder stays + // the shared input's own "Search" — it deliberately does not describe the active scope + // (FR-003 as amended). + await expect(search.trigger).toHaveAttribute('aria-label', 'Search in'); + await expect(search.input).toHaveAttribute('placeholder', 'Search'); + }); + + test('marks the active option and explains every option in the panel', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + await search.open(); + + await search.expectOptionMarked('ALL_FIELDS'); + // FR-022: each option carries the explanation of what it matches. Asserted on a stable + // phrase of the English copy, so rewording the rest does not break the test. + await search.expectOptionExplained('TITLE', 'name only'); + await search.expectOptionExplained('ALL_FIELDS', 'any text inside it'); + }); + + test('narrows the results to title matches when title scope is chosen @critical', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + + // The default scope sends no scope field at all — omitted, not null (FR-017) — and finds + // both rows, the title one and the body-field one. + const allFields = await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + expect(allFields.filters).not.toHaveProperty('searchScope'); + expect(allFields.filters.text).toBe(seeded.term); + await drive.expectListContainsTitle(seeded.titleRowTitle); + await drive.expectListContainsTitle(seeded.fieldRowTitle); + + // Choosing a scope re-runs the search immediately (FR-004), carrying the scope (FR-017). + // The prefix match reaches the title row but not the body-field row (FR-008). + const title = await drive.captureSearchPayload( + () => search.choose('TITLE'), + (payload) => payload.filters.searchScope === 'TITLE' + ); + expect(title.filters.text).toBe(seeded.term); + expect(title.filters.searchScope).toBe('TITLE'); + await drive.expectListContainsTitle(seeded.titleRowTitle); + await expect(drive.listTitles.filter({ hasText: seeded.fieldRowTitle })).toHaveCount(0); + }); + + test('restores all-fields results when the scope returns to all fields @critical', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + await search.choose('TITLE'); + + // Back to the default: the scope is dropped from the request entirely, never sent as a + // literal 'ALL_FIELDS' (FR-017, mirrored by the store deleting the key), and the row only + // All Fields can see comes back (FR-009's no-regression promise). + const restored = await drive.captureSearchPayload( + () => search.choose('ALL_FIELDS'), + (payload) => payload.filters.text === seeded.term && !('searchScope' in payload.filters) + ); + expect(restored.filters).not.toHaveProperty('searchScope'); + await drive.expectListContainsTitle(seeded.fieldRowTitle); + }); + + test('does not search again when the active scope is re-selected @critical', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + + // Type the term the guard protects. The capture waits for THIS search's response — a + // predicate that only a term-carrying request satisfies — so the portlet's own startup + // searches drain behind it without spending round trips flushing them by hand. + await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + + // p-listbox is single-select with toggle semantics: re-clicking the active option emits + // `null`, which the component must ignore. A re-run would reset the user to page 1 for + // nothing, and only watching the wire can prove it did not happen. The guard reads + // payloads, not URLs: a re-search the click caused would carry this same term, while a + // leftover startup search (empty `text`) is not a violation — the predicate keeps the two + // apart, so the proof stays exact without any settling choreography. + await drive.expectNoSearchWhile( + async () => { + await search.open(); + await search.option('ALL_FIELDS').click(); + // A re-search would be a state change → store effect → request, all synchronous + // with the click itself — unlike the typing path, which carries the debounce. The + // panel closing is the deterministic signal that the click's consequences ran. + await expect(search.panel).toBeHidden(); + }, + (payload) => payload.filters.text === seeded.term + ); + }); + + test('searches drive-wide and leaves the selected folder behind', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const tree = new ContentDriveTree(adminPage); + + await drive.goTo(); + + // Narrow the listing to the (empty) seeded folder first, so what appears next cannot be + // explained by the folder scope: both contentlets live at the site root, outside it. + await tree.selectFolder(seeded.folderName); + + await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + + await drive.expectListContainsTitle(seeded.titleRowTitle); + await drive.expectListContainsTitle(seeded.fieldRowTitle); + }); + + test('returns the scope to all fields when all filters are cleared @critical', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + await search.choose('TITLE'); + + const clearAll = adminPage.getByTestId('clear-all-filters'); + await expect(clearAll).toBeVisible(); + await clearAll.click(); + + // The scope qualifies the term; with the filters gone it must be gone too, or the trigger + // would claim a scope the listing is not using (FR-020). + await search.expectActive('ALL_FIELDS'); + }); + + test('resolves a deep link to the scope it names and degrades an unknown one @critical', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + // The address carries the scope beside the other filters (FR-014), and the drive runs the + // filtered search on load — no typing needed. + await adminPage.goto( + `${Portlet.ContentDrive}?filters=${encodeURIComponent( + `title:${seeded.term};searchScope:TITLE` + )}` + ); + await expect(drive.toolbar).toBeVisible({ timeout: 20000 }); + + await search.expectActive('TITLE'); + await drive.expectListContainsTitle(seeded.titleRowTitle); + await expect(drive.listTitles.filter({ hasText: seeded.fieldRowTitle })).toHaveCount(0); + + // A stale or hand-edited scope must degrade to the default, never 400 the listing + // (FR-015): the unknown value is dropped and the term keeps its All Fields breadth. + await adminPage.goto( + `${Portlet.ContentDrive}?filters=${encodeURIComponent( + `title:${seeded.term};searchScope:WAT` + )}` + ); + await expect(drive.toolbar).toBeVisible({ timeout: 20000 }); + + await search.expectActive('ALL_FIELDS'); + await drive.expectListContainsTitle(seeded.fieldRowTitle); + }); + + test('starts a clean visit on all fields after a scope was changed', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + await search.choose('TITLE'); + + // The scope is filter state in the address, not a per-user preference (FR-016): a clean + // entry — no query string — starts over on the default. A second goto alone would only + // change the hash inside the same Angular document: no reload, no startup requests, and + // goTo's own waits would stall on responses that never fire. Leaving the app first makes + // the return a real page load — a genuinely clean visit, which is the thing under test. + await adminPage.goto('about:blank'); + await drive.goTo(); + + await search.expectActive('ALL_FIELDS'); + }); + + test('searches a term made of query syntax without breaking the listing', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + + await drive.goTo(); + + // The term is treated as literal text (FR-027): the capture's predicate already proves + // the request answered 200 — an unescaped term would have failed the query before any + // results could render (FR-029), surfacing as the search error toast and a stale grid. + await drive.captureSearchPayload( + () => drive.searchField.fill('(a+b)'), + (payload) => payload.filters.text === '(a+b)' + ); + + await expect(adminPage.locator('.p-toast-message')).toHaveCount(0); + await expect(drive.listTitles).toHaveCount(0); + }); + + test('matches folders by name identically in both scopes', async ({ + adminPage, + apiHelpers, + request, + testSuffix + }) => { + const seeded = await seed(request, apiHelpers, testSuffix); + const drive = new ContentDrivePage(adminPage); + const search = new ContentDriveSearchScope(adminPage); + + await drive.goTo(); + + await drive.captureSearchPayload( + () => drive.searchField.fill(seeded.term), + (payload) => payload.filters.text === seeded.term + ); + await drive.expectListContainsTitle(seeded.folderName); + + // Folder names match the term as a substring no matter the scope (FR-011): the scope + // narrows contentlets, not the structure the user navigates through. + await search.choose('TITLE'); + await drive.expectListContainsTitle(seeded.folderName); + }); +}); diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/content-drive/helpers/content-drive-search.ts b/core-web/apps/dotcms-ui-e2e/src/tests/content-drive/helpers/content-drive-search.ts new file mode 100644 index 000000000000..e75f94771a66 --- /dev/null +++ b/core-web/apps/dotcms-ui-e2e/src/tests/content-drive/helpers/content-drive-search.ts @@ -0,0 +1,90 @@ +import { expect, type Locator, type Page } from '@playwright/test'; + +/** + * The two values the scope control offers, mirroring `DotContentDriveSearchScope` on the UI side. + * They are also the `data-testid` suffixes of the panel's options. + */ +export type DriveSearchScope = 'TITLE' | 'ALL_FIELDS'; + +/** + * The English trigger label for each scope. The instance runs the default locale, and the label is + * how a scope change is observed without opening the panel, so the strings live here rather than + * being inlined at every call site. Source of truth: `content-drive.search.scope.*` in + * `Language.properties`. + */ +const SCOPE_LABEL: Record = { + TITLE: 'Title', + ALL_FIELDS: 'All Fields' +}; + +/** + * Locator wrapper for the Content Drive search scope control — the trigger in the toolbar addon + * and the popover panel it opens (issue #37479). + * + * Every `data-testid` lives on `dot-content-drive-search-input` itself, so this stays a locator + * question: no behavior is re-implemented here, and a testid change fails loudly at the locator. + */ +export class ContentDriveSearchScope { + readonly trigger: Locator; + readonly activeLabel: Locator; + readonly panel: Locator; + readonly input: Locator; + + constructor(private page: Page) { + this.trigger = page.getByTestId('search-scope-trigger'); + this.activeLabel = page.getByTestId('search-scope-active'); + this.panel = page.getByTestId('search-scope-panel'); + this.input = page.getByTestId('search-input-field'); + } + + /** An option's inner span, reached through its value-named test id. */ + option(scope: DriveSearchScope): Locator { + return this.panel.getByTestId(`search-scope-option-${scope}`); + } + + /** Opens the panel and waits for it to render. */ + async open() { + await this.trigger.click(); + await expect(this.panel).toBeVisible({ timeout: 5000 }); + } + + /** + * Picks a scope through the panel and waits for the choice to land: the panel closes and the + * trigger reads the chosen scope. Choosing re-runs the search (FR-004), so a test that needs + * the request itself arms a capture around this — see + * `ContentDrivePage.captureSearchPayload`. + */ + async choose(scope: DriveSearchScope) { + await this.open(); + await this.option(scope).click(); + await expect(this.panel).toBeHidden({ timeout: 5000 }); + await this.expectActive(scope); + } + + /** The trigger reads the active scope as its label (FR-002). */ + async expectActive(scope: DriveSearchScope) { + await expect(this.activeLabel).toHaveText(SCOPE_LABEL[scope]); + } + + /** The panel marks the active option, so the user can tell which one is on. */ + async expectOptionMarked(scope: DriveSearchScope) { + // The option row is reached by role and label — PrimeNG renders `role="option"` with the + // option's label as its accessible name — and the selection state on `aria-selected`. + await expect(this.optionRow(scope)).toHaveAttribute('aria-selected', 'true'); + } + + /** + * The option's explanation comes up as a tooltip on hover (FR-022). Asserted on a stable + * phrase of the English copy rather than the whole string, so rewording the rest of the + * sentence does not break the test. + */ + async expectOptionExplained(scope: DriveSearchScope, phrase: string) { + await this.option(scope).hover(); + await expect(this.page.locator('.p-tooltip')).toContainText(phrase, { timeout: 5000 }); + } + + /** The listbox option element that carries the option's span. */ + private optionRow(scope: DriveSearchScope): Locator { + return this.panel.getByRole('option', { name: SCOPE_LABEL[scope] }); + } +} diff --git a/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts b/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts index bc8f5b9740e4..3ba982a6fc9f 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts @@ -189,6 +189,19 @@ export interface DotContentDriveQueryFilters { * Text to search for. */ text: string; + + /** + * Which fields {@link text} is matched against. + * + * Sits here rather than at the top level because it qualifies `text` and means nothing without + * it — the same reason {@link filterFolders} lives here. Omit it for the historical behaviour: + * an absent scope is processed exactly as it was before the field existed, which is what keeps + * the AssetPicker unaffected. + * + * Not to be confused with a *browse* scope, which says where you are browsing rather than which + * fields a search reads. + */ + searchScope?: 'TITLE' | 'ALL_FIELDS'; } /** 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..a05adc23666b --- /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,84 @@ + + + + + + + + + + + + + + + + + {{ item.label }} + + + + 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 c4cd5134b188..33b10773a88e 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 @@ -1,4 +1,5 @@ import { + byTestId, createComponentFactory, mockProvider, Spectator, @@ -8,6 +9,7 @@ import { vi } from 'vitest'; import { By } from '@angular/platform-browser'; +import { Tooltip } from 'primeng/tooltip'; import { ZIndexUtils } from 'primeng/utils'; import { DotMessageService } from '@dotcms/data-access'; @@ -28,7 +30,8 @@ describe('DotContentDriveSearchInputComponent', () => { mockProvider(DotContentDriveStore, { getFilterValue: vi.fn().mockReturnValue(undefined), setGlobalSearch: vi.fn(), - selectRootNode: vi.fn() + selectRootNode: vi.fn(), + setSearchScope: vi.fn() }), { provide: DotMessageService, @@ -90,6 +93,147 @@ describe('DotContentDriveSearchInputComponent', () => { // The claim lives here rather than in the shell because this component is the one holding the // search box; the shell would have to reach three levels down to find it. + describe('search scope', () => { + // Absent from the filters means the default. The key is deliberately NOT `title` — that + // holds the search TERM, and a scope whose value is 'TITLE' beside it would be a collision. + const withScope = (scope?: string) => + store.getFilterValue.mockImplementation((key: string) => + key === 'searchScope' ? scope : undefined + ); + + it('should render the scope control next to the search input', () => { + spectator.detectChanges(); + + expect(spectator.query(byTestId('search-scope-trigger'))).toBeTruthy(); + expect(searchInput()).toBeTruthy(); + }); + + it('should show the active scope on the trigger', () => { + withScope('TITLE'); + spectator.detectChanges(); + + expect(spectator.component['$activeScopeLabel']()).toBe( + 'content-drive.search.scope.title' + ); + }); + + it('should start on All Fields when nothing is stored', () => { + withScope(undefined); + spectator.detectChanges(); + + expect(spectator.component['$searchScope']()).toBe('ALL_FIELDS'); + }); + + it('should read the stored scope when one is set', () => { + withScope('TITLE'); + spectator.detectChanges(); + + expect(spectator.component['$searchScope']()).toBe('TITLE'); + }); + + // The placeholder deliberately does NOT describe the active scope — it is always the + // shared component's own default ("Search"), so no [placeholder] binding is passed at all. + // Regression guard for both directions: scope changes must not reintroduce one. + it('should leave the input on its default placeholder in Title scope', () => { + withScope('TITLE'); + spectator.detectChanges(); + + expect(searchInput().componentInstance.$placeholder()).toBe('search'); + }); + + it('should leave the input on its default placeholder in All Fields scope', () => { + withScope(undefined); + spectator.detectChanges(); + + expect(searchInput().componentInstance.$placeholder()).toBe('search'); + }); + + it('should record a newly chosen scope', () => { + withScope(undefined); + spectator.detectChanges(); + + spectator.component['onScopeChange']('TITLE'); + + expect(store.setSearchScope).toHaveBeenCalledWith('TITLE'); + }); + + it('should ignore re-selecting the scope that is already active', () => { + withScope('TITLE'); + spectator.detectChanges(); + + spectator.component['onScopeChange']('TITLE'); + + // Re-running cannot change the results, and it would reset the user to page 1. + expect(store.setSearchScope).not.toHaveBeenCalled(); + }); + + // p-listbox is single-select with metaKeySelection=false, so re-clicking the already + // active option TOGGLES it and emits `null` via (ngModelChange) instead of the option's + // value. Before the null guard, that null slipped past the "already active" check (`null + // !== 'TITLE'`) and reached the store as a real scope change — silently dropping the user + // back to All Fields and marking a clean drive as filtered (issue #37554 review). + it('should ignore a null emission from re-clicking the active scope in the listbox', () => { + withScope('TITLE'); + spectator.detectChanges(); + + spectator.component['onScopeChange'](null); + + expect(store.setSearchScope).not.toHaveBeenCalled(); + }); + + it('should name the control for assistive technology', () => { + spectator.detectChanges(); + + expect( + spectator.query(byTestId('search-scope-trigger'))?.getAttribute('aria-label') + ).toBeTruthy(); + }); + + // Lara centers a button's content, so label and chevron recentered as one group every time + // the active scope changed — "Title" and "All Fields" rendered at different offsets. + // `TRIGGER_PT` pins them to the button's edges instead; asserted through the rendered + // inline style, so a lost PT slot fails here rather than in QA. + it('should keep the trigger label and chevron in place when the scope changes', () => { + spectator.detectChanges(); + + const trigger = spectator.query(byTestId('search-scope-trigger')) as HTMLElement; + + expect(trigger.style.justifyContent).toBe('space-between'); + }); + + // The gray of `p-button-secondary` must go white through the PT inline style, not a + // Tailwind class: PrimeNG's styles are appended to the head after the Tailwind stylesheet, + // so `bg-white` loses the cascade war at equal specificity. The value is the input's own + // token variable, so both halves of the field stay the same white in any theme. + it('should paint the trigger with the field background', () => { + spectator.detectChanges(); + + const trigger = spectator.query(byTestId('search-scope-trigger')) as HTMLElement; + + expect(trigger.style.background).toBe('var(--p-inputtext-background)'); + }); + + it('should offer a distinct explanation for each option in the panel', () => { + spectator.detectChanges(); + + // Two labels do not carry the distinction between "the item's name" and "anything + // written inside it", and the control is new. The explanation lives on each option + // row in the panel, not on the trigger — asserted through the directive instances + // rather than an ng-reflect attribute, which Angular only emits in development mode. + spectator.click(byTestId('search-scope-trigger')); + spectator.detectChanges(); + + const tooltips = spectator.queryAll(Tooltip); + + expect(tooltips.length).toBe(2); + expect(tooltips.every((tooltip) => !!tooltip.content)).toBe(true); + + const contents = tooltips.map((tooltip) => tooltip.content); + + expect(new Set(contents).size).toBe(2); + }); + }); + describe('search shortcut', () => { /** Dispatches from `target` (the document unless a field is given), the way a browser would. */ const press = (init: KeyboardEventInit, target: EventTarget = document): KeyboardEvent => { 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 2aff99ad324c..8590767391df 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 @@ -6,14 +6,32 @@ import { OnDestroy, viewChild } from '@angular/core'; +import { FormsModule } from '@angular/forms'; +import { ButtonModule } from 'primeng/button'; +import { InputGroupModule } from 'primeng/inputgroup'; +import { InputGroupAddonModule } from 'primeng/inputgroupaddon'; +import { ListboxModule } from 'primeng/listbox'; +import { PopoverModule } from 'primeng/popover'; +import { TooltipModule } from 'primeng/tooltip'; + +import { DotMessageService } from '@dotcms/data-access'; import { + CHIP_FILTER_LISTBOX_PT, + CHIP_FILTER_POPOVER_PT, + CHIP_FILTER_SCROLL_HEIGHT, DotKeyboardShortcutService, DotKeyboardShortcutUnregister, + DotMessagePipe, DotSearchInputComponent, hasOverlayAbove } from '@dotcms/ui'; +import { DEFAULT_SEARCH_SCOPE, SEARCH_SCOPE_FILTER_KEY } from '../../../../shared/constants'; +import { + DOT_CONTENT_DRIVE_SEARCH_SCOPE, + DotContentDriveSearchScope +} from '../../../../shared/models'; import { DotContentDriveStore } from '../../../../store/dot-content-drive.store'; /** @@ -23,17 +41,25 @@ import { DotContentDriveStore } from '../../../../store/dot-content-drive.store' */ @Component({ selector: 'dot-content-drive-search-input', - template: ` - - - `, + templateUrl: './dot-content-drive-search-input.component.html', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [DotSearchInputComponent], + imports: [ + DotSearchInputComponent, + ButtonModule, + InputGroupModule, + InputGroupAddonModule, + ListboxModule, + PopoverModule, + TooltipModule, + DotMessagePipe, + FormsModule + ], host: { class: 'w-full' } }) export class DotContentDriveSearchInputComponent implements OnDestroy { readonly #store = inject(DotContentDriveStore); readonly #shortcuts = inject(DotKeyboardShortcutService); + readonly #messageService = inject(DotMessageService); /** Withdrawals for the claims made in the constructor, released in {@link ngOnDestroy}. */ #withdrawShortcuts: DotKeyboardShortcutUnregister[] = []; @@ -47,6 +73,108 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { () => (this.#store.getFilterValue('title') as string) ?? '' ); + /** + * The two options, in the order they read best: the narrow one first, because it is the choice + * the user is here to make. The wide one is called "All Fields" rather than "All Content" — it + * widens which FIELDS are read, not which content is searched, and Content Drive is separately + * gaining a browse scope where "All" genuinely means all content. + * + * `label` is resolved here, eagerly, rather than left as a raw i18n key for the item template + * to pipe through `| dm`. `p-listbox` puts its own `[attr.aria-label]` on each option element + * from `getOptionLabel(option)`, which falls back to the raw `option.label` — it does not go + * through this component's template, so a piped label left a screen reader announcing the key + * itself (e.g. "content-drive.search.scope.title") instead of the translated text. + */ + protected readonly scopeOptions = [ + { + label: this.#messageService.get('content-drive.search.scope.title'), + help: 'content-drive.search.scope.title.help', + value: DOT_CONTENT_DRIVE_SEARCH_SCOPE.TITLE + }, + { + label: this.#messageService.get('content-drive.search.scope.all-fields'), + help: 'content-drive.search.scope.all-fields.help', + value: DOT_CONTENT_DRIVE_SEARCH_SCOPE.ALL_FIELDS + } + ]; + + /** + * 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 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( + () => + (this.#store.getFilterValue(SEARCH_SCOPE_FILTER_KEY) as DotContentDriveSearchScope) ?? + DEFAULT_SEARCH_SCOPE + ); + + /** + * The trigger shows the active scope, as the mock does. Already-translated text — see the + * comment on `scopeOptions` for why the label is resolved there rather than piped through + * `| dm` in the template. + */ + protected readonly $activeScopeLabel = computed( + () => + this.scopeOptions.find((option) => option.value === this.$searchScope())?.label ?? + this.#messageService.get('content-drive.search.scope.all-fields') + ); + + /** + * The trigger's own border removed and its background set to the field's white, both via PT + * rather than Tailwind classes, and its content pinned to the edges so the active scope reads + * from the same place no matter which scope is active. + * + * `pButtonPT`'s `root.style` is consumed through `[style]`/`[class]` HOST BINDINGS on the + * directive's own host element (see `Bind`, the directive backing this), which Angular applies + * the same way any `[style]` binding is — as a real inline style. That wins the cascade over + * PrimeNG's own injected `.p-button-secondary` rule unconditionally, the same guarantee + * `!important` gives, without reaching for it. A plain `bg-white` class cannot: PrimeNG's + * styles are appended to the head at runtime, after the Tailwind stylesheet, so at equal + * specificity the injected rule wins — which is why the border went through PT first and the + * background follows it. + * + * The background value is the same design token the neighboring input paints with — + * `inputtext.background`, read as the CSS variable PrimeNG emits for every token — so the two + * halves of the field stay the same white in any theme, including dark mode, and a restyling + * of the input re-whites the trigger for free. As a side effect the hover recolor + * `.p-button-secondary` would apply is also beaten by the inline style, which is what a + * dropdown trigger wants: the surface does not change under the pointer, the way `p-select` + * behaves. + * + * The last entry exists because Lara centers a button's content (`justify-content: center`, + * hardcoded in the injected stylesheet — no design token exposes it). Centered, the label and + * the chevron are one group that recenters itself as its width changes, so "Title" and + * "All Fields" rendered with both at different offsets. `space-between` — the layout PrimeNG's + * own `p-select` trigger uses — parks the label on the left edge and the chevron on the right + * edge of the fixed-width button, so switching scopes moves neither. + */ + protected readonly TRIGGER_PT = { + root: { + style: { + border: 'none', + background: 'var(--p-inputtext-background)', + justifyContent: 'space-between' + } + } + }; + + /** + * Squares off the search input's connecting edge — the side that meets the scope addon. This + * component's own host sits between `p-inputgroup` and the real ``, breaking PrimeNG's + * structural CSS, so the override rides the design token through `inputDt` instead of + * competing for specificity (see the template comment). Hoisted like `TRIGGER_PT`: an inline + * object literal would be recreated on every change detection cycle. + */ + protected readonly INPUT_DT = { + border: { radius: '{form.field.border.radius} 0 0 {form.field.border.radius}' } + }; + /** * Claims the search shortcuts for as long as this box is on screen. * @@ -116,4 +244,28 @@ export class DotContentDriveSearchInputComponent implements OnDestroy { this.#store.setGlobalSearch(term); this.#store.selectRootNode(); } + + /** + * Records the new scope and lets the store re-run the search. + * + * Re-selecting the scope that is already active is ignored: the results cannot change, and + * `patchFilters` would reset the user to page 1 for nothing. + * + * The `null` guard is load-bearing, not defensive filler. `p-listbox` is single-select with + * `metaKeySelection` at its default of `false`, which makes it a TOGGLE: re-clicking the + * option that is already selected emits `null` instead of the option's value + * (`onOptionSelectSingle` in `primeng/listbox`). Without the guard that `null` reaches + * `setSearchScope`, which only special-cases the real default value — `null` is neither that + * nor the current scope, so it gets written into the filters as `searchScope: null`. That + * silently flips the active search to All Fields and lights up "Clear all" on a drive with + * nothing filtered, and the bad value can then never be reselected away because + * `$searchScope()` already reads back as a real scope. + */ + protected onScopeChange(scope: DotContentDriveSearchScope | null): void { + if (scope == null || scope === this.$searchScope()) { + return; + } + + this.#store.setSearchScope(scope); + } } 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 957e4812ddee..2011fdf52b50 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,9 +37,29 @@ (uploadFiles)="onRequestUpload($event)" (dragEnter)="onDropzoneDragEnter()" class="col-start-2 row-start-3 overflow-auto"> - + + @if ($searchFailed()) { + +
+ {{ 'content-drive.search.error.message' | dm }} + +
+ + } + { expect(location.go).not.toHaveBeenCalled(); }); + // The search scope rides the generic filter machinery rather than a mechanism of its own, + // which is what makes it survive reload, Back/Forward and a shared link for free. These pin + // that it actually reaches the address, and that the default never does. + it('should carry a non-default search scope into the address', () => { + store.isTreeExpanded.mockReturnValue(false); + store.path.mockReturnValue('/'); + filtersSignal.set({ title: 'pricing', searchScope: 'TITLE' }); + spectator.detectChanges(); + + expect(location.replaceState).toHaveBeenCalledWith( + expect.stringContaining('searchScope%3ATITLE') + ); + }); + + it('should keep the default search scope out of the address', () => { + store.isTreeExpanded.mockReturnValue(false); + store.path.mockReturnValue('/'); + // The store removes the key rather than storing the default, so the address stays as + // clean as it would have been had the control never been touched. + filtersSignal.set({ title: 'pricing' }); + spectator.detectChanges(); + + expect(location.replaceState).not.toHaveBeenCalledWith( + expect.stringContaining('searchScope') + ); + }); + it('pushes a history entry when the user navigates to a different folder', () => { // Folder navigation is a real user action, so Back must step back up the tree. Only the // automatic filter seed is denied an entry. @@ -1581,6 +1608,57 @@ describe('DotContentDriveShellComponent', () => { }); }); + // 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