From 62503461b45fd02bde031835c10c3c345c96a9a8 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 2 Oct 2026 13:12:34 +1000 Subject: [PATCH 1/3] fix(signage-manager): fix display and zone selection and states - Load a routed display or zone again after Back, and show an error when it cannot be loaded. - Refresh other views after a display is created, edited or removed. - Show loading and error states with Retry in the display list, zone tree and content tabs. - Show the server total of signage zones in the zone header. - Load the selected display's template mappings once. --- apps/signage-manager/USER_STORIES.md | 4 + .../app/displays/display-content.component.ts | 33 ++++++- .../app/displays/display-list.component.ts | 14 ++- .../displays/display-schedule.component.ts | 29 ++---- .../src/app/displays/displays.component.ts | 80 +++------------- .../src/app/displays/routed-selection.util.ts | 63 ++++++++++++ .../app/displays/signage-display.service.ts | 73 +++++++++++++- .../src/app/zones/signage-zone.service.ts | 66 ++++++++++--- .../src/app/zones/zone-content.component.ts | 33 ++++++- .../src/app/zones/zone-header.component.ts | 9 +- .../src/app/zones/zone-list.component.ts | 25 ++++- .../src/app/zones/zones.component.ts | 51 ++-------- .../display-content.component.spec.ts | 59 ++++++++++++ .../displays/display-list.component.spec.ts | 32 +++++++ .../display-schedule.component.spec.ts | 31 +++--- .../tests/displays/displays.component.spec.ts | 92 ++++++++++-------- .../displays/signage-display.service.spec.ts | 95 ++++++++++++++++++- .../src/tests/signage-display-search.spec.ts | 18 ++++ .../src/tests/signage-displays-zones.spec.ts | 5 +- .../src/tests/signage-zone-search.spec.ts | 39 ++++++++ .../zones/zone-content.component.spec.ts | 61 ++++++++++++ .../tests/zones/zone-header.component.spec.ts | 26 ++++- .../tests/zones/zone-list.component.spec.ts | 56 ++++++++--- .../src/tests/zones/zones.component.spec.ts | 22 +++++ 24 files changed, 782 insertions(+), 234 deletions(-) create mode 100644 apps/signage-manager/src/app/displays/routed-selection.util.ts diff --git a/apps/signage-manager/USER_STORIES.md b/apps/signage-manager/USER_STORIES.md index 4bfbdebcdc6..e5e55d72d83 100644 --- a/apps/signage-manager/USER_STORIES.md +++ b/apps/signage-manager/USER_STORIES.md @@ -228,6 +228,7 @@ These stories cover the current app workflows: **Acceptance Criteria:** - The zones page lists signage zones and supports direct routes to a selected zone. +- The header count shows the number of signage zones. When users search in a zone, it shows the number of results. - System administrators and signage group managers can create, edit, and delete signage zones. - New and edited signage zones keep the `signage` tag and require a parent from the active group's accessible zone tree. - Zone management controls are not shown for untagged parent zones in the tree. @@ -237,6 +238,7 @@ These stories cover the current app workflows: - 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. +- While the zone tree or a tab loads, it shows a loading state. If the zone tree or the display tab cannot load, it shows an error with a retry button. --- @@ -255,6 +257,8 @@ These stories cover the current app workflows: - The playlist tab shows playlists assigned directly to the display and their status. - Users with update permission can add or remove direct playlist assignments. - The zone tab shows zones assigned to the display. +- While the display list or a tab loads, it shows a loading state. If the list or the zone tab cannot load, it shows an error with a retry button. +- If a link opens a display or zone that cannot load, an error message shows. --- diff --git a/apps/signage-manager/src/app/displays/display-content.component.ts b/apps/signage-manager/src/app/displays/display-content.component.ts index 7000618be12..28e703545a7 100644 --- a/apps/signage-manager/src/app/displays/display-content.component.ts +++ b/apps/signage-manager/src/app/displays/display-content.component.ts @@ -2,7 +2,11 @@ import { Component, computed, inject, input } from '@angular/core'; import { MatRippleModule } from '@angular/material/core'; import { MatTooltipModule } from '@angular/material/tooltip'; import { RouterLink } from '@angular/router'; -import { IconComponent, TranslatePipe } from '@placeos/components'; +import { + IconComponent, + LoadErrorComponent, + TranslatePipe, +} from '@placeos/components'; import { SignagePlaylist } from '@placeos/ts-client'; import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { PlaylistThumbnailComponent } from '../shared/playlist-thumbnail.component'; @@ -215,6 +219,13 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; } } + } @else if (playlists_loading()) { +
+ {{ 'COMMON.LOADING' | translate }} +
} @else {
} + } @else if (display_zones_loading()) { +
+ {{ 'COMMON.LOADING' | translate }} +
+ } @else if (display_zones_error()) { + } @else {
+ } @else if (error()) { + } @else if (!loading()) {
{{ 'COMMON.END_OF_LIST' | translate }} @@ -131,6 +133,8 @@ import { SignageDisplayService } from './signage-display.service'; > {{ 'COMMON.LOADING' | translate }}
+ } @else if (error()) { + } @else {
- this._context.templates_enabled() - ? this.selected_display()?.id - : undefined, - loader: ({ params }) => - this._template_service.listTemplateMappings({ - control_system_id: params, - }), - }); - public readonly templates_loading = this._template_mappings.isLoading; - public readonly templates_error = this._template_mappings.error; - public readonly template_mappings = computed(() => - this._template_mappings.hasValue() - ? this._template_mappings.value() - : [], - ); + public readonly templates_loading = + this._display_service.selected_display_template_mappings_loading; + public readonly templates_error = + this._display_service.selected_display_template_mappings_error; + public readonly template_mappings = + this._display_service.selected_display_template_mappings; public readonly day_blocks = computed(() => buildDisplayScheduleDays( diff --git a/apps/signage-manager/src/app/displays/displays.component.ts b/apps/signage-manager/src/app/displays/displays.component.ts index dfd2f263813..bc104b82968 100644 --- a/apps/signage-manager/src/app/displays/displays.component.ts +++ b/apps/signage-manager/src/app/displays/displays.component.ts @@ -4,9 +4,7 @@ import { effect, inject, input, - resource, signal, - untracked, } from '@angular/core'; import { MatRippleModule } from '@angular/material/core'; import { MatTooltipModule } from '@angular/material/tooltip'; @@ -17,10 +15,10 @@ import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { NavFooterComponent } from '../shared/nav-footer.component'; import { NavSidebarComponent } from '../shared/nav-sidebar.component'; import { SignageContextService } from '../signage-context.service'; -import { SignageTemplateService } from '../templates/signage-template.service'; import { DisplayContentComponent } from './display-content.component'; import { DisplayHeaderComponent } from './display-header.component'; import { DisplayListComponent } from './display-list.component'; +import { selectRoutedItem } from './routed-selection.util'; import { showSignageDisplay } from './signage-display'; import { SignageDisplayService } from './signage-display.service'; @@ -339,7 +337,6 @@ export class DisplaysSectionComponent { private readonly _context = inject(SignageContextService); private readonly _display_service = inject(SignageDisplayService); private readonly _playlist_service = inject(SignagePlaylistService); - private readonly _template_service = inject(SignageTemplateService); private readonly _route = inject(ActivatedRoute); private readonly _router = inject(Router); @@ -354,33 +351,14 @@ export class DisplaysSectionComponent { public readonly can_update = this._context.can_update; public readonly can_delete_displays = this._context.can_delete_displays; - private readonly _displays = this._display_service.displays; - - private readonly _template_mappings = resource({ - params: () => { - const id: string = this.selected_display()?.id; - return this.templates_enabled() && id - ? { - id, - revision: - this._template_service.template_mappings_revision(), - } - : undefined; - }, - loader: ({ params }) => - this._template_service.listTemplateMappings({ - control_system_id: params.id, - }), - }); - public readonly template_count_loading = this._template_mappings.isLoading; + public readonly template_count_loading = + this._display_service.selected_display_template_mappings_loading; public readonly playlist_count_loading = this._playlist_service.playlists_loading; public readonly zone_count_loading = this._display_service.selected_display_zones_loading; - public readonly template_count = computed(() => - this._template_mappings.hasValue() - ? this._template_mappings.value().length - : 0, + public readonly template_count = computed( + () => this._display_service.selected_display_template_mappings().length, ); public readonly playlist_count = computed( @@ -400,10 +378,6 @@ export class DisplaysSectionComponent { return `${signage_path.replace(/\/$/, '')}/#/signage/${encodeURIComponent(display.id)}?debug=true`; }); - private _route_resolved = false; - // Last display id fetched for a link, so a missing id is fetched once - private _requested_id = ''; - constructor() { effect(() => { const route_tab = parseDisplayTab(this.tab()); @@ -416,47 +390,15 @@ export class DisplaysSectionComponent { } }); - effect(() => { - const id = this.id(); - const list = this._displays(); - if (id) { - const match = list.find((d) => d.id === id); - if (match) { - if ( - this._display_service.selected_display()?.id !== - match.id - ) { - this._display_service.selected_display.set(match); - } - this._route_resolved = true; - } else if ( - untracked(this._display_service.selected_display)?.id !== id - ) { - // The list holds only the pages loaded so far - untracked(() => this._loadDisplay(id)); - } - } else if (this._route_resolved) { - this._display_service.selected_display.set(null); - } + selectRoutedItem({ + id: this.id, + list: this._display_service.displays, + selected: this._display_service.selected_display, + // The list holds only the pages loaded so far + load: showSignageDisplay, }); } - /** Select a display from a link that the loaded pages do not include */ - private async _loadDisplay(id: string) { - if (this._requested_id === id) return; - this._requested_id = id; - const display = await showSignageDisplay(id).catch(() => null); - if ( - !display || - this.id() !== id || - this._display_service.selected_display()?.id === id - ) { - return; - } - this._display_service.selected_display.set(display); - this._route_resolved = true; - } - public deselectDisplay() { this._display_service.selected_display.set(null); this._router.navigate(['/displays'], {}); diff --git a/apps/signage-manager/src/app/displays/routed-selection.util.ts b/apps/signage-manager/src/app/displays/routed-selection.util.ts new file mode 100644 index 00000000000..1784b908195 --- /dev/null +++ b/apps/signage-manager/src/app/displays/routed-selection.util.ts @@ -0,0 +1,63 @@ +import { effect, untracked, type WritableSignal } from '@angular/core'; +import { i18n, notifyError } from '@placeos/common'; + +/** + * Select the item that the route names, for a page with a list and details. + * Takes the item from the loaded list, or loads it by id when the list does + * not hold it, such as a link to an item past the loaded pages. Clears the + * selection when the route drops the id after it selected an item. + * + * Creates an effect, so call it in an injection context. + */ +export function selectRoutedItem(options: { + /** Id in the route, empty when the route has no item */ + id: () => string; + /** Items loaded so far */ + list: () => readonly T[]; + selected: WritableSignal; + /** Load one item by id. A rejection shows an error. */ + load: (id: string) => Promise; +}) { + const { id, list, selected, load } = options; + let route_resolved = false; + // Id the route had when the effect last ran, and the id loaded for it. + // The load runs once each time the route moves to an id, so going back + // to an id loaded before loads it again. + let route_id_seen = ''; + let requested_id = ''; + + async function loadItem(item_id: string) { + const item = await load(item_id).catch(() => null); + if (id() !== item_id || untracked(selected)?.id === item_id) return; + if (!item) { + notifyError(i18n('COMMON.LOAD_ERROR')); + return; + } + selected.set(item); + route_resolved = true; + } + + effect(() => { + const route_id = id(); + const items = list(); + if (route_id !== route_id_seen) { + route_id_seen = route_id; + requested_id = ''; + } + if (!route_id) { + if (route_resolved) selected.set(null); + return; + } + const match = items.find((item) => item.id === route_id); + if (match) { + if (selected()?.id !== match.id) selected.set(match); + route_resolved = true; + } else if ( + untracked(selected)?.id !== route_id && + requested_id !== route_id + ) { + requested_id = route_id; + untracked(() => loadItem(route_id)); + } + }); +} diff --git a/apps/signage-manager/src/app/displays/signage-display.service.ts b/apps/signage-manager/src/app/displays/signage-display.service.ts index 5d84e125cb0..77144ce77d6 100644 --- a/apps/signage-manager/src/app/displays/signage-display.service.ts +++ b/apps/signage-manager/src/app/displays/signage-display.service.ts @@ -36,6 +36,7 @@ import { queryAll, searchParam, } from '../signage-service.util'; +import { SignageTemplateService } from '../templates/signage-template.service'; import { SignageZoneService } from '../zones/signage-zone.service'; import { type ZoneNode } from './display-zones.util'; import { @@ -58,6 +59,7 @@ export class SignageDisplayService { private readonly _context = inject(SignageContextService); private readonly _zone_service = inject(SignageZoneService); private readonly _playlist_service = inject(SignagePlaylistService); + private readonly _template_service = inject(SignageTemplateService); private readonly _display_overrides = signal>( {}, @@ -96,8 +98,11 @@ export class SignageDisplayService { ); public readonly displays_loading = this._display_list.loading; public readonly displays_has_more = this._display_list.has_more; + /** Whether the last page of displays failed to load */ + public readonly displays_error = this._display_list.error; /** Number of displays the server has for the current query */ public readonly displays_total = this._display_list.total; + private readonly _displays_reload = signal(0); private readonly _reload_displays = effect(() => { const initialised = this._org.initialised(); @@ -105,6 +110,7 @@ export class SignageDisplayService { const group_id = this._context.api_group_id_debounced.value(); const search = this._display_search_debounced.value().trim(); this._context.data_change(); + this._displays_reload(); untracked(() => { // A data change on the same query keeps the loaded rows on screen // and reloads as many rows as were loaded, so the list does not @@ -163,6 +169,14 @@ export class SignageDisplayService { this._display_list.loadMore(); } + /** Load the display page that failed again: the next page when some + * pages are loaded, otherwise the first page. */ + public retryDisplays() { + if (!this._display_list.retry()) { + this._displays_reload.update((count) => count + 1); + } + } + // Cleared when the user switches group public readonly selected_display = linkedSignal( { @@ -214,6 +228,13 @@ export class SignageDisplayService { ); public readonly selected_display_zones_loading = this._selected_display_zones.isLoading; + public readonly selected_display_zones_error = computed( + () => !!this._selected_display_zones.error(), + ); + + public reloadSelectedDisplayZones() { + this._selected_display_zones.reload(); + } /** * Displays in the selected zone. Queried by zone, as the display list @@ -244,6 +265,44 @@ export class SignageDisplayService { }); public readonly selected_zone_displays_loading = this._selected_zone_displays.isLoading; + public readonly selected_zone_displays_error = computed( + () => !!this._selected_zone_displays.error(), + ); + + public reloadSelectedZoneDisplays() { + this._selected_zone_displays.reload(); + } + + /** + * Template mappings of the selected display, loaded once for the tab + * count and the schedule. Empty when templates are off. + */ + private readonly _selected_display_template_mappings = resource({ + params: () => { + const id = this.selected_display()?.id; + return this._context.templates_enabled() && id + ? { + id, + revision: + this._template_service.template_mappings_revision(), + } + : undefined; + }, + loader: ({ params }) => + this._template_service.listTemplateMappings({ + control_system_id: params.id, + }), + }); + public readonly selected_display_template_mappings = computed(() => + this._selected_display_template_mappings.hasValue() + ? this._selected_display_template_mappings.value() + : [], + ); + public readonly selected_display_template_mappings_loading = + this._selected_display_template_mappings.isLoading; + public readonly selected_display_template_mappings_error = computed( + () => !!this._selected_display_template_mappings.error(), + ); /** * Keep a saved display until the lists reload. @@ -344,6 +403,8 @@ export class SignageDisplayService { if (!result) return null; const saved = this._addDisplayToList(result); this.selected_display.set(saved); + // Zone, schedule and report views hold their own copies of displays + this._context.changed(); notifySuccess(i18n('SIGNAGE_MANAGER.SVC_DISPLAY_SAVED')); return saved; } @@ -390,6 +451,7 @@ export class SignageDisplayService { if (this.selected_display()?.id === display.id) { this.selected_display.set(null); } + this._context.changed(); notifySuccess(i18n('SIGNAGE_MANAGER.SVC_DISPLAY_REMOVED')); return true; } @@ -404,11 +466,12 @@ export class SignageDisplayService { ? [active_zone] : []; if (group_id && !roots.length) { - const result = await queryZones({ - group_id, - limit: 500, - include_children_count: true, - } as any).catch(() => null); + const result = await queryZones( + this._context.groupQueryParams( + { limit: 500, include_children_count: true }, + group_id, + ), + ).catch(() => null); roots = (result?.data || []).map(decodeEntityNames); } return this._zone_service.zoneIdsWithAncestors(roots); diff --git a/apps/signage-manager/src/app/zones/signage-zone.service.ts b/apps/signage-manager/src/app/zones/signage-zone.service.ts index 540046a5c12..980f5815955 100644 --- a/apps/signage-manager/src/app/zones/signage-zone.service.ts +++ b/apps/signage-manager/src/app/zones/signage-zone.service.ts @@ -5,6 +5,7 @@ import { inject, Injectable, linkedSignal, + type Resource, resource, signal, untracked, @@ -34,6 +35,17 @@ import { SignageContextService } from '../signage-context.service'; import { dialogClosed, mergeItems } from '../signage-service.util'; import type { ZoneEditFormModel } from './zone-edit-modal.component'; +/** One load of a zone list, with the number of zones the server has */ +interface ZoneList { + zones: PlaceZone[]; + total: number; +} + +/** Zones of a zone list resource, empty while it loads or after it fails */ +function loadedZones(list: Resource) { + return list.hasValue() ? list.value().zones : []; +} + /** Signage zones, the selected zone, and the playlists assigned to zones */ @Injectable({ providedIn: 'root', @@ -77,13 +89,13 @@ export class SignageZoneService { parent_id, limit: 2500, include_children_count: true, - } as any); + }); } /** * Zones of the debounced group, loaded again after each save. Empty when - * the lists cannot be queried, when `load` gives no query or when the - * query fails. + * the lists cannot be queried or when `load` gives no query. A failed + * query puts the resource in its error state. */ private _zoneResource( load: (group_id: string) => QueryResponse | null, @@ -95,13 +107,16 @@ export class SignageZoneService { group_id: this._context.api_group_id_debounced.value(), can_query: this._context.can_query_group_data(), }), - loader: async ({ params }) => { + loader: async ({ params }): Promise => { const query = params.initialised && params.can_query ? load(params.group_id) : null; - const result = await query?.catch(() => null); - return (result?.data || []).map(decodeEntityNames); + const result = await query; + return { + zones: (result?.data || []).map(decodeEntityNames), + total: result?.total || 0, + }; }, }); } @@ -111,11 +126,15 @@ export class SignageZoneService { this._context.groupQueryParams( { limit: 250, tags: 'signage' }, group_id, - ) as any, + ), ), ); public readonly zones = computed(() => - mergeItems(this._zone_list.value() || [], this._zone_overrides()), + mergeItems(loadedZones(this._zone_list), this._zone_overrides()), + ); + /** Number of signage zones the server has, loaded or not */ + public readonly signage_zone_count = computed(() => + this._zone_list.hasValue() ? this._zone_list.value().total : 0, ); private readonly _all_zone_list = this._zoneResource((group_id) => @@ -127,7 +146,7 @@ export class SignageZoneService { ), ); public readonly all_zones = computed(() => - mergeItems(this._all_zone_list.value() || [], this._zone_overrides()), + mergeItems(loadedZones(this._all_zone_list), this._zone_overrides()), ); // A selected group has its own zones as roots, which the all zones list @@ -139,20 +158,45 @@ export class SignageZoneService { limit: 500, include_children_count: true, parent_id: 'root', - } as any), + }), ); public readonly root_zones = computed(() => { if (this._context.api_group_id_debounced.value()) { return this.all_zones(); } const org_zone_id = this._org.organisation?.id; - const zones = this._org_root_list.value() || []; + const zones = loadedZones(this._org_root_list); return mergeItems( org_zone_id ? zones.filter(({ id }) => id === org_zone_id) : zones, this._zone_overrides(), ); }); + /** Whether the zones of the zone tree are loading */ + public readonly zones_loading = computed( + () => + this._all_zone_list.isLoading() || this._org_root_list.isLoading(), + ); + /** Whether the zones of the zone tree failed to load */ + public readonly zones_error = computed( + () => !!this._all_zone_list.error() || !!this._org_root_list.error(), + ); + + /** + * Load the zone lists that failed again. Includes the signage zone list, + * which the header count and zone pickers read. + */ + public reloadZones() { + const lists = [ + this._zone_list, + this._all_zone_list, + this._org_root_list, + ]; + for (const list of lists) { + if (list.error()) list.reload(); + } + } + /** Zone tree callbacks for the modals that pick zones */ public zoneTreeData() { return { diff --git a/apps/signage-manager/src/app/zones/zone-content.component.ts b/apps/signage-manager/src/app/zones/zone-content.component.ts index 3e2c12219fb..6ab1436c83c 100644 --- a/apps/signage-manager/src/app/zones/zone-content.component.ts +++ b/apps/signage-manager/src/app/zones/zone-content.component.ts @@ -2,7 +2,11 @@ import { Component, computed, inject, input } from '@angular/core'; import { MatRippleModule } from '@angular/material/core'; import { MatTooltipModule } from '@angular/material/tooltip'; import { RouterLink } from '@angular/router'; -import { IconComponent, TranslatePipe } from '@placeos/components'; +import { + IconComponent, + LoadErrorComponent, + TranslatePipe, +} from '@placeos/components'; import { SignagePlaylist } from '@placeos/ts-client'; import { SignageDisplayService } from '../displays/signage-display.service'; import { SignagePlaylistService } from '../playlists/signage-playlist.service'; @@ -185,6 +189,13 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; }
} + } @else if (playlists_loading()) { +
+ {{ 'COMMON.LOADING' | translate }} +
} @else {
} + } @else if (zone_displays_loading()) { +
+ {{ 'COMMON.LOADING' | translate }} +
+ } @else if (zone_displays_error()) { + } @else {
this._zone_service.filtered_zones().length, + /** Search results while searching, otherwise the server total of + * signage zones, as the tree also shows untagged parent zones */ + public readonly total_count = computed(() => + this._zone_service.selected_zone()?.id && + this._zone_service.zone_search_term().trim() + ? this._zone_service.filtered_zones().length + : this._zone_service.signage_zone_count(), ); public readonly can_manage_zones = this._context.can_manage_zones; diff --git a/apps/signage-manager/src/app/zones/zone-list.component.ts b/apps/signage-manager/src/app/zones/zone-list.component.ts index 714b1825a47..6522339de61 100644 --- a/apps/signage-manager/src/app/zones/zone-list.component.ts +++ b/apps/signage-manager/src/app/zones/zone-list.component.ts @@ -14,7 +14,11 @@ import { MatInputModule } from '@angular/material/input'; import { MatTooltipModule } from '@angular/material/tooltip'; import { Router, RouterLink } from '@angular/router'; import { OrganisationService } from '@placeos/common'; -import { IconComponent, TranslatePipe } from '@placeos/components'; +import { + IconComponent, + LoadErrorComponent, + TranslatePipe, +} from '@placeos/components'; import { PlaceZone } from '@placeos/ts-client'; import { SignageZoneService } from './signage-zone.service'; @@ -55,8 +59,7 @@ interface FlatZoneTreeNode extends ZoneTreeNode { '', } " - [ngModel]="search()" - (ngModelChange)="search.set($event)" + [(ngModel)]="search" [attr.aria-label]=" 'SIGNAGE_MANAGER.SEARCH_IN_ZONE' | translate @@ -220,6 +223,15 @@ interface FlatZoneTreeNode extends ZoneTreeNode { } + } @else if (loading()) { +
+ {{ 'COMMON.LOADING' | translate }} +
+ } @else if (error()) { + } @else {
!!this.selected()?.id); public readonly show_search_results = computed( () => this.search_enabled() && !!this.search().trim(), @@ -392,6 +407,10 @@ export class ZoneListComponent { this.loadNodeChildren(current); } + public retry() { + this._zone_service.reloadZones(); + } + public retryChildren(node: ZoneTreeNode) { this.loadNodeChildren(node); } diff --git a/apps/signage-manager/src/app/zones/zones.component.ts b/apps/signage-manager/src/app/zones/zones.component.ts index b7a589bc493..e31028ce6a1 100644 --- a/apps/signage-manager/src/app/zones/zones.component.ts +++ b/apps/signage-manager/src/app/zones/zones.component.ts @@ -6,13 +6,13 @@ import { input, resource, signal, - untracked, } from '@angular/core'; import { MatRippleModule } from '@angular/material/core'; import { MatTooltipModule } from '@angular/material/tooltip'; import { ActivatedRoute, Router } from '@angular/router'; import { IconComponent, TranslatePipe } from '@placeos/components'; import { showZone } from '@placeos/ts-client'; +import { selectRoutedItem } from '../displays/routed-selection.util'; import { SignageDisplayService } from '../displays/signage-display.service'; import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { decodeEntityNames } from '../shared/decode-entity-names.util'; @@ -316,11 +316,9 @@ export class ZonesSectionComponent { ); }); - private readonly _zones = this._zone_service.all_zones; - private readonly _template_mappings = resource({ params: () => { - const id: string = this.selected_zone()?.id; + const id = this.selected_zone()?.id; return this.templates_enabled() && id ? { id, @@ -354,10 +352,6 @@ export class ZonesSectionComponent { () => this._display_service.selected_zone_displays().length, ); - private _route_resolved = false; - // Last zone id fetched for a link, so a missing id is fetched once - private _requested_id = ''; - constructor() { effect(() => { const route_tab = parseZoneTab(this.tab()); @@ -370,44 +364,15 @@ export class ZonesSectionComponent { } }); - effect(() => { - const id = this.id(); - const list = this._zones(); - if (id) { - const match = list.find((z) => z.id === id); - if (match) { - if (this._zone_service.selected_zone()?.id !== match.id) { - this._zone_service.selected_zone.set(match); - } - this._route_resolved = true; - } else if ( - untracked(this._zone_service.selected_zone)?.id !== id - ) { - // `all_zones` holds only the first 500 zones of the group - untracked(() => this._loadZone(id)); - } - } else if (this._route_resolved) { - this._zone_service.selected_zone.set(null); - } + selectRoutedItem({ + id: this.id, + list: this._zone_service.all_zones, + selected: this._zone_service.selected_zone, + // `all_zones` holds only the first 500 zones of the group + load: async (id) => decodeEntityNames(await showZone(id)), }); } - /** Select a zone from a link that the loaded zones do not include */ - private async _loadZone(id: string) { - if (this._requested_id === id) return; - this._requested_id = id; - const zone = await showZone(id).catch(() => null); - if ( - !zone || - this.id() !== id || - this._zone_service.selected_zone()?.id === id - ) { - return; - } - this._zone_service.selected_zone.set(decodeEntityNames(zone)); - this._route_resolved = true; - } - public deselectZone() { this._zone_service.selected_zone.set(null); this._router.navigate(['/zones'], {}); diff --git a/apps/signage-manager/src/tests/displays/display-content.component.spec.ts b/apps/signage-manager/src/tests/displays/display-content.component.spec.ts index 8823c8505ec..94bd970d590 100644 --- a/apps/signage-manager/src/tests/displays/display-content.component.spec.ts +++ b/apps/signage-manager/src/tests/displays/display-content.component.spec.ts @@ -1,5 +1,6 @@ import { signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; +import { provideRouter } from '@angular/router'; import { DisplayContentComponent } from '../../app/displays/display-content.component'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; @@ -16,10 +17,17 @@ describe('DisplayContentComponent', () => { const can_update = signal(true); const add_playlist = vi.fn(); const remove_playlist = vi.fn(); + const zones_loading = signal(false); + const zones_error = signal(false); + const playlists_loading = signal(false); + const reload_zones = vi.fn(); const context_stub = { can_update }; const display_stub = { selected_display, selected_display_zones, + selected_display_zones_loading: zones_loading, + selected_display_zones_error: zones_error, + reloadSelectedDisplayZones: reload_zones, addPlaylistToDisplay: add_playlist, removePlaylistFromDisplay: remove_playlist, }; @@ -28,12 +36,14 @@ describe('DisplayContentComponent', () => { playlists().filter(({ id }) => ids.includes(id)), playlist_approval_status, playlist_thumbnail_media, + playlists_loading, }; async function make() { await TestBed.configureTestingModule({ imports: [DisplayContentComponent], providers: [ + provideRouter([]), { provide: SignageContextService, useValue: context_stub }, { provide: SignageDisplayService, useValue: display_stub }, { provide: SignagePlaylistService, useValue: playlist_stub }, @@ -47,12 +57,32 @@ describe('DisplayContentComponent', () => { .componentInstance; } + /** Render a tab of the selected display */ + async function render(tab: 'playlists' | 'zones') { + await TestBed.configureTestingModule({ + imports: [DisplayContentComponent], + providers: [ + provideRouter([]), + { provide: SignageContextService, useValue: context_stub }, + { provide: SignageDisplayService, useValue: display_stub }, + { provide: SignagePlaylistService, useValue: playlist_stub }, + ], + }).compileComponents(); + const fixture = TestBed.createComponent(DisplayContentComponent); + fixture.componentRef.setInput('activeTab', tab); + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + beforeEach(() => { vi.clearAllMocks(); selected_display.set(null); playlists.set([]); selected_display_zones.set([]); playlist_approval_status.set({}); + zones_loading.set(false); + zones_error.set(false); + playlists_loading.set(false); }); it('lists the playlists and the queried zones of the display', async () => { @@ -137,4 +167,33 @@ describe('DisplayContentComponent', () => { expect(event.stopPropagation).toHaveBeenCalled(); expect(remove_playlist).toHaveBeenCalledWith(display, 'p1'); }); + + // The empty states use these icons + it('shows that the zones are loading in place of the empty state', async () => { + selected_display.set({ id: 'd1', zones: ['z1'] }); + zones_loading.set(true); + const element = await render('zones'); + + expect(element.querySelector('[role="status"]')).not.toBeNull(); + expect(element.textContent).not.toContain('layers_clear'); + }); + + it('offers a retry when the zones of the display fail to load', async () => { + selected_display.set({ id: 'd1', zones: ['z1'] }); + zones_error.set(true); + const element = await render('zones'); + + expect(element.textContent).not.toContain('layers_clear'); + element.querySelector('load-error button')?.click(); + expect(reload_zones).toHaveBeenCalledTimes(1); + }); + + it('shows that the playlists are loading in place of the empty state', async () => { + selected_display.set({ id: 'd1', playlists: ['p1'] }); + playlists_loading.set(true); + const element = await render('playlists'); + + expect(element.querySelector('[role="status"]')).not.toBeNull(); + expect(element.textContent).not.toContain('playlist_remove'); + }); }); diff --git a/apps/signage-manager/src/tests/displays/display-list.component.spec.ts b/apps/signage-manager/src/tests/displays/display-list.component.spec.ts index 07f13433b18..e11032a5e29 100644 --- a/apps/signage-manager/src/tests/displays/display-list.component.spec.ts +++ b/apps/signage-manager/src/tests/displays/display-list.component.spec.ts @@ -1,5 +1,6 @@ import { signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; +import { provideRouter } from '@angular/router'; import { DisplayListComponent } from '../../app/displays/display-list.component'; import { signageDisplay } from '../../app/displays/signage-display'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; @@ -10,19 +11,24 @@ describe('DisplayListComponent', () => { const selected_display = signal(null); const displays_has_more = signal(false); const displays_loading = signal(false); + const displays_error = signal(false); const load_more = vi.fn(); + const retry_displays = vi.fn(); const display_stub = { display_search_term, filtered_displays, selected_display, displays_has_more, displays_loading, + displays_error, loadMoreDisplays: load_more, + retryDisplays: retry_displays, }; function make() { TestBed.configureTestingModule({ providers: [ + provideRouter([]), { provide: SignageDisplayService, useValue: display_stub }, ], }); @@ -33,6 +39,8 @@ describe('DisplayListComponent', () => { beforeEach(() => { load_more.mockReset(); + retry_displays.mockReset(); + displays_error.set(false); display_search_term.set(''); filtered_displays.set([]); selected_display.set(null); @@ -110,4 +118,28 @@ describe('DisplayListComponent', () => { component.loadMore(); expect(load_more).toHaveBeenCalledTimes(1); }); + + it.each([ + ['no displays loaded', []], + ['some displays loaded', [{ id: 'd1', name: 'Lobby' }]], + ])( + 'offers a retry in place of the list end when a page fails with %s', + (_, items) => { + filtered_displays.set(items); + displays_error.set(true); + make(); + const fixture = TestBed.createComponent(DisplayListComponent); + fixture.detectChanges(); + const element: HTMLElement = fixture.nativeElement; + + expect(element.querySelector('load-error')).not.toBeNull(); + expect(element.textContent).not.toContain('No displays'); + expect(element.textContent).not.toContain('End of list'); + + element + .querySelector('load-error button')! + .click(); + expect(retry_displays).toHaveBeenCalledTimes(1); + }, + ); }); diff --git a/apps/signage-manager/src/tests/displays/display-schedule.component.spec.ts b/apps/signage-manager/src/tests/displays/display-schedule.component.spec.ts index 1a22fbd1e2e..72d2159910d 100644 --- a/apps/signage-manager/src/tests/displays/display-schedule.component.spec.ts +++ b/apps/signage-manager/src/tests/displays/display-schedule.component.spec.ts @@ -5,26 +5,27 @@ import { addDays, isSameDay, startOfWeek } from 'date-fns'; import { DisplayScheduleComponent } from '../../app/displays/display-schedule.component'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; -import { SignageContextService } from '../../app/signage-context.service'; import { HydratedSignageTemplateMapping } from '../../app/signage-template-mapping'; -import { SignageTemplateService } from '../../app/templates/signage-template.service'; describe('DisplayScheduleComponent', () => { const selected_display = signal(null); const selected_display_zones = signal([]); const playlists = signal([]); - const context_stub = { templates_enabled: signal(false) }; - const display_stub = { selected_display, selected_display_zones }; + const template_mappings = signal([]); + const display_stub = { + selected_display, + selected_display_zones, + selected_display_template_mappings: template_mappings, + selected_display_template_mappings_loading: signal(false), + selected_display_template_mappings_error: signal(false), + }; const playlist_stub = { playlistsById: (ids: readonly string[]) => playlists().filter(({ id }) => ids.includes(id)), }; - const template_stub = { listTemplateMappings: vi.fn() }; const stub_providers = [ - { provide: SignageContextService, useValue: context_stub }, { provide: SignageDisplayService, useValue: display_stub }, { provide: SignagePlaylistService, useValue: playlist_stub }, - { provide: SignageTemplateService, useValue: template_stub }, ]; function make() { @@ -39,8 +40,7 @@ describe('DisplayScheduleComponent', () => { selected_display.set(null); selected_display_zones.set([]); playlists.set([]); - context_stub.templates_enabled.set(false); - template_stub.listTemplateMappings.mockReset().mockResolvedValue([]); + template_mappings.set([]); }); it('renders a full seven-day week starting on the current Monday', () => { @@ -151,8 +151,7 @@ describe('DisplayScheduleComponent', () => { } }); - it('loads display mappings and renders linked playlists inside templates', async () => { - context_stub.templates_enabled.set(true); + it('renders linked playlists inside the templates of the display', async () => { selected_display.set({ id: 'd1', playlists: ['p1'] }); playlists.set([ { @@ -162,7 +161,7 @@ describe('DisplayScheduleComponent', () => { schedules: [{ play_cron: '0 9 * * *', play_period: 60 }], }, ]); - template_stub.listTemplateMappings.mockResolvedValue([ + template_mappings.set([ new HydratedSignageTemplateMapping({ id: 'm1', template_id: 't1', @@ -183,16 +182,10 @@ describe('DisplayScheduleComponent', () => { expect( parent?.querySelector('ul a[href="/playlists/p1"]')?.textContent, ).toContain('Morning playlist'); - expect(template_stub.listTemplateMappings).toHaveBeenCalledWith({ - control_system_id: 'd1', - }); selected_display.set({ id: 'd2', playlists: [] }); - template_stub.listTemplateMappings.mockResolvedValue([]); + template_mappings.set([]); await fixture.whenStable(); - expect(template_stub.listTemplateMappings).toHaveBeenLastCalledWith({ - control_system_id: 'd2', - }); expect(element.querySelector('a[href="/templates/t1"]')).toBeNull(); }); diff --git a/apps/signage-manager/src/tests/displays/displays.component.spec.ts b/apps/signage-manager/src/tests/displays/displays.component.spec.ts index 470b9c215c9..49ac6cf0f74 100644 --- a/apps/signage-manager/src/tests/displays/displays.component.spec.ts +++ b/apps/signage-manager/src/tests/displays/displays.component.spec.ts @@ -1,13 +1,14 @@ import { NO_ERRORS_SCHEMA, signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { MatSnackBar } from '@angular/material/snack-bar'; import { ActivatedRoute, Router } from '@angular/router'; +import { setNotifyOutlet } from '@placeos/common'; import { TranslatePipe } from '@placeos/components'; import { PlaceSystem, show } from '@placeos/ts-client'; import { DisplaysSectionComponent } from '../../app/displays/displays.component'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; import { SignageContextService } from '../../app/signage-context.service'; -import { SignageTemplateService } from '../../app/templates/signage-template.service'; vi.mock('@placeos/ts-client', { spy: true }); @@ -21,8 +22,8 @@ describe('DisplaysSectionComponent', () => { const templates_enabled = signal(true); const playlists_loading = signal(false); const related_loading = signal(false); - const template_mappings_revision = signal(0); - const list_template_mappings = vi.fn(); + const template_mappings = signal<{ id: string }[]>([]); + const template_mappings_loading = signal(false); const navigate = vi.fn(); const edit_display = vi.fn(); const remove_display = vi.fn(); @@ -36,6 +37,8 @@ describe('DisplaysSectionComponent', () => { displays, selected_display_zones, selected_display_zones_loading: related_loading, + selected_display_template_mappings: template_mappings, + selected_display_template_mappings_loading: template_mappings_loading, editDisplay: edit_display, removeDisplay: remove_display, }; @@ -44,10 +47,6 @@ describe('DisplaysSectionComponent', () => { playlists().filter(({ id }) => ids.includes(id)), playlists_loading, }; - const template_stub = { - template_mappings_revision, - listTemplateMappings: list_template_mappings, - }; const router_stub = { navigate }; async function make( @@ -61,7 +60,6 @@ describe('DisplaysSectionComponent', () => { { provide: SignageContextService, useValue: context_stub }, { provide: SignageDisplayService, useValue: display_stub }, { provide: SignagePlaylistService, useValue: playlist_stub }, - { provide: SignageTemplateService, useValue: template_stub }, { provide: Router, useValue: router_stub }, { provide: ActivatedRoute, useValue: {} }, ], @@ -87,19 +85,14 @@ describe('DisplaysSectionComponent', () => { templates_enabled.set(true); playlists_loading.set(false); related_loading.set(false); - template_mappings_revision.set(0); - list_template_mappings.mockResolvedValue([]); + template_mappings.set([]); + template_mappings_loading.set(false); remove_display.mockResolvedValue(false); }); it('shows question marks in count badges while data loads', async () => { selected_display.set({ id: 'target-1' }); - let finish_loading!: (value: []) => void; - list_template_mappings.mockReturnValue( - new Promise<[]>((resolve) => { - finish_loading = resolve; - }), - ); + template_mappings_loading.set(true); playlists_loading.set(true); related_loading.set(true); const [, fixture] = await make(true); @@ -114,7 +107,7 @@ describe('DisplaysSectionComponent', () => { } }); - finish_loading([]); + template_mappings_loading.set(false); playlists_loading.set(false); related_loading.set(false); await fixture.whenStable(); @@ -124,34 +117,16 @@ describe('DisplaysSectionComponent', () => { } }); - it('counts template mappings and refreshes after assignment changes', async () => { + it('counts the template mappings of the selected display', async () => { selected_display.set({ id: 'target-1' }); - list_template_mappings.mockResolvedValue([{ id: 'm1' }, { id: 'm2' }]); + template_mappings.set([{ id: 'm1' }, { id: 'm2' }]); const [component, fixture] = await make(true); await fixture.whenStable(); - expect(list_template_mappings).toHaveBeenCalledWith({ - control_system_id: 'target-1', - }); expect(component.template_count()).toBe(2); const element: HTMLElement = fixture.nativeElement; const tab = element.querySelector('#display-templates-tab span'); expect(tab?.textContent?.trim()).toBe('2'); - - list_template_mappings.mockResolvedValue([{ id: 'm1' }]); - template_mappings_revision.update((value) => value + 1); - await fixture.whenStable(); - expect(component.template_count()).toBe(1); - expect(tab?.textContent?.trim()).toBe('1'); - - selected_display.set({ id: 'target-2' }); - list_template_mappings.mockResolvedValue([]); - await fixture.whenStable(); - expect(list_template_mappings).toHaveBeenLastCalledWith({ - control_system_id: 'target-2', - }); - expect(component.template_count()).toBe(0); - expect(tab?.textContent?.trim()).toBe('0'); }); it('counts the playlists and zones attached to the selected display', async () => { @@ -207,6 +182,49 @@ describe('DisplaysSectionComponent', () => { ); }); + it('loads a linked display again after going back to it', async () => { + displays.set([{ id: 'd1' }]); + const far = new PlaceSystem({ id: 'd-far', name: 'Far' }); + vi.mocked(show).mockResolvedValue(far); + const [, fixture] = await make(); + const route = (id: string) => { + fixture.componentRef.setInput('id', id); + fixture.detectChanges(); + TestBed.flushEffects(); + }; + + route('d-far'); + await vi.waitFor(() => expect(selected_display()).toBe(far)); + route('d1'); + expect(selected_display()?.id).toBe('d1'); + route('d-far'); + + await vi.waitFor(() => expect(selected_display()).toBe(far)); + expect(show).toHaveBeenCalledTimes(2); + }); + + it('shows an error when a linked display cannot be loaded', async () => { + const notify_open = vi.fn(() => ({ + onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }), + dismiss: vi.fn(), + })); + setNotifyOutlet({ open: notify_open } as unknown as MatSnackBar, true); + vi.mocked(show).mockRejectedValue(new Error('Not found')); + const [, fixture] = await make(); + fixture.componentRef.setInput('id', 'd-gone'); + fixture.detectChanges(); + TestBed.flushEffects(); + + await vi.waitFor(() => + expect(notify_open).toHaveBeenCalledWith( + expect.any(String), + expect.anything(), + expect.objectContaining({ panelClass: ['error'] }), + ), + ); + expect(selected_display()).toBeNull(); + }); + it('clears the selection when navigating back to the list', async () => { displays.set([{ id: 'd1' }]); const [, fixture] = await make(); diff --git a/apps/signage-manager/src/tests/displays/signage-display.service.spec.ts b/apps/signage-manager/src/tests/displays/signage-display.service.spec.ts index b63d36e8fa6..4098e2b5844 100644 --- a/apps/signage-manager/src/tests/displays/signage-display.service.spec.ts +++ b/apps/signage-manager/src/tests/displays/signage-display.service.spec.ts @@ -6,9 +6,18 @@ import { setNotifyOutlet, SettingsService, } from '@placeos/common'; -import { PlaceSystem, show, SignagePlaylist, update } from '@placeos/ts-client'; +import { + PlaceSystem, + removeSystem, + show, + SignagePlaylist, + update, +} from '@placeos/ts-client'; +import { NEVER, of } from 'rxjs'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; import { SignageContextService } from '../../app/signage-context.service'; +import { HydratedSignageTemplateMapping } from '../../app/signage-template-mapping'; +import { SignageTemplateService } from '../../app/templates/signage-template.service'; vi.mock('@placeos/ts-client', { spy: true }); @@ -149,4 +158,88 @@ describe('SignageDisplayService', () => { expect(update).not.toHaveBeenCalled(); expect(changed).not.toHaveBeenCalled(); }); + + /** The next confirm modal returns "done" */ + function confirmNextDialog() { + dialog.open.mockReturnValue({ + componentInstance: { + event: of({ reason: 'done' }), + loading: { set: vi.fn() }, + }, + afterClosed: () => NEVER, + close: vi.fn(), + }); + } + + // Zone, schedule and report views keep their own copies of displays + it('reloads the other views after a display is saved or removed', async () => { + const service = createService(); + const changed = vi.spyOn( + TestBed.inject(SignageContextService), + 'changed', + ); + closeNextDialogWith(new PlaceSystem({ id: 'd1', name: 'Lobby' })); + + await service.editDisplay(new PlaceSystem({ id: 'd1' })); + expect(changed).toHaveBeenCalledTimes(1); + + vi.mocked(removeSystem).mockResolvedValue({}); + confirmNextDialog(); + await service.removeDisplay(new PlaceSystem({ id: 'd1' })); + expect(changed).toHaveBeenCalledTimes(2); + }); + + it('takes a display used outside signage off signage instead of deleting it', async () => { + const service = createService(); + const display = new PlaceSystem({ + id: 'room-1', + version: 4, + modules: ['mod-1'], + }); + vi.mocked(update).mockResolvedValue(display); + confirmNextDialog(); + + expect(await service.removeDisplay(display)).toBe(true); + + expect(removeSystem).not.toHaveBeenCalled(); + expect(update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'room-1', + form_data: { signage: false }, + method: 'patch', + }), + ); + }); + + it('loads the template mappings of the selected display once for its views', async () => { + vi.spyOn( + TestBed.inject(SignageContextService), + 'hasFeature', + ).mockReturnValue(true); + const templates = TestBed.inject(SignageTemplateService); + const list = vi + .spyOn(templates, 'listTemplateMappings') + .mockResolvedValue([ + new HydratedSignageTemplateMapping({ id: 'm1' }), + ]); + const service = createService(); + + service.selected_display.set(new PlaceSystem({ id: 'd1' })); + await vi.waitFor(() => + expect(service.selected_display_template_mappings()).toHaveLength( + 1, + ), + ); + expect(list).toHaveBeenCalledExactlyOnceWith({ + control_system_id: 'd1', + }); + + list.mockResolvedValue([]); + templates.template_mappings_revision.update((value) => value + 1); + TestBed.tick(); + await vi.waitFor(() => expect(list).toHaveBeenCalledTimes(2)); + await vi.waitFor(() => + expect(service.selected_display_template_mappings()).toEqual([]), + ); + }); }); diff --git a/apps/signage-manager/src/tests/signage-display-search.spec.ts b/apps/signage-manager/src/tests/signage-display-search.spec.ts index b49835e8352..e2fccaadeb6 100644 --- a/apps/signage-manager/src/tests/signage-display-search.spec.ts +++ b/apps/signage-manager/src/tests/signage-display-search.spec.ts @@ -116,6 +116,24 @@ describe('SignageDisplayService display search', () => { ]); }); + it('loads the first page again after it failed', async () => { + mockDisplayList(pageOf(['lobby'], 1)); + vi.mocked(query).mockRejectedValueOnce(new Error('offline')); + const service = TestBed.inject(SignageDisplayService); + TestBed.tick(); + await flush(); + expect(service.displays_error()).toBe(true); + + service.retryDisplays(); + TestBed.tick(); + await flush(); + + expect(service.displays_error()).toBe(false); + expect(service.filtered_displays().map(({ id }) => id)).toEqual([ + 'lobby', + ]); + }); + it('should page the search results', async () => { const service = await init(); mockDisplayList(pageOf(['lobby-1'], 2, pageOf(['lobby-2'], 2))); diff --git a/apps/signage-manager/src/tests/signage-displays-zones.spec.ts b/apps/signage-manager/src/tests/signage-displays-zones.spec.ts index 3c11d232d83..764d2178029 100644 --- a/apps/signage-manager/src/tests/signage-displays-zones.spec.ts +++ b/apps/signage-manager/src/tests/signage-displays-zones.spec.ts @@ -205,7 +205,10 @@ describe('SignageDisplayService and SignageZoneService', () => { d1: new PlaceSystem({ id: 'd1', name: 'Edited' }), other: new PlaceSystem({ id: 'other', name: 'Other group' }), }); - zones['_all_zone_list'].set([new PlaceZone({ id: 'z1' })]); + zones['_all_zone_list'].set({ + zones: [new PlaceZone({ id: 'z1' })], + total: 1, + }); zones['_zone_overrides'].set({ other: new PlaceZone({ id: 'other', name: 'Other group' }), }); diff --git a/apps/signage-manager/src/tests/signage-zone-search.spec.ts b/apps/signage-manager/src/tests/signage-zone-search.spec.ts index 276903d73d9..3a68484baeb 100644 --- a/apps/signage-manager/src/tests/signage-zone-search.spec.ts +++ b/apps/signage-manager/src/tests/signage-zone-search.spec.ts @@ -75,6 +75,45 @@ describe('SignageZoneService zone search', () => { ]); }); + it('reports a failed zone load and loads it again on retry', async () => { + vi.mocked(queryZones).mockRejectedValue(new Error('offline')); + const service = TestBed.inject(SignageZoneService); + TestBed.tick(); + await flush(); + expect(service.zones_error()).toBe(true); + expect(service.all_zones()).toEqual([]); + + vi.mocked(queryZones).mockResolvedValue({ + data: [new PlaceZone({ id: 'z1', tags: ['signage'] })], + total: 7, + next: null, + }); + service.reloadZones(); + TestBed.tick(); + await flush(); + + expect(service.zones_error()).toBe(false); + expect(service.all_zones().map(({ id }) => id)).toEqual(['z1']); + // The header count reads the signage zone list, so it reloads too + expect(service.signage_zone_count()).toBe(7); + }); + + it('counts signage zones from the server total', async () => { + vi.mocked(queryZones).mockResolvedValue({ + data: [new PlaceZone({ id: 'z1', tags: ['signage'] })], + total: 7, + next: null, + }); + const service = TestBed.inject(SignageZoneService); + TestBed.tick(); + await flush(); + + expect(service.signage_zone_count()).toBe(7); + expect(queryZones).toHaveBeenCalledWith( + expect.objectContaining({ tags: 'signage' }), + ); + }); + it('searches selectable zones beneath the selected zone', () => { const service = TestBed.inject(SignageZoneService); diff --git a/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts index 89ad317b893..2cc2e5bb41c 100644 --- a/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts @@ -1,5 +1,6 @@ import { signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; +import { provideRouter } from '@angular/router'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; import { SignageContextService } from '../../app/signage-context.service'; @@ -18,9 +19,16 @@ describe('ZoneContentComponent', () => { const add_playlist = vi.fn(); const remove_playlist = vi.fn(); const add_display = vi.fn(); + const displays_loading = signal(false); + const displays_error = signal(false); + const playlists_loading = signal(false); + const reload_displays = vi.fn(); const context_stub = { can_update }; const display_stub = { selected_zone_displays, + selected_zone_displays_loading: displays_loading, + selected_zone_displays_error: displays_error, + reloadSelectedZoneDisplays: reload_displays, addDisplayToZone: add_display, }; const playlist_stub = { @@ -28,6 +36,7 @@ describe('ZoneContentComponent', () => { playlists().filter(({ id }) => ids.includes(id)), playlist_approval_status, playlist_thumbnail_media, + playlists_loading, }; const zone_stub = { selected_zone, @@ -52,12 +61,35 @@ describe('ZoneContentComponent', () => { return TestBed.createComponent(ZoneContentComponent).componentInstance; } + /** Render a tab of the selected zone */ + async function render(tab: 'playlists' | 'displays') { + await TestBed.configureTestingModule({ + imports: [ZoneContentComponent], + providers: [ + provideRouter([]), + { provide: SignageContextService, useValue: context_stub }, + { provide: SignageDisplayService, useValue: display_stub }, + { provide: SignagePlaylistService, useValue: playlist_stub }, + { provide: SignageZoneService, useValue: zone_stub }, + ], + }).compileComponents(); + const fixture = TestBed.createComponent(ZoneContentComponent); + fixture.componentRef.setInput('activeTab', tab); + fixture.detectChanges(); + return fixture.nativeElement.querySelector( + `#zone-${tab}-panel`, + ) as HTMLElement; + } + beforeEach(() => { vi.clearAllMocks(); selected_zone.set(null); playlists.set([]); selected_zone_displays.set([]); playlist_approval_status.set({}); + displays_loading.set(false); + displays_error.set(false); + playlists_loading.set(false); }); it('lists the playlists and the queried displays of the zone', async () => { @@ -112,4 +144,33 @@ describe('ZoneContentComponent', () => { expect(remove_playlist).toHaveBeenCalledWith(zone, 'p1'); expect(event.preventDefault).toHaveBeenCalled(); }); + + // The empty states use these icons + it('shows that the displays are loading in place of the empty state', async () => { + selected_zone.set({ id: 'z1' }); + displays_loading.set(true); + const panel = await render('displays'); + + expect(panel.querySelector('[role="status"]')).not.toBeNull(); + expect(panel.textContent).not.toContain('tv_off'); + }); + + it('offers a retry when the displays of the zone fail to load', async () => { + selected_zone.set({ id: 'z1' }); + displays_error.set(true); + const panel = await render('displays'); + + expect(panel.textContent).not.toContain('tv_off'); + panel.querySelector('load-error button')?.click(); + expect(reload_displays).toHaveBeenCalledTimes(1); + }); + + it('shows that the playlists are loading in place of the empty state', async () => { + selected_zone.set({ id: 'z1', playlists: ['p1'] }); + playlists_loading.set(true); + const panel = await render('playlists'); + + expect(panel.querySelector('[role="status"]')).not.toBeNull(); + expect(panel.textContent).not.toContain('playlist_remove'); + }); }); diff --git a/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts index f04f7dac202..01004df9a5a 100644 --- a/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts @@ -6,13 +6,19 @@ import { SignageZoneService } from '../../app/zones/signage-zone.service'; import { ZoneHeaderComponent } from '../../app/zones/zone-header.component'; describe('ZoneHeaderComponent', () => { - const filtered_zones = signal([]); + const filtered_zones = signal<{ id: string }[]>([]); + const signage_zone_count = signal(0); + const selected_zone = signal<{ id: string } | null>(null); + const zone_search_term = signal(''); const can_manage_zones = signal(false); const add_zone = vi.fn(); const navigate = vi.fn(); const context_stub = { can_manage_zones }; const zone_stub = { filtered_zones, + signage_zone_count, + selected_zone, + zone_search_term, addZone: add_zone, }; @@ -30,15 +36,29 @@ describe('ZoneHeaderComponent', () => { beforeEach(() => { vi.clearAllMocks(); filtered_zones.set([]); + signage_zone_count.set(0); + selected_zone.set(null); + zone_search_term.set(''); can_manage_zones.set(false); add_zone.mockResolvedValue(null); }); - it('reports the number of filtered zones', () => { + // The tree also lists untagged parents, so it is not the zone count + it('reports the server total of signage zones', () => { const component = make(); - expect(component.total_count()).toBe(0); + filtered_zones.set([{ id: 'org' }, { id: 'building' }, { id: 'a' }]); + signage_zone_count.set(1); + expect(component.total_count()).toBe(1); + }); + + it('reports the number of search results while searching a zone', () => { + const component = make(); + signage_zone_count.set(40); + selected_zone.set({ id: 'building' }); + zone_search_term.set('lobby'); filtered_zones.set([{ id: 'a' }, { id: 'b' }]); + expect(component.total_count()).toBe(2); }); diff --git a/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts index 220a6c03949..e8f0f94235b 100644 --- a/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts @@ -14,6 +14,9 @@ describe('ZoneListComponent', () => { const zone_tree_expanded = signal>({}); const zone_tree_children_cache = signal>({}); const zone_children = vi.fn(); + const zones_loading = signal(false); + const zones_error = signal(false); + const reload_zones = vi.fn(); const org_stub = { initialised }; const zone_stub = { all_zones, @@ -24,9 +27,12 @@ describe('ZoneListComponent', () => { zone_tree_expanded, zone_tree_children_cache, zoneChildren: zone_children, + zones_loading, + zones_error, + reloadZones: reload_zones, }; - async function make() { + async function make(render_template = false) { await TestBed.configureTestingModule({ imports: [ZoneListComponent], providers: [ @@ -34,12 +40,15 @@ describe('ZoneListComponent', () => { { provide: OrganisationService, useValue: org_stub }, ], }) - .overrideComponent(ZoneListComponent, { set: { template: '' } }) + .overrideComponent( + ZoneListComponent, + render_template ? {} : { set: { template: '' } }, + ) .compileComponents(); const fixture = TestBed.createComponent(ZoneListComponent); fixture.detectChanges(); TestBed.flushEffects(); - return fixture.componentInstance; + return fixture; } beforeEach(() => { @@ -53,10 +62,29 @@ describe('ZoneListComponent', () => { zone_tree_expanded.set({}); zone_tree_children_cache.set({}); zone_children.mockResolvedValue([]); + zones_loading.set(false); + zones_error.set(false); + }); + + it('shows that the zones are loading in place of the empty state', async () => { + zones_loading.set(true); + const element: HTMLElement = (await make(true)).nativeElement; + + expect(element.querySelector('[role="status"]')).not.toBeNull(); + expect(element.textContent).not.toContain('No zones'); + }); + + it('offers a retry when the zones fail to load', async () => { + zones_error.set(true); + const element: HTMLElement = (await make(true)).nativeElement; + + expect(element.textContent).not.toContain('No zones'); + element.querySelector('load-error button')?.click(); + expect(reload_zones).toHaveBeenCalledTimes(1); }); it('only enables search after selecting a zone', async () => { - const component = await make(); + const component = (await make()).componentInstance; expect(component.search_enabled()).toBe(false); expect(component.show_search_results()).toBe(false); @@ -72,7 +100,7 @@ describe('ZoneListComponent', () => { selected_zone.set({ id: 'r1' }); zone_search_term.set('lobby'); filtered_zones.set([{ id: 'z1', parent_id: 'r1', children_count: 2 }]); - const component = await make(); + const component = (await make()).componentInstance; expect( component @@ -95,7 +123,7 @@ describe('ZoneListComponent', () => { { id: 'c2', parent_id: 'r1' }, { id: 'gc1', parent_id: 'c1' }, ]); - const component = await make(); + const component = (await make()).componentInstance; expect(component.child_count_lookup()).toEqual({ r1: 2, c1: 1 }); expect(component.children_lookup()['r1'].map((z: any) => z.id)).toEqual( @@ -105,14 +133,14 @@ describe('ZoneListComponent', () => { it('reports child count from the lookup for a known parent zone', async () => { all_zones.set([{ id: 'r1' }, { id: 'c1', parent_id: 'r1' }]); - const component = await make(); + const component = (await make()).componentInstance; expect(component.childCount('r1')).toBe(1); expect(component.childCount('c1')).toBe(0); }); it('falls back to a zone children_count when it has no lookup entry', async () => { - const component = await make(); + const component = (await make()).componentInstance; expect( component.childCount({ id: 'x', children_count: 4 } as any), ).toBe(4); @@ -121,7 +149,7 @@ describe('ZoneListComponent', () => { it('builds root tree nodes from the service root zones', async () => { root_zones.set([{ id: 'r1', name: 'Root 1' }]); all_zones.set([{ id: 'r1' }, { id: 'c1', parent_id: 'r1' }]); - const component = await make(); + const component = (await make()).componentInstance; const flat = component.flat_tree_nodes(); expect(flat.map((n) => n.zone.id)).toEqual(['r1']); @@ -130,7 +158,7 @@ describe('ZoneListComponent', () => { it('selects a zone through the shared selection signal', async () => { zone_search_term.set('lobby'); - const component = await make(); + const component = (await make()).componentInstance; component.selectZone({ id: 'z9' } as any); expect(selected_zone()?.id).toBe('z9'); expect(zone_search_term()).toBe(''); @@ -140,7 +168,7 @@ describe('ZoneListComponent', () => { root_zones.set([{ id: 'r1', name: 'Root 1', children_count: 1 }]); all_zones.set([{ id: 'r1' }, { id: 'c1', parent_id: 'r1' }]); zone_children.mockResolvedValue([{ id: 'c1', parent_id: 'r1' }]); - const component = await make(); + const component = (await make()).componentInstance; const node = component.tree_nodes()[0]; expect(component.isExpanded(node)).toBe(true); @@ -150,7 +178,7 @@ describe('ZoneListComponent', () => { it('keeps a node unloaded with a retry when its children fail to load', async () => { root_zones.set([{ id: 'r1', name: 'Root 1', children_count: 1 }]); zone_children.mockRejectedValueOnce(new Error('Network')); - const component = await make(); + const component = (await make()).componentInstance; await vi.waitFor(() => expect(component.tree_nodes()[0].children_error).toBe(true), ); @@ -177,7 +205,7 @@ describe('ZoneListComponent', () => { { id: 'a', parent_id: 'b' }, { id: 'b', parent_id: 'a' }, ]); - const component = await make(); + const component = (await make()).componentInstance; expect(component['getZonePath']('a')).toEqual([]); }); @@ -185,7 +213,7 @@ describe('ZoneListComponent', () => { it('allows the automatically expanded root to be collapsed', async () => { root_zones.set([{ id: 'r1', name: 'Root 1', children_count: 1 }]); all_zones.set([{ id: 'r1' }, { id: 'c1', parent_id: 'r1' }]); - const component = await make(); + const component = (await make()).componentInstance; const node = component.tree_nodes()[0]; component.onExpandedChange(node, false); diff --git a/apps/signage-manager/src/tests/zones/zones.component.spec.ts b/apps/signage-manager/src/tests/zones/zones.component.spec.ts index 125ffcfd7cf..6eb377a8b81 100644 --- a/apps/signage-manager/src/tests/zones/zones.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zones.component.spec.ts @@ -187,6 +187,28 @@ describe('ZonesSectionComponent', () => { expect(showZone).toHaveBeenCalledExactlyOnceWith('z-far'); }); + it('loads a linked zone again after going back to it', async () => { + all_zones.set([{ id: 'z1' }]); + vi.mocked(showZone).mockResolvedValue( + new PlaceZone({ id: 'z-far', name: 'Far' }), + ); + const [, fixture] = await make(); + const route = (id: string) => { + fixture.componentRef.setInput('id', id); + fixture.detectChanges(); + TestBed.flushEffects(); + }; + + route('z-far'); + await vi.waitFor(() => expect(selected_zone()?.id).toBe('z-far')); + route('z1'); + expect(selected_zone()?.id).toBe('z1'); + route('z-far'); + + await vi.waitFor(() => expect(selected_zone()?.id).toBe('z-far')); + expect(showZone).toHaveBeenCalledTimes(2); + }); + it('clears the selection when navigating back to the list', async () => { all_zones.set([{ id: 'z1' }]); const [, fixture] = await make(); From 3f7d185e3b3344041a6eb344f15842acf77b8e1f Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 2 Oct 2026 13:26:07 +1000 Subject: [PATCH 2/3] fix(signage-manager): show zone count and playlist load failures - The signage zone count is hidden while it loads or after it fails, and counts toward the zone error so Retry refetches it. - A failed zone list shows an error with Retry above the zones that did load. - The display and zone playlist tabs show a load error with Retry instead of "no playlists". --- apps/signage-manager/USER_STORIES.md | 6 ++-- .../app/displays/display-content.component.ts | 8 +++++ .../src/app/zones/signage-zone.service.ts | 23 ++++++++++---- .../src/app/zones/zone-content.component.ts | 8 +++++ .../src/app/zones/zone-header.component.ts | 20 +++++++------ .../src/app/zones/zone-list.component.ts | 4 +++ .../display-content.component.spec.ts | 15 ++++++++++ .../src/tests/signage-zone-search.spec.ts | 30 +++++++++++++++++++ .../zones/zone-content.component.spec.ts | 15 ++++++++++ .../tests/zones/zone-header.component.spec.ts | 2 +- .../tests/zones/zone-list.component.spec.ts | 12 ++++++++ 11 files changed, 124 insertions(+), 19 deletions(-) diff --git a/apps/signage-manager/USER_STORIES.md b/apps/signage-manager/USER_STORIES.md index e5e55d72d83..0676ed175ef 100644 --- a/apps/signage-manager/USER_STORIES.md +++ b/apps/signage-manager/USER_STORIES.md @@ -228,7 +228,7 @@ These stories cover the current app workflows: **Acceptance Criteria:** - The zones page lists signage zones and supports direct routes to a selected zone. -- The header count shows the number of signage zones. When users search in a zone, it shows the number of results. +- The header count shows the number of signage zones. When users search in a zone, it shows the number of results. If the count cannot load, the header does not show it. - System administrators and signage group managers can create, edit, and delete signage zones. - New and edited signage zones keep the `signage` tag and require a parent from the active group's accessible zone tree. - Zone management controls are not shown for untagged parent zones in the tree. @@ -238,7 +238,7 @@ These stories cover the current app workflows: - 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. -- While the zone tree or a tab loads, it shows a loading state. If the zone tree or the display tab cannot load, it shows an error with a retry button. +- While the zone tree or a tab loads, it shows a loading state. If a zone list, the playlist tab, or the display tab cannot load, it shows an error with a retry button. The error shows above the zones that loaded. --- @@ -257,7 +257,7 @@ These stories cover the current app workflows: - The playlist tab shows playlists assigned directly to the display and their status. - Users with update permission can add or remove direct playlist assignments. - The zone tab shows zones assigned to the display. -- While the display list or a tab loads, it shows a loading state. If the list or the zone tab cannot load, it shows an error with a retry button. +- While the display list or a tab loads, it shows a loading state. If the list, the playlist tab, or the zone tab cannot load, it shows an error with a retry button. - If a link opens a display or zone that cannot load, an error message shows. --- diff --git a/apps/signage-manager/src/app/displays/display-content.component.ts b/apps/signage-manager/src/app/displays/display-content.component.ts index 28e703545a7..c9120ce28f0 100644 --- a/apps/signage-manager/src/app/displays/display-content.component.ts +++ b/apps/signage-manager/src/app/displays/display-content.component.ts @@ -226,6 +226,8 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; > {{ 'COMMON.LOADING' | translate }}
+ } @else if (playlists_error()) { + } @else {
mergeItems(loadedZones(this._zone_list), this._zone_overrides()), ); - /** Number of signage zones the server has, loaded or not */ + /** + * Number of signage zones the server has, loaded or not. Null while the + * count loads or after it fails, as the count is not known. + */ public readonly signage_zone_count = computed(() => - this._zone_list.hasValue() ? this._zone_list.value().total : 0, + this._zone_list.hasValue() ? this._zone_list.value().total : null, ); private readonly _all_zone_list = this._zoneResource((group_id) => @@ -172,14 +175,22 @@ export class SignageZoneService { ); }); - /** Whether the zones of the zone tree are loading */ + /** Whether the zone lists of the zones page are loading */ public readonly zones_loading = computed( () => - this._all_zone_list.isLoading() || this._org_root_list.isLoading(), + this._zone_list.isLoading() || + this._all_zone_list.isLoading() || + this._org_root_list.isLoading(), ); - /** Whether the zones of the zone tree failed to load */ + /** + * Whether a zone list of the zones page failed to load. The tree can + * still show the lists that loaded, so it shows this beside them. + */ public readonly zones_error = computed( - () => !!this._all_zone_list.error() || !!this._org_root_list.error(), + () => + !!this._zone_list.error() || + !!this._all_zone_list.error() || + !!this._org_root_list.error(), ); /** diff --git a/apps/signage-manager/src/app/zones/zone-content.component.ts b/apps/signage-manager/src/app/zones/zone-content.component.ts index 6ab1436c83c..9edbdb5a599 100644 --- a/apps/signage-manager/src/app/zones/zone-content.component.ts +++ b/apps/signage-manager/src/app/zones/zone-content.component.ts @@ -196,6 +196,8 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; > {{ 'COMMON.LOADING' | translate }}
+ } @else if (playlists_error()) { + } @else {
-
- {{ - 'COMMON.ITEM_COUNT' - | translate - : { count: total_count() } - : total_count() - }} -
+ @let count = total_count(); + @if (count !== null) { +
+ {{ + 'COMMON.ITEM_COUNT' + | translate: { count } : count + }} +
+ }
@@ -62,7 +63,8 @@ export class ZoneHeaderComponent { private readonly _router = inject(Router); /** Search results while searching, otherwise the server total of - * signage zones, as the tree also shows untagged parent zones */ + * signage zones, as the tree also shows untagged parent zones. Null + * when the total is not known. */ public readonly total_count = computed(() => this._zone_service.selected_zone()?.id && this._zone_service.zone_search_term().trim() diff --git a/apps/signage-manager/src/app/zones/zone-list.component.ts b/apps/signage-manager/src/app/zones/zone-list.component.ts index 6522339de61..7b55a017509 100644 --- a/apps/signage-manager/src/app/zones/zone-list.component.ts +++ b/apps/signage-manager/src/app/zones/zone-list.component.ts @@ -75,6 +75,10 @@ interface FlatZoneTreeNode extends ZoneTreeNode {
@if (tree_nodes().length) { + @if (error()) { + + + } { const zones_loading = signal(false); const zones_error = signal(false); const playlists_loading = signal(false); + const playlists_error = signal(false); + const reload_playlists = vi.fn(); const reload_zones = vi.fn(); const context_stub = { can_update }; const display_stub = { @@ -37,6 +39,8 @@ describe('DisplayContentComponent', () => { playlist_approval_status, playlist_thumbnail_media, playlists_loading, + playlists_error, + reloadPlaylists: reload_playlists, }; async function make() { @@ -83,6 +87,7 @@ describe('DisplayContentComponent', () => { zones_loading.set(false); zones_error.set(false); playlists_loading.set(false); + playlists_error.set(false); }); it('lists the playlists and the queried zones of the display', async () => { @@ -196,4 +201,14 @@ describe('DisplayContentComponent', () => { expect(element.querySelector('[role="status"]')).not.toBeNull(); expect(element.textContent).not.toContain('playlist_remove'); }); + + it('offers a retry when the playlists fail to load', async () => { + selected_display.set({ id: 'd1', playlists: ['p1'] }); + playlists_error.set(true); + const element = await render('playlists'); + + expect(element.textContent).not.toContain('playlist_remove'); + element.querySelector('load-error button')?.click(); + expect(reload_playlists).toHaveBeenCalledTimes(1); + }); }); diff --git a/apps/signage-manager/src/tests/signage-zone-search.spec.ts b/apps/signage-manager/src/tests/signage-zone-search.spec.ts index 3a68484baeb..525b68deb46 100644 --- a/apps/signage-manager/src/tests/signage-zone-search.spec.ts +++ b/apps/signage-manager/src/tests/signage-zone-search.spec.ts @@ -114,6 +114,36 @@ describe('SignageZoneService zone search', () => { ); }); + // The tree loads from other lists, so it can look complete without it + it('reports a failed signage zone count and loads it again on retry', async () => { + let fail_count = true; + vi.mocked(queryZones).mockImplementation(async (params) => { + if (params?.tags === 'signage' && fail_count) { + throw new Error('offline'); + } + return { + data: [new PlaceZone({ id: 'z1', tags: ['signage'] })], + total: 7, + next: null, + }; + }); + const service = TestBed.inject(SignageZoneService); + TestBed.tick(); + await flush(); + + expect(service.all_zones().map(({ id }) => id)).toEqual(['z1']); + expect(service.signage_zone_count()).toBeNull(); + expect(service.zones_error()).toBe(true); + + fail_count = false; + service.reloadZones(); + TestBed.tick(); + await flush(); + + expect(service.signage_zone_count()).toBe(7); + expect(service.zones_error()).toBe(false); + }); + it('searches selectable zones beneath the selected zone', () => { const service = TestBed.inject(SignageZoneService); diff --git a/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts index 2cc2e5bb41c..bdf820ebd66 100644 --- a/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts @@ -22,6 +22,8 @@ describe('ZoneContentComponent', () => { const displays_loading = signal(false); const displays_error = signal(false); const playlists_loading = signal(false); + const playlists_error = signal(false); + const reload_playlists = vi.fn(); const reload_displays = vi.fn(); const context_stub = { can_update }; const display_stub = { @@ -37,6 +39,8 @@ describe('ZoneContentComponent', () => { playlist_approval_status, playlist_thumbnail_media, playlists_loading, + playlists_error, + reloadPlaylists: reload_playlists, }; const zone_stub = { selected_zone, @@ -90,6 +94,7 @@ describe('ZoneContentComponent', () => { displays_loading.set(false); displays_error.set(false); playlists_loading.set(false); + playlists_error.set(false); }); it('lists the playlists and the queried displays of the zone', async () => { @@ -173,4 +178,14 @@ describe('ZoneContentComponent', () => { expect(panel.querySelector('[role="status"]')).not.toBeNull(); expect(panel.textContent).not.toContain('playlist_remove'); }); + + it('offers a retry when the playlists fail to load', async () => { + selected_zone.set({ id: 'z1', playlists: ['p1'] }); + playlists_error.set(true); + const panel = await render('playlists'); + + expect(panel.textContent).not.toContain('playlist_remove'); + panel.querySelector('load-error button')?.click(); + expect(reload_playlists).toHaveBeenCalledTimes(1); + }); }); diff --git a/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts index 01004df9a5a..49845360374 100644 --- a/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-header.component.spec.ts @@ -7,7 +7,7 @@ import { ZoneHeaderComponent } from '../../app/zones/zone-header.component'; describe('ZoneHeaderComponent', () => { const filtered_zones = signal<{ id: string }[]>([]); - const signage_zone_count = signal(0); + const signage_zone_count = signal(0); const selected_zone = signal<{ id: string } | null>(null); const zone_search_term = signal(''); const can_manage_zones = signal(false); diff --git a/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts index e8f0f94235b..aa2c7aa3460 100644 --- a/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-list.component.spec.ts @@ -1,5 +1,6 @@ import { signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; +import { provideRouter } from '@angular/router'; import { OrganisationService } from '@placeos/common'; import { SignageZoneService } from '../../app/zones/signage-zone.service'; import { ZoneListComponent } from '../../app/zones/zone-list.component'; @@ -36,6 +37,7 @@ describe('ZoneListComponent', () => { await TestBed.configureTestingModule({ imports: [ZoneListComponent], providers: [ + provideRouter([]), { provide: SignageZoneService, useValue: zone_stub }, { provide: OrganisationService, useValue: org_stub }, ], @@ -83,6 +85,16 @@ describe('ZoneListComponent', () => { expect(reload_zones).toHaveBeenCalledTimes(1); }); + it('offers a retry beside the zones that loaded when a list fails', async () => { + root_zones.set([{ id: 'org-1', name: 'Organisation' }]); + zones_error.set(true); + const element: HTMLElement = (await make(true)).nativeElement; + + expect(element.textContent).toContain('Organisation'); + element.querySelector('load-error button')?.click(); + expect(reload_zones).toHaveBeenCalledTimes(1); + }); + it('only enables search after selecting a zone', async () => { const component = (await make()).componentInstance; expect(component.search_enabled()).toBe(false); From eb329d6b1cdb3e67037947b5b7a5c49c8c8e0af1 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 2 Oct 2026 13:31:46 +1000 Subject: [PATCH 3/3] fix(signage-manager): keep the empty playlist tab for items with no playlists The playlist tabs showed the shared playlist list's loading or error state even when the display or zone had no playlists. That state now applies only when the item has playlist ids. --- .../src/app/displays/display-content.component.ts | 15 +++++++++++++-- .../src/app/zones/zone-content.component.ts | 13 +++++++++++-- .../displays/display-content.component.spec.ts | 15 +++++++++++++++ .../tests/zones/zone-content.component.spec.ts | 15 +++++++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/apps/signage-manager/src/app/displays/display-content.component.ts b/apps/signage-manager/src/app/displays/display-content.component.ts index c9120ce28f0..c15aa1b4885 100644 --- a/apps/signage-manager/src/app/displays/display-content.component.ts +++ b/apps/signage-manager/src/app/displays/display-content.component.ts @@ -219,14 +219,20 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; }
} - } @else if (playlists_loading()) { + } @else if ( + has_assigned_playlists() && + playlists_loading() + ) {
{{ 'COMMON.LOADING' | translate }}
- } @else if (playlists_error()) { + } @else if ( + has_assigned_playlists() && + playlists_error() + ) { } @else {
!!this.selected_display()?.playlists?.length, + ); public reloadPlaylists() { this._playlist_service.reloadPlaylists(); diff --git a/apps/signage-manager/src/app/zones/zone-content.component.ts b/apps/signage-manager/src/app/zones/zone-content.component.ts index 9edbdb5a599..ad8b65f4e28 100644 --- a/apps/signage-manager/src/app/zones/zone-content.component.ts +++ b/apps/signage-manager/src/app/zones/zone-content.component.ts @@ -189,14 +189,18 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; }
} - } @else if (playlists_loading()) { + } @else if ( + has_assigned_playlists() && playlists_loading() + ) {
{{ 'COMMON.LOADING' | translate }}
- } @else if (playlists_error()) { + } @else if ( + has_assigned_playlists() && playlists_error() + ) { } @else {
!!this.selected_zone()?.playlists?.length, + ); public reloadPlaylists() { this._playlist_service.reloadPlaylists(); diff --git a/apps/signage-manager/src/tests/displays/display-content.component.spec.ts b/apps/signage-manager/src/tests/displays/display-content.component.spec.ts index c874b1d115a..8ac12fc9687 100644 --- a/apps/signage-manager/src/tests/displays/display-content.component.spec.ts +++ b/apps/signage-manager/src/tests/displays/display-content.component.spec.ts @@ -211,4 +211,19 @@ describe('DisplayContentComponent', () => { element.querySelector('load-error button')?.click(); expect(reload_playlists).toHaveBeenCalledTimes(1); }); + + // The shared playlist list does not feed a tab with no playlist ids + it.each(['loading', 'error'] as const)( + 'keeps the empty state of a display with no playlists while the playlist list is in %s', + async (state) => { + selected_display.set({ id: 'd1', playlists: [] }); + playlists_loading.set(state === 'loading'); + playlists_error.set(state === 'error'); + const element = await render('playlists'); + + expect(element.textContent).toContain('playlist_remove'); + expect(element.querySelector('load-error')).toBeNull(); + expect(element.querySelector('[role="status"]')).toBeNull(); + }, + ); }); diff --git a/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts b/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts index bdf820ebd66..e4c48db2c30 100644 --- a/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts +++ b/apps/signage-manager/src/tests/zones/zone-content.component.spec.ts @@ -188,4 +188,19 @@ describe('ZoneContentComponent', () => { panel.querySelector('load-error button')?.click(); expect(reload_playlists).toHaveBeenCalledTimes(1); }); + + // The shared playlist list does not feed a tab with no playlist ids + it.each(['loading', 'error'] as const)( + 'keeps the empty state of a zone with no playlists while the playlist list is in %s', + async (state) => { + selected_zone.set({ id: 'z1', playlists: [] }); + playlists_loading.set(state === 'loading'); + playlists_error.set(state === 'error'); + const panel = await render('playlists'); + + expect(panel.textContent).toContain('playlist_remove'); + expect(panel.querySelector('load-error')).toBeNull(); + expect(panel.querySelector('[role="status"]')).toBeNull(); + }, + ); });