fix(dev-tools): restore Query Tool's persisted query + clear FileAsset example warnings (#36692)

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<DotQueryToolPageComponent>;
+    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<string, string> = {}) =>
+        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');
+    });
+});

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();
         }

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', () => {

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'
     }
 ];