diff --git a/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.spec.ts b/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.spec.ts index a184c4e81aef..ca2fefb35c58 100644 --- a/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.spec.ts +++ b/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.spec.ts @@ -11,13 +11,17 @@ import { Mock, MockInstance, vi } from 'vitest'; import { Location } from '@angular/common'; import { ActivatedRoute, convertToParamMap } from '@angular/router'; +import { MessageService } from 'primeng/api'; + import { + buildPersistedQueryKey, DotContentletEditUrlService, DotContentSearchService, DotCurrentUserService, DotGlobalMessageService, DotHttpErrorManagerService, - DotMessageService + DotMessageService, + DotPropertiesService } from '@dotcms/data-access'; import { ComponentStatus, FeaturedFlags } from '@dotcms/dotcms-models'; import { DotEditContentSidePanelComponent } from '@dotcms/edit-content'; @@ -198,6 +202,22 @@ describe('DotQueryToolPageComponent', () => { const store = spectator.inject(DotQueryToolStore, true); expect(store.runSearch).not.toHaveBeenCalled(); }); + + it('leaves the store query untouched when the URL carries no q', () => { + // The store hydrates `query` from localStorage in its own onInit, which runs before + // ngOnInit. Writing the (absent) `q` param over it wipes the restored query from the + // UI. Storage itself survives — the persistence pipeline's `skip(1)` absorbs the stray + // '' — so the damage is confined to the editor the user is looking at. + setup({}); + const store = spectator.inject(DotQueryToolStore, true); + expect(store.setQuery).not.toHaveBeenCalled(); + }); + + it('applies an explicitly empty q, so a shared ?q= link still clears the editor', () => { + setup({ q: '' }); + const store = spectator.inject(DotQueryToolStore, true); + expect(store.setQuery).toHaveBeenCalledWith(''); + }); }); describe('Submit button', () => { @@ -715,3 +735,117 @@ describe('DotQueryToolPageComponent (editContent deep link)', () => { expect(spectator.component.$editPanelRequest()).toBeNull(); }); }); + +// Sibling top-level describe with the REAL store (every other describe here mocks it, so the +// store's own `onInit` never runs and the ngOnInit/hydration ordering can't be observed). This is +// the QA-reported path: type a query, leave the portlet, come back, query is gone. +describe('DotQueryToolPageComponent (persisted query, real store)', () => { + let spectator: Spectator; + const persistedKey = buildPersistedQueryKey('query-tool'); + + const createComponent = createComponentFactory({ + component: DotQueryToolPageComponent, + overrideComponents: [ + [ + DotQueryToolPageComponent, + { + remove: { imports: [DotEditContentSidePanelComponent] }, + add: { imports: [MockComponent(DotEditContentSidePanelComponent)] } + } + ] + ], + providers: [ + mockProvider(DotMessageService, { get: vi.fn().mockReturnValue('') }), + mockProvider(DotHttpErrorManagerService), + mockProvider(DotGlobalMessageService, { error: vi.fn() }), + mockProvider(DotQueryToolService, { + search: vi + .fn() + .mockReturnValue(of({ resultsSize: 0, jsonObjectView: { contentlets: [] } })) + }), + mockProvider(DotContentletEditUrlService, { resolveEditUrl: vi.fn() }), + mockProvider(DotContentSearchService, { + get: vi.fn().mockReturnValue(of({ jsonObjectView: { contentlets: [] } })) + }), + mockProvider(DotPropertiesService, { getFeatureFlags: vi.fn().mockReturnValue(of({})) }) + ], + // Spectator REPLACES the component's own `providers` with this list, so the real store has + // to be named here — that is the whole point of this describe. + componentProviders: [ + DotQueryToolStore, + mockProvider(DotCurrentUserService, { + getCurrentUser: vi.fn().mockReturnValue(of({ admin: true })) + }), + // The store is built by the component injector, so `withFlags`' DotPropertiesService + // has to be reachable from here rather than only at the TestBed root. + mockProvider(DotPropertiesService, { + getFeatureFlags: vi.fn().mockReturnValue(of({})) + }), + DotClipboardUtil, + MessageService + ] + }); + + const mount = (params: Record = {}) => + createComponent({ + providers: [ + { + provide: ActivatedRoute, + useValue: { snapshot: { queryParamMap: convertToParamMap(params) } } + }, + { + provide: Location, + useValue: { + replaceState: vi.fn(), + go: vi.fn(), + path: vi.fn().mockReturnValue(''), + subscribe: vi.fn().mockReturnValue({ unsubscribe: vi.fn() }) + } + } + ] + }); + + beforeEach(() => { + vi.useFakeTimers(); + window.localStorage.clear(); + }); + + afterEach(() => { + vi.useRealTimers(); + window.localStorage.clear(); + }); + + it('restores the persisted query when the portlet is reopened without params', () => { + window.localStorage.setItem(persistedKey, JSON.stringify('+contentType:Blog')); + + spectator = mount(); + + expect(spectator.component.store.query()).toBe('+contentType:Blog'); + }); + + it('never erases the stored entry on reopen, whatever the restore path does', () => { + // NOT a regression test for the ngOnInit guard — this passes with that guard reverted, + // because the persistence pipeline's `skip(1)` absorbs the stray '' before it can reach + // `removeKey`. It guards the invariant underneath: reopening the portlet is a read, and + // must never destroy the saved query. That `skip(1)` is the only thing standing between + // a stray write-on-init and silent data loss, so it is worth pinning on its own. + window.localStorage.setItem(persistedKey, JSON.stringify('+contentType:Blog')); + + spectator = mount(); + // flushEffects() pushes the field signal through the rxMethod effect; only then does the + // debounce timer exist for the fake clock to advance. Skipping it is what makes fake + // timers look like they do not drive this pipeline. + spectator.flushEffects(); + vi.advanceTimersByTime(500); + + expect(window.localStorage.getItem(persistedKey)).toBe(JSON.stringify('+contentType:Blog')); + }); + + it('lets a ?q= deep link win over the persisted query', () => { + window.localStorage.setItem(persistedKey, JSON.stringify('+contentType:Blog')); + + spectator = mount({ q: '+live:true' }); + + expect(spectator.component.store.query()).toBe('+live:true'); + }); +}); diff --git a/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.ts b/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.ts index 02d305733fbd..0699393bcb37 100644 --- a/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.ts +++ b/core-web/libs/portlets/dot-query-tool/src/lib/dot-query-tool-page/dot-query-tool-page.component.ts @@ -269,12 +269,23 @@ export class DotQueryToolPageComponent implements OnInit { const sort = params.get('sort') ?? ''; const userId = params.get('userId') ?? ''; - this.store.setQuery(query); + // Only write `query` when the URL actually carries `q`. The store's `withPersistedQuery` + // restored the user's last query in its own onInit, which runs before this — writing the + // absent param's '' over it is what made Query Tool lose the query on every return to the + // portlet while ES Search and Velocity Playground kept theirs (neither reads URL params). + // `has` rather than a truthiness check, so an explicit `?q=` in a shared link still clears + // the editor instead of silently resurrecting the previous query. + if (params.has('q')) { + this.store.setQuery(query); + } + this.store.setOffset(offset); this.store.setLimit(limit); this.store.setSort(sort); this.store.setUserId(userId); + // Auto-run only for a deep link. A restored query is left in the editor unexecuted, as in + // the sibling dev tools — returning to the portlet should not fire a search by itself. if (query.trim()) { this.store.runSearch(); } diff --git a/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.spec.ts b/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.spec.ts index a5305140f01f..8874cf8ce4a7 100644 --- a/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.spec.ts +++ b/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.spec.ts @@ -143,6 +143,22 @@ describe('dot-velocity-playground.utils', () => { expect(ex.code).toBeTruthy(); } }); + + it('reads file size off the binary field, not the contentlet', () => { + // `$f.fileSize` is not a contentlet property: it resolved to null on every pulled + // file, so running this example greeted the user with five NULL_METHOD_RESULT + // warnings and printed the literal `$f.fileSize` in the output. Size is a + // metadata-backed getter on FileAsset, reachable through the `fileAsset` binary + // field. (`fileName` and `mimeType` DO resolve on the contentlet — only size does + // not, which is exactly what made the mistake easy to miss.) + const fileExample = VELOCITY_HELP_EXAMPLES.find((ex) => + ex.code.includes('+contentType:FileAsset') + ); + + expect(fileExample).toBeDefined(); + expect(fileExample?.code).toContain('$f.fileAsset.fileSize'); + expect(fileExample?.code).not.toMatch(/\$f\.fileSize/); + }); }); describe('constants surface', () => { diff --git a/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.ts b/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.ts index 654b1cd857d7..ec80882a590a 100644 --- a/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.ts +++ b/core-web/libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.ts @@ -258,6 +258,13 @@ export const formatWarnings = (warnings: VelocityWarning[]): string => { /** * Static catalog of example snippets shown in the help popover. Titles and * descriptions are i18n keys resolved with DotMessagePipe at render time. + * + * Every reference here must resolve against a real dotCMS instance: the VTL + * endpoint reports unresolved ones back as NULL_METHOD_RESULT warnings, and an + * example that greets the user with a warning banner reads as a broken tool. + * Notably, file size lives on the binary field (`$f.fileAsset.fileSize`, a + * metadata-backed getter on FileAsset) — `$f.fileSize` is NOT a contentlet + * property and resolves to null, unlike `fileName` and `mimeType`. */ export const VELOCITY_HELP_EXAMPLES: VelocityHelpExample[] = [ { @@ -278,6 +285,6 @@ export const VELOCITY_HELP_EXAMPLES: VelocityHelpExample[] = [ { title: 'velocityPlayground.help.example.pullFiles', description: 'velocityPlayground.help.example.pullFiles.desc', - code: '#set($files = $dotcontent.pull("+contentType:FileAsset +live:true +conhost:$host.identifier", 5, "modDate desc"))\n#foreach($f in $files)\n - $f.fileName ($f.fileSize bytes, $f.mimeType)\n#end' + code: '#set($files = $dotcontent.pull("+contentType:FileAsset +live:true +conhost:$host.identifier", 5, "modDate desc"))\n#foreach($f in $files)\n - $f.fileName ($f.fileAsset.fileSize bytes, $f.mimeType)\n#end' } ];