fix(signage-manager): fix shared list, decode and picker behaviour

apps/signage-manager/USER_STORIES.md

@@ -236,6 +236,7 @@ These stories cover the current app workflows:
 - Users with update permission can add or remove playlists from the zone.
 - The display tab shows displays assigned to the zone.
 - Users with update permission can add displays to the zone.
+- When the display or zone search in an add dialog fails, the dialog shows an error with a retry, not an empty result.
 
 ---
 
@@ -272,6 +273,7 @@ These stories cover the current app workflows:
 - The current time indicator appears when the selected date is today.
 - Users can search schedules by display, zone, playlist, and source labels where applicable.
 - Timeline rows link to the related display or zone detail page.
+- Display rows show the same online, offline, or never seen status as the displays page.
 - Empty and filtered states explain when no rows are available.
 - Users must confirm a change that makes two takeover playlists play at the same time on a display in the next 14 days. The check runs when users save playlist schedules and when they assign a playlist to a display or zone. The warning names the display, the other playlist, and the start time of the overlap.
 

apps/signage-manager/src/app/shared/command-palette.component.ts

@@ -186,8 +186,12 @@ export class CommandPaletteComponent {
                 detail: '',
                 select: () => this._open([item.route]),
             }));
-        // Keep old matches out once the term is cleared
-        const matches = term ? this._matches.value() : undefined;
+        // Keep out matches for an older term, such as while the new term
+        // waits for its debounce, so Enter cannot open a stale match
+        const current =
+            !!term &&
+            term === this._search_debounced.value().trim().toLowerCase();
+        const matches = current ? this._matches.value() : undefined;
         if (!matches) return pages;
         return [
             ...pages,

apps/signage-manager/src/app/shared/command-palette.service.ts

@@ -7,12 +7,12 @@ import {
     querySignageMedia,
     querySignagePlaylists,
     querySignageTemplates,
-    querySystems,
     queryZones,
     SignageMedia,
     SignagePlaylist,
     SignageTemplate,
 } from '@placeos/ts-client';
+import { querySignageDisplays } from '../displays/signage-display';
 import { SignageContextService } from '../signage-context.service';
 import { searchParam } from '../signage-service.util';
 import { decodeEntityNames } from './decode-entity-names.util';
@@ -90,16 +90,12 @@ export class CommandPaletteService {
         };
         const [displays, playlists, templates, zones, media] =
             await Promise.all([
-                settle<PlaceSystem>(
-                    querySystems({ ...params, signage: true } as any),
-                ),
+                settle(querySignageDisplays({ ...params, signage: true })),
                 settle(querySignagePlaylists(params)),
                 this._context.templates_enabled()
                     ? settle(querySignageTemplates(group_params))
                     : Promise.resolve([] as SignageTemplate[]),
-                settle<PlaceZone>(
-                    queryZones({ ...group_params, tags: 'signage' } as any),
-                ),
+                settle(queryZones({ ...group_params, tags: 'signage' })),
                 settle(querySignageMedia(params)),
             ]);
         return { displays, playlists, templates, zones, media };

apps/signage-manager/src/app/shared/decode-entity-names.util.ts

@@ -16,21 +16,32 @@ export function decodeEntities(value: string): string {
 
 const NAME_FIELDS = ['name', 'display_name'];
 const NESTED_FIELDS = ['group', 'user', 'zone'];
+// Copies this module made. Some items pass the data boundary twice, such as
+// displays from `querySignageDisplays` read through `queryAll`, and a second
+// decode would turn a saved `&` into `&`.
+const _decoded = new WeakSet<object>();
 
 /** Returns a shallow copy of `item` with name fields (and one nested level of
- * group/user/zone) HTML-entity decoded. Idempotent for already-decoded values. */
+ * group/user/zone) HTML-entity decoded. The copy keeps the prototype of
+ * `item`, so getters such as `SignageMedia.media_url` still work. An item
+ * this function returned is returned as is, so names decode only once. */
 export function decodeEntityNames<T>(item: T): T {
-    if (!item || typeof item !== 'object') return item;
-    const copy: any = { ...item };
+    if (!item || typeof item !== 'object' || _decoded.has(item)) return item;
+    const copy: T = Object.assign(
+        Object.create(Object.getPrototypeOf(item)),
+        item,
+    );
+    const fields = copy as Record<string, unknown>;
     for (const field of NAME_FIELDS) {
-        if (typeof copy[field] === 'string') {
-            copy[field] = decodeEntities(copy[field]);
-        }
+        const value = fields[field];
+        if (typeof value === 'string') fields[field] = decodeEntities(value);
     }
     for (const field of NESTED_FIELDS) {
-        if (copy[field] && typeof copy[field] === 'object') {
-            copy[field] = decodeEntityNames(copy[field]);
+        const value = fields[field];
+        if (value && typeof value === 'object') {
+            fields[field] = decodeEntityNames(value);
         }
     }
+    _decoded.add(fields);
     return copy;
 }

apps/signage-manager/src/app/shared/display-select-modal.component.ts

@@ -4,7 +4,11 @@ import { MatRippleModule } from '@angular/material/core';
 import { MatDialogModule } from '@angular/material/dialog';
 import { MatFormFieldModule } from '@angular/material/form-field';
 import { MatInputModule } from '@angular/material/input';
-import { IconComponent, TranslatePipe } from '@placeos/components';
+import {
+    IconComponent,
+    LoadErrorComponent,
+    TranslatePipe,
+} from '@placeos/components';
 import { PlaceSystem } from '@placeos/ts-client';
 import { SignageDisplayService } from '../displays/signage-display.service';
 import { IntersectDirective } from './intersect.directive';
@@ -81,6 +85,8 @@ import { byDisplayName, PagedSearch } from './paged-search';
                         intersect
                         (intersect)="list.loadMore()"
                     ></div>
+                } @else if (list.error()) {
+                    <load-error (retry)="list.retry()" />
                 }
             } @else if (list.loading()) {
                 <div
@@ -90,6 +96,8 @@ import { byDisplayName, PagedSearch } from './paged-search';
                         {{ 'COMMON.LOADING' | translate }}
                     </div>
                 </div>
+            } @else if (list.error()) {
+                <load-error (retry)="list.retry()" />
             } @else {
                 <div
                     class="bg-base-200 flex h-[calc(100%-3.5rem)] w-full flex-col items-center justify-center space-y-4 rounded-lg p-16"
@@ -109,6 +117,7 @@ import { byDisplayName, PagedSearch } from './paged-search';
         MatFormFieldModule,
         MatInputModule,
         IconComponent,
+        LoadErrorComponent,
         TranslatePipe,
         IntersectDirective,
     ],

apps/signage-manager/src/app/shared/paged-list.ts

@@ -65,6 +65,9 @@ export class PagedList<T extends { id: string }> {
         this._next = null;
         this._has_more.set(false);
         this._error.set(false);
+        // A page of the old query may still be in flight. It will not clear
+        // the flag, as its token is stale.
+        this._loading.set(false);
         this._replace = !!query && keep_items;
         if (!this._replace) {
             this._items.set([]);

apps/signage-manager/src/app/shared/paged-search.ts

@@ -15,31 +15,50 @@ export class PagedSearch<T extends { id: string }> {
     public readonly items: Signal<T[]>;
     public readonly loading: Signal<boolean>;
     public readonly has_more: Signal<boolean>;
+    /** Whether the last page failed to load. `retry` loads it again. */
+    public readonly error: Signal<boolean>;
+    // Term of the current query, so a retry can run it again
+    private _term = '';
 
     constructor(
         /** Builds the first page of results, null when the user may not query */
-        query: (search: string) => QueryResponse<T> | null,
+        private readonly _query: (search: string) => QueryResponse<T> | null,
         sort?: (a: T, b: T) => number,
         debounce_ms = 400,
     ) {
         this._list = new PagedList<T>({ sort });
         this.items = this._list.items;
         this.loading = this._list.loading;
         this.has_more = this._list.has_more;
+        this.error = this._list.error;
         const search_debounced = debounced(this.search, debounce_ms);
         effect(() => {
             const term = search_debounced.value();
-            untracked(() => this._list.reset(query(term)));
+            untracked(() => {
+                this._term = term;
+                this._list.reset(this._query(term));
+            });
         });
     }
 
     public loadMore() {
         this._list.loadMore();
     }
+
+    /** Load the page that failed again, or the first page when it failed */
+    public retry() {
+        // A new term waits for its debounce. Its search replaces the failed
+        // one, so a retry now would only show matches for the old term.
+        if (this.search() !== this._term) return;
+        if (!this._list.retry()) this._list.reset(this._query(this._term));
+    }
 }
 
 /** Displays and zones show a display_name in preference to their name */
-export function byDisplayName(a: any, b: any) {
+export function byDisplayName(
+    a: { name: string; display_name?: string },
+    b: { name: string; display_name?: string },
+) {
     return (a.display_name || a.name).localeCompare(b.display_name || b.name);
 }
 

apps/signage-manager/src/app/shared/zone-select-tree.component.ts

@@ -15,7 +15,11 @@ import { MatRippleModule } from '@angular/material/core';
 import { MatFormFieldModule } from '@angular/material/form-field';
 import { MatInputModule } from '@angular/material/input';
 import { MatTooltipModule } from '@angular/material/tooltip';
-import { IconComponent, TranslatePipe } from '@placeos/components';
+import {
+    IconComponent,
+    LoadErrorComponent,
+    TranslatePipe,
+} from '@placeos/components';
 import { PlaceZone } from '@placeos/ts-client';
 import { IntersectDirective } from './intersect.directive';
 import { PagedSearch } from './paged-search';
@@ -203,6 +207,8 @@ interface ZoneSelectTreeNode {
                     intersect
                     (intersect)="list().loadMore()"
                 ></div>
+            } @else if (list().error()) {
+                <load-error (retry)="list().retry()" />
             }
         } @else if (list().loading()) {
             <div
@@ -212,6 +218,8 @@ interface ZoneSelectTreeNode {
                     {{ 'COMMON.LOADING' | translate }}
                 </div>
             </div>
+        } @else if (list().error()) {
+            <load-error (retry)="list().retry()" />
         } @else {
             <div
                 class="bg-base-200 flex h-[calc(100%-3.5rem)] w-full flex-col items-center justify-center space-y-4 rounded-lg p-16"
@@ -244,6 +252,7 @@ interface ZoneSelectTreeNode {
         MatTooltipModule,
         CdkTreeModule,
         IconComponent,
+        LoadErrorComponent,
         TranslatePipe,
         IntersectDirective,
     ],

apps/signage-manager/src/app/signage-inventory.service.ts

@@ -4,12 +4,12 @@ import {
     PlaceZone,
     querySignageMedia,
     querySignagePlaylists,
-    querySystems,
     queryZones,
     showSignageMedia,
     SignageMedia,
     SignagePlaylist,
 } from '@placeos/ts-client';
+import { querySignageDisplays } from './displays/signage-display';
 import {
     findTakeoverConflicts,
     type TakeoverConflict,
@@ -78,18 +78,18 @@ export class SignageInventoryService {
         const limit = PAGE_SIZE;
         const [displays, zones, playlists] = await Promise.all([
             queryAll(
-                querySystems({
+                querySignageDisplays({
                     ...this._context.orgZoneQueryParams({}),
                     limit,
                     signage: true,
-                } as any),
+                }),
             ),
             queryAll(
                 queryZones(
                     this._context.groupQueryParams({
                         limit,
                         tags: 'signage',
-                    }) as any,
+                    }),
                 ),
             ),
             queryAll(

apps/signage-manager/src/tests/shared/command-palette.component.spec.ts

@@ -97,6 +97,20 @@ describe('CommandPaletteComponent', () => {
         ]);
     });
 
+    // Enter before the debounce ends would open a match for the old term
+    it('hides matches for the previous term while the new term waits', async () => {
+        search_all.mockResolvedValue({
+            ...EMPTY,
+            displays: [{ id: 'd1', name: 'SIGNAGE 1', display_name: 'Lobby' }],
+        });
+        const component = make();
+        await type(component, 'lob');
+
+        component.search.set('xyz');
+
+        expect(labels(component)).toEqual([]);
+    });
+
     it('moves the highlight with the arrow keys and wraps', async () => {
         const component = make();
         const key = (key: string) =>

apps/signage-manager/src/tests/shared/decode-entity-names.util.spec.ts

@@ -1,3 +1,4 @@
+import { SignageMedia } from '@placeos/ts-client';
 import {
     decodeEntities,
     decodeEntityNames,
@@ -37,6 +38,24 @@ describe('decodeEntityNames', () => {
         expect(result.user.name).toBe('Tom & Jerry');
     });
 
+    // Edit and preview build the media address from these getters
+    it('keeps the class of the item, so its getters still work', () => {
+        const media = decodeEntityNames(
+            new SignageMedia({ name: 'R&D', media_id: 'upload-1' }),
+        );
+
+        expect(media).toBeInstanceOf(SignageMedia);
+        expect(media.name).toBe('R&D');
+        expect(media.media_url).toBe('/api/engine/v2/uploads/upload-1/url');
+    });
+
+    // Some items pass the data boundary twice, e.g. displays through queryAll
+    it('decodes an item only once', () => {
+        const item = decodeEntityNames({ id: '1', name: 'A & B' });
+
+        expect(decodeEntityNames(item).name).toBe('A & B');
+    });
+
     it('passes through non-objects', () => {
         expect(decodeEntityNames(null as any)).toBeNull();
         expect(decodeEntityNames('x' as any)).toBe('x');

apps/signage-manager/src/tests/shared/display-select-modal.component.spec.ts

@@ -44,3 +44,50 @@ describe('DisplaySelectModalComponent', () => {
         ).toEqual(['d2', 'd1']);
     });
 });
+
+// The real template, so a failed search shows an error and not "no displays"
+describe('DisplaySelectModalComponent errors', () => {
+    const flush = () => new Promise((resolve) => setTimeout(resolve));
+    const queryDisplays = vi.fn();
+
+    beforeEach(async () => {
+        vi.clearAllMocks();
+        await TestBed.configureTestingModule({
+            imports: [DisplaySelectModalComponent],
+            providers: [
+                { provide: MAT_DIALOG_DATA, useValue: {} },
+                { provide: SignageDisplayService, useValue: { queryDisplays } },
+            ],
+        }).compileComponents();
+    });
+
+    it('shows a load error with a retry that queries again', async () => {
+        queryDisplays
+            .mockReturnValueOnce(Promise.reject(new Error('offline')))
+            .mockReturnValueOnce(
+                Promise.resolve({
+                    data: [{ id: 'd1', name: 'Lobby' }],
+                    total: 1,
+                    next: null,
+                }),
+            );
+        const fixture = TestBed.createComponent(DisplaySelectModalComponent);
+        fixture.detectChanges();
+        await flush();
+        fixture.detectChanges();
+
+        const element: HTMLElement = fixture.nativeElement;
+        const retry = element.querySelector<HTMLButtonElement>(
+            'load-error button',
+        );
+        expect(retry).toBeTruthy();
+
+        retry?.click();
+        await flush();
+        fixture.detectChanges();
+
+        expect(queryDisplays).toHaveBeenCalledTimes(2);
+        expect(element.querySelector('load-error')).toBeNull();
+        expect(element.textContent).toContain('Lobby');
+    });
+});