From c92aa6cbb214b4be3632a35d71a6e5af7e58703e Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 2 Oct 2026 13:12:35 +1000 Subject: [PATCH 1/2] fix(signage-manager): fix playlist approval, adding media and status - Fetch a playlist by ID to check distribution when it is not loaded. - Keep a pending approval request when items load. - Send null for cleared validity dates. - Re-check the revision before approving. - Show errors for add-media, item loading and deep links. - One playlistStatus helper for every playlist status badge. - Move items up and down with the keyboard. - Map animation indexes from the backend to their names, and keep a playlist's animation unless the user changes it. - Block the save hotkey while a dropdown has focus. --- apps/signage-manager/USER_STORIES.md | 14 +- .../app/displays/display-content.component.ts | 31 +- .../app/media/playlist-sidebar.component.ts | 31 +- .../playlist-item-details.component.ts | 21 +- .../app/playlists/playlist-items.component.ts | 599 +++++++++++------- .../app/playlists/playlist-list.component.ts | 34 +- .../src/app/playlists/playlists.component.ts | 57 +- .../app/playlists/signage-playlist.service.ts | 251 +++++--- .../src/app/shared/playlist-approval.util.ts | 16 + .../playlist-approve-modal.component.ts | 30 +- .../shared/playlist-edit-modal.component.ts | 109 ++-- .../playlist-item-schedule-modal.component.ts | 3 +- .../playlist-schedule-form.component.ts | 65 +- .../shared/playlist-select-modal.component.ts | 3 +- .../src/app/signage-playlist.util.ts | 108 +++- .../src/app/zones/zone-content.component.ts | 31 +- .../display-content.component.spec.ts | 1 + .../playlist-item-details.component.spec.ts | 48 +- .../playlist-items.component.spec.ts | 100 ++- .../playlists/playlists.component.spec.ts | 75 +++ .../signage-playlist.service.spec.ts | 151 ++++- .../playlist-approve-modal.component.spec.ts | 41 ++ .../playlist-edit-modal.component.spec.ts | 168 ++++- ...list-item-schedule-modal.component.spec.ts | 2 +- .../src/tests/signage-playlist.util.spec.ts | 99 +++ .../zones/zone-content.component.spec.ts | 1 + shared/assets/locale/ar.json | 1 - shared/assets/locale/en-AU.json | 6 +- shared/assets/locale/en-GB.json | 6 +- shared/assets/locale/en-US.json | 6 +- shared/assets/locale/es.json | 1 - shared/assets/locale/fr-CA.json | 1 - shared/assets/locale/fr.json | 1 - shared/assets/locale/it.json | 1 - shared/assets/locale/ja-JP.json | 1 - shared/assets/locale/pt.json | 1 - shared/assets/locale/zh.json | 1 - 37 files changed, 1528 insertions(+), 587 deletions(-) diff --git a/apps/signage-manager/USER_STORIES.md b/apps/signage-manager/USER_STORIES.md index e8ce7068ac9..255c2bebff5 100644 --- a/apps/signage-manager/USER_STORIES.md +++ b/apps/signage-manager/USER_STORIES.md @@ -134,11 +134,12 @@ These stories cover the current app workflows: - The playlists page shows a searchable playlist list. - Playlist rows show thumbnail previews when available. -- Playlist rows show disabled, expired, pending, awaiting review, and approval-required states. +- Playlist rows show disabled, expired, pending, awaiting review, and approval-required states. A playlist is expired when its end date has passed or all its schedules have ended. Playlist lists on the media, zone, and display pages show the same states. - Additional playlists load as the user scrolls. - Users with create permission can create a new playlist. - Users with create permission can duplicate a playlist. The copy has the same settings, items, and item schedules. It is not approved and is not assigned to displays or zones. - Selecting a playlist opens its items and details. +- A link to a playlist opens it, also when the loaded list does not include it. When the playlist cannot load, a warning shows and no playlist is selected. --- @@ -150,14 +151,15 @@ These stories cover the current app workflows: **Acceptance Criteria:** -- Users can view media items in the selected playlist. +- Users can view media items in the selected playlist. When the items cannot load, the list shows an error with a retry action. - Non-distribution playlists show the item count and the time to play each item once. The time uses the same fallbacks as the player: item play time, video length, playlist default, then 15 seconds. - Users can preview a playlist item. -- Users with update permission can reorder playlist items by drag and drop. +- Users with update permission can reorder playlist items by drag and drop, or with the move up and move down actions in the item menu. - Distribution playlists cannot be reordered from the item list. - Users with update permission can remove media from the playlist. +- Adding media to a distribution playlist always asks for the item schedule first. When adding media fails, an error shows. - Users can expand, collapse, and edit item schedules. -- Keyboard selection is supported for playlist items. +- Keyboard selection is supported for playlist items. Enter and Space on the checkbox or the actions button of a row operate that control. --- @@ -170,7 +172,7 @@ These stories cover the current app workflows: **Acceptance Criteria:** - Users can view playlist item count, enabled state, description, validity dates, animation, schedules, and next play sessions. -- Users with update permission can edit playlist details. +- Users with update permission can edit playlist details. Clearing a validity date removes it. - Users with update permission can add or remove display assignments. - Users with update permission can add or remove zone assignments. - Users with share permission can share playlists. @@ -192,6 +194,8 @@ These stories cover the current app workflows: - Users without approval permission can request approval for a selected playlist. - Approval request actions show a loading state while submitting. - Approval preview shows changed media versions and allows media preview. +- Approval applies only to the version that the reviewer saw. When the playlist changed after the preview loaded, the preview shows the new version with a warning, and the playlist is not approved. +- A playlist that is awaiting review keeps that state when the user selects it. --- 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 c15aa1b4885..775870a0fd7 100644 --- a/apps/signage-manager/src/app/displays/display-content.component.ts +++ b/apps/signage-manager/src/app/displays/display-content.component.ts @@ -12,11 +12,10 @@ import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { PlaylistThumbnailComponent } from '../shared/playlist-thumbnail.component'; import { TemplateMappingsComponent } from '../shared/template-mappings.component'; import { SignageContextService } from '../signage-context.service'; +import { playlistStatus } from '../signage-playlist.util'; import { DisplayScheduleComponent } from './display-schedule.component'; import { SignageDisplayService } from './signage-display.service'; -type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; - @Component({ selector: 'display-content', template: ` @@ -166,12 +165,22 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; }} } + @case ('awaiting_review') { + + {{ + 'SIGNAGE_MANAGER.STATUS_AWAITING_REVIEW' + | translate + }} + + } @case ('awaiting_approval') { {{ - 'SIGNAGE_MANAGER.STATUS_AWAITING_APPROVAL' + 'COMMON.APPROVAL_REQUIRED' | translate }} @@ -441,15 +450,11 @@ export class DisplayContentComponent { ); } - public getStatus(playlist: SignagePlaylist): PlaylistStatus { - const now_s = Math.floor(Date.now() / 1000); - if (playlist.valid_until && playlist.valid_until < now_s) - return 'expired'; - if (playlist.valid_from && playlist.valid_from > now_s) - return 'pending'; - const approvals = this.playlist_approval_status(); - if (playlist.id in approvals && !approvals[playlist.id]) - return 'awaiting_approval'; - return null; + public getStatus(playlist: SignagePlaylist) { + return playlistStatus( + playlist, + this.playlist_approval_status(), + this._playlist_service.playlist_approval_requested_status(), + ); } } diff --git a/apps/signage-manager/src/app/media/playlist-sidebar.component.ts b/apps/signage-manager/src/app/media/playlist-sidebar.component.ts index ac8baf76228..638271eec72 100644 --- a/apps/signage-manager/src/app/media/playlist-sidebar.component.ts +++ b/apps/signage-manager/src/app/media/playlist-sidebar.component.ts @@ -13,13 +13,7 @@ import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { IntersectDirective } from '../shared/intersect.directive'; import { PlaylistThumbnailComponent } from '../shared/playlist-thumbnail.component'; import { SignageContextService } from '../signage-context.service'; - -type PlaylistStatus = - | 'expired' - | 'pending' - | 'awaiting_approval' - | 'awaiting_review' - | null; +import { playlistStatus } from '../signage-playlist.util'; @Component({ selector: 'playlist-sidebar', @@ -269,22 +263,11 @@ export class PlaylistSidebarComponent { ); } - public getStatus(playlist: SignagePlaylist): PlaylistStatus { - const now_s = Math.floor(Date.now() / 1000); - if (playlist.valid_until && playlist.valid_until < now_s) - return 'expired'; - if (playlist.valid_from && playlist.valid_from > now_s) - return 'pending'; - const approvals = this.playlist_approval_status(); - const approval_requests = this.playlist_approval_requested_status(); - if ( - playlist.id in approvals && - !approvals[playlist.id] && - approval_requests[playlist.id] - ) - return 'awaiting_review'; - if (playlist.id in approvals && !approvals[playlist.id]) - return 'awaiting_approval'; - return null; + public getStatus(playlist: SignagePlaylist) { + return playlistStatus( + playlist, + this.playlist_approval_status(), + this.playlist_approval_requested_status(), + ); } } diff --git a/apps/signage-manager/src/app/playlists/playlist-item-details.component.ts b/apps/signage-manager/src/app/playlists/playlist-item-details.component.ts index 47bcaa6c66e..b12b6d4b70f 100644 --- a/apps/signage-manager/src/app/playlists/playlist-item-details.component.ts +++ b/apps/signage-manager/src/app/playlists/playlist-item-details.component.ts @@ -15,13 +15,19 @@ import { MediaDurationPipe, TranslatePipe, } from '@placeos/components'; -import { MediaAnimation, SignagePlaylist } from '@placeos/ts-client'; +import { + MediaAnimation, + PlaceSystem, + PlaceZone, + SignagePlaylist, +} from '@placeos/ts-client'; import { SignageDisplayService } from '../displays/signage-display.service'; import { playlistSchedules } from '../schedules/signage-schedule.util'; import { SignageSharedWithComponent } from '../shared/signage-shared-with.component'; import { SignageContextService } from '../signage-context.service'; import { SignageInventoryService } from '../signage-inventory.service'; import { + playlistAnimation, playlistNextPlayLabels, playlistScheduleExpiryTooltip, playlistScheduleLabel, @@ -643,7 +649,12 @@ export class PlaylistItemDetailsComponent { loader: () => this._inventory_service.loadSignageInventory(), }); - public readonly item_count = computed(() => this._items().length); + /** Number of items. A dash while the items cannot load. */ + public readonly item_count = computed(() => + this._playlist_service.playlist_media_error() + ? '—' + : this._items().length, + ); public readonly can_update = this._context.can_update; public readonly selected_group_id = computed( () => this._context.selected_group()?.group.id || '', @@ -670,7 +681,7 @@ export class PlaylistItemDetailsComponent { public readonly animation_label = computed(() => { const pl = this.playlist(); if (!pl) return 'COMMON.DEFAULT'; - switch (pl.default_animation) { + switch (playlistAnimation(pl)) { case MediaAnimation.Cut: return 'SIGNAGE_MANAGER.ANIM_CUT'; case MediaAnimation.CrossFade: @@ -732,7 +743,7 @@ export class PlaylistItemDetailsComponent { if (playlist) this._zone_service.addZoneToPlaylist(playlist); } - public removeDisplay(event: Event, display: any) { + public removeDisplay(event: Event, display: PlaceSystem) { event.preventDefault(); event.stopPropagation(); const playlist = this.playlist(); @@ -740,7 +751,7 @@ export class PlaylistItemDetailsComponent { this._display_service.removeDisplayFromPlaylist(playlist, display); } - public removeZone(event: Event, zone: any) { + public removeZone(event: Event, zone: PlaceZone) { event.preventDefault(); event.stopPropagation(); const playlist = this.playlist(); diff --git a/apps/signage-manager/src/app/playlists/playlist-items.component.ts b/apps/signage-manager/src/app/playlists/playlist-items.component.ts index 3e644c1d836..35ac8d50ea1 100644 --- a/apps/signage-manager/src/app/playlists/playlist-items.component.ts +++ b/apps/signage-manager/src/app/playlists/playlist-items.component.ts @@ -4,11 +4,15 @@ import { moveItemInArray, } from '@angular/cdk/drag-drop'; import { + afterNextRender, Component, computed, + ElementRef, inject, + Injector, linkedSignal, signal, + viewChildren, } from '@angular/core'; import { MatCheckboxModule } from '@angular/material/checkbox'; import { MatRippleModule } from '@angular/material/core'; @@ -18,6 +22,7 @@ import { MatTooltipModule } from '@angular/material/tooltip'; import { i18n } from '@placeos/common'; import { IconComponent, + LoadErrorComponent, MediaDurationPipe, TranslatePipe, } from '@placeos/components'; @@ -84,6 +89,8 @@ import { SignagePlaylistService } from './signage-playlist.service'; }}

+ } @else if (load_error()) { + } @else if (items().length > 0 && !is_distribution()) {
@for (item of items(); track item.id + '-' + $index) { -
- drag_indicator - +
- -
-
- {{ item.name }} -
-
- - {{ item.media_type }} - - @if (item.play_time) { - - {{ - item.play_time / 1000 - | mediaDuration - }} - - } -
-
- - - - @if (can_update()) { - - } - -
- } -
- } @else if (items().length > 0) { -
-
- -
- @for (item of items(); track item.id + '-' + $index) { - @let schedule = itemSchedule(item, $index); -
-
+ +
+ } +
+ } @else if (items().length > 0) { +
+
+ +
+ @for (item of items(); track item.id + '-' + $index) { + @let schedule = itemSchedule(item, $index); +
- - @if (schedulesOpen(item, $index)) { - @if (schedule?.schedules?.length) { - @for ( - item_schedule of schedule.schedules; - track $index - ) { +
+ + +
+
+ {{ item.name }} +
+
+ + {{ item.media_type }} + + @if (item.play_time) { + + {{ + item.play_time / 1000 + | mediaDuration + }} + + } +
+
+ + + + @if (can_update()) { + + + } + +
+
+ + @if (schedulesOpen(item, $index)) { + @if (schedule?.schedules?.length) { + @for ( + item_schedule of schedule.schedules; + track $index + ) { +
+ {{ + scheduleLabel( + item_schedule + ) + }} +
+ } + } @else { +
{{ - scheduleLabel(item_schedule) + 'SIGNAGE_MANAGER.NO_SCHEDULES' + | translate }}
} - } @else { -
- {{ - 'SIGNAGE_MANAGER.NO_SCHEDULES' - | translate - }} -
} - } +
} @@ -598,6 +683,7 @@ import { SignagePlaylistService } from './signage-playlist.service'; MatProgressSpinnerModule, MatTooltipModule, IconComponent, + LoadErrorComponent, MediaDurationPipe, TranslatePipe, MediaThumbnailComponent, @@ -608,6 +694,8 @@ export class PlaylistItemsComponent { private readonly _context = inject(SignageContextService); private readonly _media_service = inject(SignageMediaService); private readonly _playlist_service = inject(SignagePlaylistService); + private readonly _injector = inject(Injector); + private readonly _rows = viewChildren>('item_row'); public readonly selected_playlist = this._playlist_service.selected_playlist; @@ -623,6 +711,8 @@ export class PlaylistItemsComponent { this._playlist_service.playlist_media_loading() && !this.items().length, ); + /** Whether the items failed to load */ + public readonly load_error = this._playlist_service.playlist_media_error; /** Time in milliseconds to play each item once */ public readonly loop_duration = computed(() => playlistLoopDuration( @@ -761,11 +851,17 @@ export class PlaylistItemsComponent { this.collapsed_schedules.set(collapsed); } + /** + * Select an item with Enter or Space on its row. Keys from controls in + * the row, such as the checkbox and the actions button, keep their own + * behaviour. + */ public selectItemWithKeyboard( event: Event, item: SignageMedia, index: number, ) { + if (event.target !== event.currentTarget) return; event.preventDefault(); event.stopPropagation(); this.selectItem(item, index); @@ -828,17 +924,38 @@ export class PlaylistItemsComponent { } } + public reloadItems() { + this._playlist_service.reloadPlaylistMedia(); + } + public async onDrop(event: CdkDragDrop) { + await this._reorder(event.previousIndex, event.currentIndex); + } + + /** + * Move an item one place, for users who cannot drag. Keeps the focus + * on the moved item, so it can be moved again. + * @param offset -1 to move up, 1 to move down + */ + public async moveItem(index: number, offset: -1 | 1) { + const target = index + offset; + const saved = this._reorder(index, target); + afterNextRender(() => this._rows()[target]?.nativeElement.focus(), { + injector: this._injector, + }); + await saved; + } + + private async _reorder(from: number, to: number) { if (!this.can_update() || this.is_distribution()) return; const playlist = this.selected_playlist(); - if (!playlist?.id) return; - if (event.previousIndex === event.currentIndex) return; const current_items = [...this.items()]; - moveItemInArray(current_items, event.previousIndex, event.currentIndex); - const media_ids = current_items.map((m) => m.id); + if (!playlist?.id || from === to) return; + if (to < 0 || to >= current_items.length) return; + moveItemInArray(current_items, from, to); await this._playlist_service.reorderPlaylistMedia( playlist.id, - media_ids, + current_items.map((m) => m.id), ); } } diff --git a/apps/signage-manager/src/app/playlists/playlist-list.component.ts b/apps/signage-manager/src/app/playlists/playlist-list.component.ts index af583476b14..dce562dd689 100644 --- a/apps/signage-manager/src/app/playlists/playlist-list.component.ts +++ b/apps/signage-manager/src/app/playlists/playlist-list.component.ts @@ -19,15 +19,9 @@ import { import { SignagePlaylist } from '@placeos/ts-client'; import { IntersectDirective } from '../shared/intersect.directive'; import { PlaylistThumbnailComponent } from '../shared/playlist-thumbnail.component'; +import { playlistStatus } from '../signage-playlist.util'; import { SignagePlaylistService } from './signage-playlist.service'; -type PlaylistStatus = - | 'expired' - | 'pending' - | 'awaiting_approval' - | 'awaiting_review' - | null; - @Component({ selector: 'playlist-list', template: ` @@ -46,8 +40,7 @@ type PlaylistStatus = [placeholder]=" 'SIGNAGE_MANAGER.SEARCH_PLAYLISTS' | translate " - [ngModel]="search()" - (ngModelChange)="search.set($event)" + [(ngModel)]="search" [attr.aria-label]=" 'SIGNAGE_MANAGER.SEARCH_PLAYLISTS' | translate " @@ -271,22 +264,11 @@ export class PlaylistListComponent { this._playlist_service.reloadPlaylists(); } - public getStatus(playlist: SignagePlaylist): PlaylistStatus { - const now_s = Math.floor(Date.now() / 1000); - if (playlist.valid_until && playlist.valid_until < now_s) - return 'expired'; - if (playlist.valid_from && playlist.valid_from > now_s) - return 'pending'; - const approvals = this.playlist_approval_status(); - const approval_requests = this.playlist_approval_requested_status(); - if ( - playlist.id in approvals && - !approvals[playlist.id] && - approval_requests[playlist.id] - ) - return 'awaiting_review'; - if (playlist.id in approvals && !approvals[playlist.id]) - return 'awaiting_approval'; - return null; + public getStatus(playlist: SignagePlaylist) { + return playlistStatus( + playlist, + this.playlist_approval_status(), + this.playlist_approval_requested_status(), + ); } } diff --git a/apps/signage-manager/src/app/playlists/playlists.component.ts b/apps/signage-manager/src/app/playlists/playlists.component.ts index a9c90bce3d5..7bfa861c60d 100644 --- a/apps/signage-manager/src/app/playlists/playlists.component.ts +++ b/apps/signage-manager/src/app/playlists/playlists.component.ts @@ -1,9 +1,11 @@ import { Component, effect, inject, input, signal } from '@angular/core'; import { MatRippleModule } from '@angular/material/core'; import { ActivatedRoute, Router } from '@angular/router'; +import { i18n, notifyWarn } from '@placeos/common'; import { IconComponent, TranslatePipe } from '@placeos/components'; import { NavFooterComponent } from '../shared/nav-footer.component'; import { NavSidebarComponent } from '../shared/nav-sidebar.component'; +import { SignageContextService } from '../signage-context.service'; import { PlaylistActionsComponent } from './playlist-actions.component'; import { PlaylistHeaderComponent } from './playlist-header.component'; import { PlaylistItemDetailsComponent } from './playlist-item-details.component'; @@ -190,6 +192,7 @@ function parsePlaylistTab(value: string | null): 'items' | 'details' { ], }) export class PlaylistsSectionComponent { + private readonly _context = inject(SignageContextService); private readonly _playlist_service = inject(SignagePlaylistService); private readonly _route = inject(ActivatedRoute); private readonly _router = inject(Router); @@ -221,17 +224,19 @@ export class PlaylistsSectionComponent { effect(() => { const id = this.id(); const list = this._playlists(); - if (!list.length) return; - if (id) { - const match = list.find((p) => p.id === id); - if (!match && this._fetched_id !== id) { - this._fetched_id = id; - void this._playlist_service.loadPlaylist(id); + if (!id) { + if (this._route_resolved) { + this._playlist_service.selected_playlist.set(null); + this._playlist_service.selected_playlist_item.set(null); + this._playlist_service.selected_playlist_item_index.set( + null, + ); } - if ( - match && - this._playlist_service.selected_playlist() !== match - ) { + return; + } + const match = list.find((p) => p.id === id); + if (match) { + if (this._playlist_service.selected_playlist() !== match) { this._playlist_service.selected_playlist.set(match); this._playlist_service.selected_playlist_item.set(null); this._playlist_service.selected_playlist_item_index.set( @@ -239,11 +244,22 @@ export class PlaylistsSectionComponent { ); } this._route_resolved = true; - } else if (this._route_resolved) { - this._playlist_service.selected_playlist.set(null); - this._playlist_service.selected_playlist_item.set(null); - this._playlist_service.selected_playlist_item_index.set(null); + return; + } + // Wait for the first page, which usually holds the playlist, + // before fetching it on its own. The page can also be empty, + // e.g. for a search with no results. + if ( + !this._context.canQueryLists() || + this._playlist_service.playlists_loading() + ) { + return; } + if (this._fetched_id !== id) { + this._fetched_id = id; + void this._loadLinkedPlaylist(id); + } + this._route_resolved = true; }); // Sync selected media item from query param @@ -265,6 +281,19 @@ export class PlaylistsSectionComponent { }); } + /** + * Fetch a linked playlist that the loaded pages lack. When it cannot + * load, clear the selection, so no other playlist shows under its link. + */ + private async _loadLinkedPlaylist(id: string) { + if (await this._playlist_service.loadPlaylist(id)) return; + notifyWarn(i18n('SIGNAGE_MANAGER.PLAYLIST_NOT_FOUND')); + if (this.id() !== id) return; + this._playlist_service.selected_playlist.set(null); + this._playlist_service.selected_playlist_item.set(null); + this._playlist_service.selected_playlist_item_index.set(null); + } + public deselectPlaylist() { this._playlist_service.selected_playlist.set(null); this._playlist_service.selected_playlist_item.set(null); diff --git a/apps/signage-manager/src/app/playlists/signage-playlist.service.ts b/apps/signage-manager/src/app/playlists/signage-playlist.service.ts index 87cbf7583e2..c7d2d5c4512 100644 --- a/apps/signage-manager/src/app/playlists/signage-playlist.service.ts +++ b/apps/signage-manager/src/app/playlists/signage-playlist.service.ts @@ -59,6 +59,7 @@ import { SignageInventoryService, } from '../signage-inventory.service'; import { + playlistAnimation, playlistItemScheduleMap, playlistMediaIds, playlistMediaItems, @@ -459,7 +460,6 @@ export class SignagePlaylistService { } private readonly _playlist_change = signal(Date.now()); - public readonly playlist_media_loading = signal(false); // Keyed by id, so a new copy of the selected playlist from a list reload // does not load the media again. @@ -473,37 +473,46 @@ export class SignagePlaylistService { }), loader: async ({ params }) => { const { playlist_id } = params; - if (!playlist_id) { - this.playlist_media_loading.set(false); - return null as SignagePlaylistMedia | null; - } - this.playlist_media_loading.set(true); - try { - const result = await listSignagePlaylistMedia(playlist_id); - this._setPlaylistMediaState( - playlist_id, - result.items || [], - result.approved, - result.schedules, - ); - return result; - } catch { - return null as SignagePlaylistMedia | null; - } finally { - this.playlist_media_loading.set(false); - } + if (!playlist_id) return null as SignagePlaylistMedia | null; + const result = await listSignagePlaylistMedia(playlist_id); + this._setPlaylistMediaState(playlist_id, result.items || [], { + approved: result.approved, + approval_requested: result.approval_requested, + schedules: result.schedules, + }); + return result; }, }); + /** Whether the items of the selected playlist are loading */ + public readonly playlist_media_loading = computed(() => + this._playlist_media_items.isLoading(), + ); + /** Whether the items of the selected playlist failed to load */ + public readonly playlist_media_error = computed( + () => this._playlist_media_items.status() === 'error', + ); public readonly playlist_media_items = computed(() => - playlistMediaItems(this._playlist_media_items.value() || {}), + playlistMediaItems(this._mediaList() || {}), ); public readonly playlist_item_schedules = computed(() => - playlistItemScheduleMap(this._playlist_media_items.value() || {}), + playlistItemScheduleMap(this._mediaList() || {}), ); public readonly playlist_item_schedule_list = computed( - () => this._playlist_media_items.value()?.schedules || [], + () => this._mediaList()?.schedules || [], ); + /** Load the items of the selected playlist again, e.g. after an error */ + public reloadPlaylistMedia() { + this._playlist_media_items.reload(); + } + + /** Media list of the selected playlist. Null while none is loaded. */ + private _mediaList() { + return this._playlist_media_items.hasValue() + ? this._playlist_media_items.value() + : null; + } + public async addPlaylist() { if ( !this._context.requirePermission( @@ -640,7 +649,7 @@ export class SignagePlaylistService { enabled: playlist.enabled, distribution: playlist.distribution, random: playlist.random, - default_animation: playlist.default_animation, + default_animation: playlistAnimation(playlist), orientation: playlist.orientation, default_duration: playlist.default_duration, schedules: playlist.distribution @@ -784,7 +793,7 @@ export class SignagePlaylistService { ) ) return; - const previous = this._playlist_media_items.value(); + const previous = this._mediaList(); let media_list: SignagePlaylistMedia; let new_items: string[]; try { @@ -807,12 +816,10 @@ export class SignagePlaylistService { notifyError(i18n('SIGNAGE_MANAGER.SVC_ERR_REMOVE_PLAYLIST_ITEMS')); return; } - this._setPlaylistMediaState( - playlist_id, - new_items, - false, - media_list.schedules, - ); + this._setPlaylistMediaState(playlist_id, new_items, { + approved: false, + schedules: media_list.schedules, + }); notifySuccess(i18n('SIGNAGE_MANAGER.SVC_ITEM_REMOVED')); this._context.changed(); } @@ -845,7 +852,7 @@ export class SignagePlaylistService { this._dialog, ); if (result.reason !== 'done') return false; - const previous = this._playlist_media_items.value(); + const previous = this._mediaList(); let media_list: SignagePlaylistMedia; const new_items: string[] = []; let removed_count = 0; @@ -875,12 +882,10 @@ export class SignagePlaylistService { notifyError(i18n('SIGNAGE_MANAGER.SVC_ERR_REMOVE_PLAYLIST_ITEMS')); return false; } - this._setPlaylistMediaState( - playlist_id, - new_items, - false, - media_list.schedules, - ); + this._setPlaylistMediaState(playlist_id, new_items, { + approved: false, + schedules: media_list.schedules, + }); const selected_index = this.selected_playlist_item_index(); if ( selected_index !== null && @@ -917,7 +922,7 @@ export class SignagePlaylistService { ) ) return; - const previous = this._playlist_media_items.value(); + const previous = this._mediaList(); const loaded = this._selected_playlist_id() === playlist_id; const items = reorderPlaylistItemIds( (loaded && previous?.items) || [], @@ -931,7 +936,7 @@ export class SignagePlaylistService { notifyError(i18n('SIGNAGE_MANAGER.SVC_ERR_REORDER_PLAYLIST')); return; } - this._setPlaylistMediaState(playlist_id, items, false); + this._setPlaylistMediaState(playlist_id, items, { approved: false }); } /** @@ -1027,8 +1032,7 @@ export class SignagePlaylistService { this._setPlaylistMediaState( playlist_id, media_list.items || [], - false, - media_list.schedules, + { approved: false, schedules: media_list.schedules }, ); return media_list; }, @@ -1057,14 +1061,27 @@ export class SignagePlaylistService { ) return; await updateSignagePlaylistMedia(playlist_id, list); - this._setPlaylistMediaState(playlist_id, list, false); + this._setPlaylistMediaState(playlist_id, list, { approved: false }); notifySuccess(i18n('SIGNAGE_MANAGER.SVC_PLAYLIST_UPDATED')); this._playlist_change.set(Date.now()); } + /** + * The playlist record, from the loaded pages or fetched by ID. Pickers + * search the backend, so their playlist may not be in the loaded pages. + * @throws When the playlist cannot be loaded + */ + private async _playlistRecord(playlist_id: string) { + const playlist = + this._playlist_cache()[playlist_id] || + (await this.loadPlaylist(playlist_id)); + if (!playlist) throw new Error(`Playlist ${playlist_id} not found`); + return playlist; + } + /** * Add media to the end of a playlist. A distribution playlist asks for - * the schedule of the media first. + * the schedule of the media first. Shows an error when the add fails. * @param media Record of the media, shown in the schedule modal */ public async addMediaToPlaylist( @@ -1079,38 +1096,48 @@ export class SignagePlaylistService { ) ) return; - const media_list = await listSignagePlaylistMedia(playlist_id); - if (media_list.items?.includes(media_id)) { - const result = await openConfirmModal( - { - title: i18n('SIGNAGE_MANAGER.SVC_ADD_DUPLICATE_TITLE'), - content: i18n('SIGNAGE_MANAGER.SVC_ADD_DUPLICATE_CONTENT'), - icon: { content: 'playlist_add' }, - }, - this._dialog, - ); - if (result.reason !== 'done') return; - result.close(); - } - const playlist = this.playlists().find( - (item) => item.id === playlist_id, - ); - const new_items = [...(media_list.items || []), media_id]; - if (playlist?.distribution) { - await this._scheduleMediaForDistributionPlaylist( - playlist_id, + try { + const [playlist, media_list] = await Promise.all([ + this._playlistRecord(playlist_id), + listSignagePlaylistMedia(playlist_id), + ]); + if (media_list.items?.includes(media_id)) { + const result = await openConfirmModal( + { + title: i18n('SIGNAGE_MANAGER.SVC_ADD_DUPLICATE_TITLE'), + content: i18n( + 'SIGNAGE_MANAGER.SVC_ADD_DUPLICATE_CONTENT', + ), + icon: { content: 'playlist_add' }, + }, + this._dialog, + ); + if (result.reason !== 'done') return; + result.close(); + } + if (playlist.distribution) { + await this._scheduleMediaForDistributionPlaylist( + playlist_id, + media_id, + media, + ); + return; + } + await this._updatePlaylistMedia(playlist_id, [ + ...(media_list.items || []), media_id, - media, - ); - return; + ]); + } catch { + notifyError(i18n('SIGNAGE_MANAGER.SVC_ERR_ADD_PLAYLIST_ITEMS')); } - await this._updatePlaylistMedia(playlist_id, new_items); } /** * Add media items that the playlist does not hold yet to its end. + * Shows an error when the add fails. * @param media Records of the media, shown in the schedule modal of a * distribution playlist + * @returns Whether the media was added */ public async addMediaItemsToPlaylist( playlist_id: string, @@ -1126,34 +1153,40 @@ export class SignagePlaylistService { return false; const unique_media_ids = [...new Set(media_ids)].filter(Boolean); if (!playlist_id || !unique_media_ids.length) return false; - const playlist = this.playlists().find( - (item) => item.id === playlist_id, - ); - const media_list = await listSignagePlaylistMedia(playlist_id); - const existing_items = media_list.items || []; - const new_media_ids = unique_media_ids.filter( - (id) => !existing_items.includes(id), - ); - if (!new_media_ids.length) { - notifyWarn(i18n('SIGNAGE_MANAGER.SVC_MEDIA_ALREADY_IN')); - return false; - } - if (playlist?.distribution) { - for (const media_id of new_media_ids) { - const added = await this._scheduleMediaForDistributionPlaylist( - playlist_id, - media_id, - media.find(({ id }) => id === media_id), - ); - if (!added) return false; + try { + const [playlist, media_list] = await Promise.all([ + this._playlistRecord(playlist_id), + listSignagePlaylistMedia(playlist_id), + ]); + const existing_items = media_list.items || []; + const new_media_ids = unique_media_ids.filter( + (id) => !existing_items.includes(id), + ); + if (!new_media_ids.length) { + notifyWarn(i18n('SIGNAGE_MANAGER.SVC_MEDIA_ALREADY_IN')); + return false; + } + if (playlist.distribution) { + for (const media_id of new_media_ids) { + const added = + await this._scheduleMediaForDistributionPlaylist( + playlist_id, + media_id, + media.find(({ id }) => id === media_id), + ); + if (!added) return false; + } + return true; } + await this._updatePlaylistMedia(playlist_id, [ + ...existing_items, + ...new_media_ids, + ]); return true; + } catch { + notifyError(i18n('SIGNAGE_MANAGER.SVC_ERR_ADD_PLAYLIST_ITEMS')); + return false; } - await this._updatePlaylistMedia(playlist_id, [ - ...existing_items, - ...new_media_ids, - ]); - return true; } private _needsPlaylistMetaRefresh(playlist: SignagePlaylist) { @@ -1240,17 +1273,26 @@ export class SignagePlaylistService { }); } + /** + * Keep the items and approval state of a playlist for its list row. + * @param state Approval flags and item schedules. A flag that is not set + * keeps its value, except that a local change (`approved: false`) also + * clears the approval request. + */ private _setPlaylistMediaState( playlist_id: string, item_ids: string[], - approved?: boolean, - schedules?: SignagePlaylistItemSchedule[], + state: { + approved?: boolean; + approval_requested?: boolean; + schedules?: SignagePlaylistItemSchedule[]; + } = {}, ) { + const { approved, approval_requested, schedules } = state; // Distribution playlist items are schedule item ids; map them to the // scheduled media ids so thumbnail URLs resolve. const schedule_map = playlistItemScheduleMap({ - schedules: - schedules || this._playlist_media_items.value()?.schedules, + schedules: schedules || this._mediaList()?.schedules, }); const media_ids = item_ids.map( (id) => schedule_map.get(id)?.media?.id || id, @@ -1266,9 +1308,10 @@ export class SignagePlaylistService { current_state?.updated_at || playlist?.updated_at || Date.now(), approved: approved ?? current_state?.approved, approval_requested: - approved === false + approval_requested ?? + (approved === false ? false - : (current_state?.approval_requested ?? false), + : (current_state?.approval_requested ?? false)), }); } @@ -1315,12 +1358,10 @@ export class SignagePlaylistService { ); if (updated_items.length === current_items.length) continue; await updateSignagePlaylistMedia(playlist_id, updated_items); - this._setPlaylistMediaState( - playlist_id, - updated_items, - false, - list.schedules, - ); + this._setPlaylistMediaState(playlist_id, updated_items, { + approved: false, + schedules: list.schedules, + }); } const selected_item = this.selected_playlist_item(); if (selected_item?.id && removed_ids.has(selected_item.id)) { diff --git a/apps/signage-manager/src/app/shared/playlist-approval.util.ts b/apps/signage-manager/src/app/shared/playlist-approval.util.ts index ab1d6cbc73e..0b31a062043 100644 --- a/apps/signage-manager/src/app/shared/playlist-approval.util.ts +++ b/apps/signage-manager/src/app/shared/playlist-approval.util.ts @@ -24,3 +24,19 @@ export async function loadPlaylistApprovalVersions( const approved = older.find((revision) => revision.approved); return approved ? [latest, approved] : [latest]; } + +/** + * Whether the media list of a playlist has changed since a revision was + * loaded. The approve action approves the latest revision, so check this + * first to approve only what the approver saw. + * @param shown Latest revision when the approver loaded the changes + */ +export async function playlistChangedSince( + playlist_id: string, + shown: SignagePlaylistMedia | undefined, +) { + const [latest] = await listSignagePlaylistMediaRevisions(playlist_id, { + limit: 1, + }); + return latest?.id !== shown?.id || latest?.updated_at !== shown?.updated_at; +} diff --git a/apps/signage-manager/src/app/shared/playlist-approve-modal.component.ts b/apps/signage-manager/src/app/shared/playlist-approve-modal.component.ts index 8ebf7657412..e826ed93578 100644 --- a/apps/signage-manager/src/app/shared/playlist-approve-modal.component.ts +++ b/apps/signage-manager/src/app/shared/playlist-approve-modal.component.ts @@ -20,7 +20,10 @@ import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { SignageContextService } from '../signage-context.service'; import { playlistMediaItems } from '../signage-playlist.util'; import { PlaylistApprovalPreviewComponent } from './playlist-approval-preview.component'; -import { loadPlaylistApprovalVersions } from './playlist-approval.util'; +import { + loadPlaylistApprovalVersions, + playlistChangedSince, +} from './playlist-approval.util'; interface PlaylistApproveModalData { playlist: SignagePlaylist; @@ -189,7 +192,7 @@ export class PlaylistApproveModalComponent { notifySuccess(i18n('SIGNAGE_MANAGER.PLAYLIST_REVERTED')); this._dialog_ref.close(true); this._playlist_service.refreshPlaylist(this._data.playlist.id); - } catch (e) { + } catch { notifyError(i18n('SIGNAGE_MANAGER.PLAYLIST_REVERT_ERROR')); } finally { this.loading.set(''); @@ -197,20 +200,31 @@ export class PlaylistApproveModalComponent { } } + /** + * Approve the playlist. When it changed after the changes were loaded, + * show the new changes and a warning instead, as the server approves + * the latest version. + */ public async approve() { if (!this.versions_loaded()) return; + const playlist_id = this._data.playlist.id; this.loading.set(i18n('SIGNAGE_MANAGER.APPROVING_PLAYLIST')); this._dialog_ref.disableClose = true; try { - await approveSignagePlaylist(this._data.playlist.id); - this._playlist_service.setPlaylistApprovalStatus( - this._data.playlist.id, - true, - ); + const [shown] = this.playlist_versions(); + if (await playlistChangedSince(playlist_id, shown)) { + notifyWarn( + i18n('SIGNAGE_MANAGER.PLAYLIST_CHANGED_BEFORE_APPROVAL'), + ); + this._playlist_versions.reload(); + return; + } + await approveSignagePlaylist(playlist_id); + this._playlist_service.setPlaylistApprovalStatus(playlist_id, true); notifySuccess(i18n('SIGNAGE_MANAGER.PLAYLIST_APPROVED')); this._dialog_ref.close(true); this._context.changed(); - } catch (e) { + } catch { notifyError(i18n('SIGNAGE_MANAGER.PLAYLIST_APPROVE_ERROR')); } finally { this.loading.set(''); diff --git a/apps/signage-manager/src/app/shared/playlist-edit-modal.component.ts b/apps/signage-manager/src/app/shared/playlist-edit-modal.component.ts index 6d46860e2e9..7ba2aa5fa97 100644 --- a/apps/signage-manager/src/app/shared/playlist-edit-modal.component.ts +++ b/apps/signage-manager/src/app/shared/playlist-edit-modal.component.ts @@ -33,6 +33,7 @@ import { updateSignagePlaylist, } from '@placeos/ts-client'; import { endOfDay, getUnixTime, startOfDay } from 'date-fns'; +import { playlistAnimation } from '../signage-playlist.util'; import { createPlaylistScheduleModel, PlaylistScheduleFormComponent, @@ -56,6 +57,33 @@ export interface PlaylistEditModalData { beforeSave?: (data: Partial) => Promise; } +/** Focus targets where a plain key press belongs to the control, not a hotkey */ +const HOTKEY_BLOCKING_FOCUS = + 'select, mat-select, [role="combobox"], [role="listbox"], [role="option"], [role="menu"], [role="menuitem"]'; + +/** Animations the user can pick, with their label keys */ +const ANIMATION_OPTIONS: { value: MediaAnimation; label: string }[] = [ + { value: MediaAnimation.Default, label: 'COMMON.DEFAULT' }, + { value: MediaAnimation.Cut, label: 'SIGNAGE_MANAGER.ANIM_CUT' }, + { + value: MediaAnimation.CrossFade, + label: 'SIGNAGE_MANAGER.ANIM_CROSS_FADE', + }, + { value: MediaAnimation.SlideTop, label: 'SIGNAGE_MANAGER.ANIM_SLIDE_TOP' }, + { + value: MediaAnimation.SlideLeft, + label: 'SIGNAGE_MANAGER.ANIM_SLIDE_LEFT', + }, + { + value: MediaAnimation.SlideRight, + label: 'SIGNAGE_MANAGER.ANIM_SLIDE_RIGHT', + }, + { + value: MediaAnimation.SlideBottom, + label: 'SIGNAGE_MANAGER.ANIM_SLIDE_BOTTOM', + }, +]; + export interface PlaylistEditFormModel { name: string; description: string; @@ -213,31 +241,14 @@ export interface PlaylistEditFormModel { | translate " > - {{ - 'COMMON.DEFAULT' | translate - }} - {{ - 'SIGNAGE_MANAGER.ANIM_CUT' | translate - }} - {{ - 'SIGNAGE_MANAGER.ANIM_CROSS_FADE' - | translate - }} - {{ - 'SIGNAGE_MANAGER.ANIM_SLIDE_TOP' | translate - }} - {{ - 'SIGNAGE_MANAGER.ANIM_SLIDE_LEFT' - | translate - }} - {{ - 'SIGNAGE_MANAGER.ANIM_SLIDE_RIGHT' - | translate - }} - {{ - 'SIGNAGE_MANAGER.ANIM_SLIDE_BOTTOM' - | translate - }} + @for ( + option of animation_options; + track option.value + ) { + {{ + option.label | translate + }} + }
@@ -342,17 +353,19 @@ export class PlaylistEditModalComponent { inject>(MatDialogRef); public readonly loading = signal(false); + public readonly animation_options = ANIMATION_OPTIONS; public readonly active_schedule_index = signal(0); public readonly playlist = this._data.playlist; public readonly group_id = this._data.group_id || ''; + /** Default animation when the modal opened */ + private readonly _loaded_animation = playlistAnimation(this.playlist); public readonly model = signal({ name: this.playlist.name || '', description: this.playlist.description || '', enabled: this.playlist.enabled ?? true, distribution: !!this.playlist.distribution, random: !!this.playlist.random, - default_animation: - this.playlist.default_animation ?? MediaAnimation.Default, + default_animation: this._loaded_animation, orientation: this.playlist.orientation || 'unspecified', default_duration: this.playlist.default_duration || 15000, schedules: playlistSchedules(this.playlist).map((schedule) => @@ -378,9 +391,13 @@ export class PlaylistEditModalComponent { }); constructor() { - const save_hotkey = inject(HotkeysService).listen(['KeyS'], () => - this.savePlaylist(), - ); + // The save hotkey is a plain key, so a select or list that has focus + // keeps it, e.g. to jump to an option such as "Square" + const save_hotkey = inject(HotkeysService).listen(['KeyS'], () => { + if (!document.activeElement?.closest(HOTKEY_BLOCKING_FOCUS)) { + this.savePlaylist(); + } + }); inject(DestroyRef).onDestroy(() => save_hotkey?.unsubscribe()); if (!this.model().distribution && !this.model().schedules.length) { this.addSchedule(); @@ -425,10 +442,21 @@ export class PlaylistEditModalComponent { await submit(this.form, async () => { this.loading.set(true); this._dialog_ref.disableClose = true; - const { schedules, valid_from, valid_until, ...fields } = - this.model(); + const { + schedules, + valid_from, + valid_until, + default_animation, + ...fields + } = this.model(); const data: Partial = { ...fields, + // Send the animation only when the user changes it. The + // loaded value can be a guess (see `playlistAnimation`), and + // saving must not change it. + ...(default_animation !== this._loaded_animation + ? { default_animation } + : {}), ...(fields.distribution ? {} : { @@ -436,12 +464,14 @@ export class PlaylistEditModalComponent { playlistSchedulePayload(schedule), ), }), - ...(valid_from - ? { valid_from: getUnixTime(startOfDay(valid_from)) } - : {}), - ...(valid_until - ? { valid_until: getUnixTime(endOfDay(valid_until)) } - : {}), + // Null clears a date. The update is a patch, so a missing + // date would keep the saved one. + valid_from: valid_from + ? getUnixTime(startOfDay(valid_from)) + : null, + valid_until: valid_until + ? getUnixTime(endOfDay(valid_until)) + : null, }; if ( this.playlist.id && @@ -466,11 +496,10 @@ export class PlaylistEditModalComponent { this._dialog_ref.disableClose = false; this._dialog_ref.close(result); notifySuccess(i18n('SIGNAGE_MANAGER.PLAYLIST_SAVED')); - } catch (e) { + } catch { this._dialog_ref.disableClose = false; this.loading.set(false); notifyError(i18n('SIGNAGE_MANAGER.PLAYLIST_SAVE_ERROR')); - throw e; } }); } diff --git a/apps/signage-manager/src/app/shared/playlist-item-schedule-modal.component.ts b/apps/signage-manager/src/app/shared/playlist-item-schedule-modal.component.ts index eea02d4334b..d33c8ae9801 100644 --- a/apps/signage-manager/src/app/shared/playlist-item-schedule-modal.component.ts +++ b/apps/signage-manager/src/app/shared/playlist-item-schedule-modal.component.ts @@ -172,11 +172,10 @@ export class PlaylistItemScheduleModalComponent { this._dialog_ref.disableClose = false; this._dialog_ref.close(true); notifySuccess(i18n('SIGNAGE_MANAGER.SVC_PLAYLIST_UPDATED')); - } catch (e) { + } catch { this._dialog_ref.disableClose = false; this.loading.set(false); notifyError(i18n('SIGNAGE_MANAGER.PLAYLIST_SAVE_ERROR')); - throw e; } }); } diff --git a/apps/signage-manager/src/app/shared/playlist-schedule-form.component.ts b/apps/signage-manager/src/app/shared/playlist-schedule-form.component.ts index b1f31b7d0d5..01668a78af8 100644 --- a/apps/signage-manager/src/app/shared/playlist-schedule-form.component.ts +++ b/apps/signage-manager/src/app/shared/playlist-schedule-form.component.ts @@ -52,12 +52,16 @@ import { } from '../signage-cron.util'; import { createScheduleMaskFilter, + DEFAULT_PLAY_PERIOD_MINUTES, formatPlayAtLocal, + formatPlayDateTime, + formatPlayDateTimeRange, hasPlayableScheduleMask, isPlayOnceSchedule, isValidScheduleMask, + ordinal, + parseCronMonthDays, parsePlayAtLocal, - playEndTime, playlistScheduleExpiryLabel, type PlaylistSchedule, } from '../signage-playlist.util'; @@ -102,7 +106,6 @@ const FULL_DAY_START_MINUTES = 0; const FULL_DAY_END_MINUTES = 23 * 60 + 59; const DEFAULT_RECURRING_TIME = '00:00'; const DEFAULT_RECURRING_CRON = '0 0 * * *'; -const DEFAULT_PLAY_PERIOD_MINUTES = 24 * 60; const WEEKDAY_OPTIONS = [ { value: 1 }, { value: 2 }, @@ -134,20 +137,6 @@ function timeToMinutes(value: string) { ); } -function ordinal(value: number) { - if (value >= 11 && value <= 13) return `${value}th`; - switch (value % 10) { - case 1: - return `${value}st`; - case 2: - return `${value}nd`; - case 3: - return `${value}rd`; - default: - return `${value}th`; - } -} - function normaliseWeekdays(value: number[] | null | undefined) { const seen_days = new Set(); for (const day of value || []) { @@ -190,17 +179,6 @@ function dayRangeForWeekOfMonth(value: number | null | undefined) { return `${start}-${start + 6}`; } -function parseCronMonthDays(value: string) { - if (!value?.trim() || value === '*') return null; - const days = new Set(); - for (const part of value.split(',')) { - const day = parseCronNumber(part, 1, 31); - if (day === null) return null; - days.add(day); - } - return normaliseMonthDays([...days]); -} - function parseRecurringCron(value: string | null | undefined) { const [minute_part, hour_part, day_part, month_part, weekday_part] = ( value || DEFAULT_RECURRING_CRON @@ -363,39 +341,6 @@ export function playlistSchedules(playlist: SignagePlaylist) { return playlist.schedules?.length ? playlist.schedules : [schedule]; } -function formatPlayDateTime(date: Date, timeZone = LOCAL_TIMEZONE) { - return date.toLocaleString(undefined, { - timeZone, - weekday: 'short', - month: 'short', - day: 'numeric', - hour: 'numeric', - minute: '2-digit', - }); -} - -function formatPlayTime(date: Date, timeZone = LOCAL_TIMEZONE) { - return date.toLocaleTimeString(undefined, { - timeZone, - hour: 'numeric', - minute: '2-digit', - }); -} - -function formatPlayDateTimeRange( - start: Date, - duration_minutes: number, - timezone = LOCAL_TIMEZONE, -) { - const end = playEndTime(start, duration_minutes); - const end_text = - toZonedTime(start, timezone).toDateString() === - toZonedTime(end, timezone).toDateString() - ? formatPlayTime(end, timezone) - : formatPlayDateTime(end, timezone); - return `${formatPlayDateTime(start, timezone)} – ${end_text}`; -} - function formatMinutes(value: number | null | undefined) { const total_minutes = Math.max(0, Math.round(value || 0)); if (!total_minutes) return ''; diff --git a/apps/signage-manager/src/app/shared/playlist-select-modal.component.ts b/apps/signage-manager/src/app/shared/playlist-select-modal.component.ts index 6938fb01213..1f530f26f3c 100644 --- a/apps/signage-manager/src/app/shared/playlist-select-modal.component.ts +++ b/apps/signage-manager/src/app/shared/playlist-select-modal.component.ts @@ -40,8 +40,7 @@ import { byName, PagedSearch } from './paged-search'; > ) { return parsePlayAtLocal(schedule.play_at_local); } -const DEFAULT_PLAY_PERIOD_MINUTES = 24 * 60; +/** Play period of a schedule that does not set one: the whole day */ +export const DEFAULT_PLAY_PERIOD_MINUTES = 24 * 60; const WEEKDAY_NAMES = [ 'Sunday', 'Monday', @@ -90,6 +92,40 @@ const WEEKDAY_NAMES = [ 'Saturday', ]; +/** + * Animation as a `MediaAnimation` value. The API accepts animation names + * but stores and returns the index of the name in `MediaAnimation`, e.g. + * 2 for `cross_fade`. Names are kept as they are. + * @returns The default animation for an index out of range or no value + */ +export function mediaAnimation( + value: MediaAnimation | number | null | undefined, +): MediaAnimation { + if (typeof value !== 'number') return value || MediaAnimation.Default; + const animations = Object.values(MediaAnimation); + return Number.isInteger(value) && value >= 0 && value < animations.length + ? animations[value] + : MediaAnimation.Default; +} + +/** + * Default animation of a playlist that ts-client loaded. The API returns + * the index of the animation, and `SignagePlaylist` replaces a falsy value + * with `cut`. So index 0 (`default`) arrives as the name `cut`, while a + * saved Cut arrives as the index 1. Read the name `cut` as the default. + * + * The player treats the default as its own transition, not as a cut, so + * the two must not be merged. + */ +export function playlistAnimation(playlist: { + default_animation?: MediaAnimation | number; +}): MediaAnimation { + const value = playlist.default_animation; + return value === MediaAnimation.Cut + ? MediaAnimation.Default + : mediaAnimation(value); +} + export function playlistMediaThumbnailUrl(item: SignageMedia) { // `SignageMedia.thumbnail_url` builds an uploads URL whether or not a // thumbnail exists, so items without one render as a broken image @@ -182,7 +218,8 @@ export function playlistItemScheduleMap(list: { return map; } -function ordinal(value: number) { +/** English ordinal of a number, e.g. "1st" or "12th" */ +export function ordinal(value: number) { if (value >= 11 && value <= 13) return `${value}th`; switch (value % 10) { case 1: @@ -220,7 +257,7 @@ function durationLabel(duration_minutes: number) { } /** Days of the month in a plain list such as "1,15". Empty for other values. */ -function parseCronMonthDays(value: string) { +export function parseCronMonthDays(value: string) { if (!value || value === '*') return []; const days = value.split(',').map((part) => parseCronNumber(part, 1, 31)); return days.every((day) => day !== null) @@ -382,6 +419,40 @@ export function playlistExpiredAt( return Math.max(...ends); } +/** Status badge of a playlist in a list. Null when it needs no badge. */ +export type PlaylistStatus = + | 'expired' + | 'pending' + | 'awaiting_approval' + | 'awaiting_review' + | null; + +/** + * Status of a playlist for list badges. Expiry uses `playlistExpiredAt`, so + * lists agree with the content report. + * @param approvals Approval state by playlist ID. A playlist that is not in + * it has no approval state, e.g. it does not need approval. + * @param requests Whether approval was requested, by playlist ID + */ +export function playlistStatus( + playlist: { + id: string; + valid_from?: number; + valid_until?: number; + schedules?: readonly Partial[]; + }, + approvals: Record, + requests: Record, + now = Date.now(), +): PlaylistStatus { + if (playlistExpiredAt(playlist, now)) return 'expired'; + if (playlist.valid_from && playlist.valid_from * 1000 > now) { + return 'pending'; + } + if (!(playlist.id in approvals) || approvals[playlist.id]) return null; + return requests[playlist.id] ? 'awaiting_review' : 'awaiting_approval'; +} + export function playlistScheduleLabel(schedule: Partial) { const period = schedulePeriod(schedule); const expiry = playlistScheduleExpiryLabel(schedule); @@ -502,8 +573,13 @@ export function createScheduleMaskFilter( }; } -function formatPlayDateTime(date: Date) { +/** + * Date and time of a play, e.g. "Mon, Jan 5, 9:00 AM". + * @param timeZone IANA timezone. The viewer's timezone when not set. + */ +export function formatPlayDateTime(date: Date, timeZone?: string) { return date.toLocaleString(undefined, { + timeZone, weekday: 'short', month: 'short', day: 'numeric', @@ -512,8 +588,9 @@ function formatPlayDateTime(date: Date) { }); } -function formatPlayTime(date: Date) { +function formatPlayTime(date: Date, timeZone?: string) { return date.toLocaleTimeString(undefined, { + timeZone, hour: 'numeric', minute: '2-digit', }); @@ -531,13 +608,24 @@ export function playEndTime(start: Date, duration_minutes: number) { ); } -function formatPlayDateTimeRange(start: Date, duration_minutes: number) { +/** + * Time range of a play. The end shows only its time when the play ends on + * the day it starts. + * @param timeZone IANA timezone. The viewer's timezone when not set. + */ +export function formatPlayDateTimeRange( + start: Date, + duration_minutes: number, + timeZone?: string, +) { const end = playEndTime(start, duration_minutes); + const day = (date: Date) => + (timeZone ? toZonedTime(date, timeZone) : date).toDateString(); const end_text = - start.toDateString() === end.toDateString() - ? formatPlayTime(end) - : formatPlayDateTime(end); - return `${formatPlayDateTime(start)} – ${end_text}`; + day(start) === day(end) + ? formatPlayTime(end, timeZone) + : formatPlayDateTime(end, timeZone); + return `${formatPlayDateTime(start, timeZone)} – ${end_text}`; } interface PlaySession { 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 ad8b65f4e28..8b4932919c3 100644 --- a/apps/signage-manager/src/app/zones/zone-content.component.ts +++ b/apps/signage-manager/src/app/zones/zone-content.component.ts @@ -13,10 +13,9 @@ import { SignagePlaylistService } from '../playlists/signage-playlist.service'; import { PlaylistThumbnailComponent } from '../shared/playlist-thumbnail.component'; import { TemplateMappingsComponent } from '../shared/template-mappings.component'; import { SignageContextService } from '../signage-context.service'; +import { playlistStatus } from '../signage-playlist.util'; import { SignageZoneService } from './signage-zone.service'; -type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; - @Component({ selector: 'zone-content', template: ` @@ -134,12 +133,22 @@ type PlaylistStatus = 'expired' | 'pending' | 'awaiting_approval' | null; }} } + @case ('awaiting_review') { + + {{ + 'SIGNAGE_MANAGER.STATUS_AWAITING_REVIEW' + | translate + }} + + } @case ('awaiting_approval') { {{ - 'SIGNAGE_MANAGER.STATUS_AWAITING_APPROVAL' + 'COMMON.APPROVAL_REQUIRED' | translate }} @@ -431,15 +440,11 @@ export class ZoneContentComponent { if (zone) this._display_service.addDisplayToZone(zone); } - public getStatus(playlist: SignagePlaylist): PlaylistStatus { - const now_s = Math.floor(Date.now() / 1000); - if (playlist.valid_until && playlist.valid_until < now_s) - return 'expired'; - if (playlist.valid_from && playlist.valid_from > now_s) - return 'pending'; - const approvals = this.playlist_approval_status(); - if (playlist.id in approvals && !approvals[playlist.id]) - return 'awaiting_approval'; - return null; + public getStatus(playlist: SignagePlaylist) { + return playlistStatus( + playlist, + this.playlist_approval_status(), + this._playlist_service.playlist_approval_requested_status(), + ); } } 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 8ac12fc9687..3ab8fdd5cb1 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 @@ -37,6 +37,7 @@ describe('DisplayContentComponent', () => { playlistsById: (ids: readonly string[]) => playlists().filter(({ id }) => ids.includes(id)), playlist_approval_status, + playlist_approval_requested_status: signal>({}), playlist_thumbnail_media, playlists_loading, playlists_error, diff --git a/apps/signage-manager/src/tests/playlists/playlist-item-details.component.spec.ts b/apps/signage-manager/src/tests/playlists/playlist-item-details.component.spec.ts index 8fe60872c8a..13f8763b0eb 100644 --- a/apps/signage-manager/src/tests/playlists/playlist-item-details.component.spec.ts +++ b/apps/signage-manager/src/tests/playlists/playlist-item-details.component.spec.ts @@ -1,6 +1,11 @@ import { ApplicationRef, signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; -import { MediaAnimation } from '@placeos/ts-client'; +import { + MediaAnimation, + PlaceSystem, + PlaceZone, + SignagePlaylist, +} from '@placeos/ts-client'; import { SignageDisplayService } from '../../app/displays/signage-display.service'; import { PlaylistItemDetailsComponent } from '../../app/playlists/playlist-item-details.component'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; @@ -11,6 +16,7 @@ import { SignageZoneService } from '../../app/zones/signage-zone.service'; describe('PlaylistItemDetailsComponent', () => { const selected_playlist = signal(null); const playlist_media_items = signal([]); + const playlist_media_error = signal(false); const displays = signal([]); const zones = signal([]); const can_update = signal(true); @@ -33,7 +39,11 @@ describe('PlaylistItemDetailsComponent', () => { removeDisplayFromPlaylist: remove_display, }; const inventory_stub = { loadSignageInventory: load_inventory }; - const playlist_stub = { selected_playlist, playlist_media_items }; + const playlist_stub = { + selected_playlist, + playlist_media_items, + playlist_media_error, + }; const zone_stub = { zones, addZoneToPlaylist: add_zone, @@ -63,6 +73,7 @@ describe('PlaylistItemDetailsComponent', () => { vi.clearAllMocks(); selected_playlist.set(null); playlist_media_items.set([]); + playlist_media_error.set(false); displays.set([]); zones.set([]); can_update.set(true); @@ -99,6 +110,12 @@ describe('PlaylistItemDetailsComponent', () => { expect(component.item_count()).toBe(2); }); + it('shows a dash, not zero, while the items cannot load', async () => { + playlist_media_error.set(true); + const component = await make(); + expect(component.item_count()).toBe('—'); + }); + it('lists only displays and zones that reference the selected playlist', async () => { selected_playlist.set({ id: 'pl-1', name: 'Lobby' }); displays.set([ @@ -127,6 +144,14 @@ describe('PlaylistItemDetailsComponent', () => { expect(component.playlist_zones()).toEqual([]); }); + it('labels a saved animation index as its animation', async () => { + selected_playlist.set({ id: 'pl-1', default_animation: 2 }); + const component = await make(); + expect(component.animation_label()).toBe( + 'SIGNAGE_MANAGER.ANIM_CROSS_FADE', + ); + }); + it('uses the selected signage group for shared playlist details', async () => { selected_group.set({ group: { id: 'grp-1' } }); const component = await make(); @@ -148,6 +173,21 @@ describe('PlaylistItemDetailsComponent', () => { expect(component.animation_label()).toBe('COMMON.DEFAULT'); }); + it('labels a playlist with the default animation as default, not cut', async () => { + // ts-client turns the saved index 0 into `cut`; a saved Cut is 1 + selected_playlist.set( + new SignagePlaylist({ + id: 'pl-1', + default_animation: 0 as unknown as MediaAnimation, + }), + ); + const component = await make(); + expect(component.animation_label()).toBe('COMMON.DEFAULT'); + + selected_playlist.set({ id: 'pl-1', default_animation: 1 }); + expect(component.animation_label()).toBe('SIGNAGE_MANAGER.ANIM_CUT'); + }); + it('converts validity timestamps from seconds to milliseconds', async () => { selected_playlist.set({ id: 'pl-1', @@ -292,7 +332,7 @@ describe('PlaylistItemDetailsComponent', () => { const playlist = { id: 'pl-1' }; selected_playlist.set(playlist); const component = await make(); - const display = { id: 'd-1' }; + const display = new PlaceSystem({ id: 'd-1' }); const event = { preventDefault: vi.fn(), stopPropagation: vi.fn() }; component.addDisplay(); @@ -308,7 +348,7 @@ describe('PlaylistItemDetailsComponent', () => { const playlist = { id: 'pl-1' }; selected_playlist.set(playlist); const component = await make(); - const zone = { id: 'z-1' }; + const zone = new PlaceZone({ id: 'z-1' }); const event = { preventDefault: vi.fn(), stopPropagation: vi.fn() }; component.addZone(); diff --git a/apps/signage-manager/src/tests/playlists/playlist-items.component.spec.ts b/apps/signage-manager/src/tests/playlists/playlist-items.component.spec.ts index f489d70c243..345ac01d424 100644 --- a/apps/signage-manager/src/tests/playlists/playlist-items.component.spec.ts +++ b/apps/signage-manager/src/tests/playlists/playlist-items.component.spec.ts @@ -1,7 +1,8 @@ -import { signal } from '@angular/core'; +import { NO_ERRORS_SCHEMA, signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { SignagePlaylistItemSchedule } from '@placeos/ts-client'; import { SignageMediaService } from '../../app/media/signage-media.service'; +import { PlaylistActionsComponent } from '../../app/playlists/playlist-actions.component'; import { PlaylistItemsComponent } from '../../app/playlists/playlist-items.component'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; import { SignageContextService } from '../../app/signage-context.service'; @@ -23,6 +24,8 @@ describe('PlaylistItemsComponent', () => { ); const can_update = signal(true); const playlist_media_loading = signal(false); + const playlist_media_error = signal(false); + const reload_media = vi.fn(); const reorder = vi.fn(); const remove_media = vi.fn().mockResolvedValue(undefined); const remove_media_items = vi.fn().mockResolvedValue(true); @@ -36,6 +39,8 @@ describe('PlaylistItemsComponent', () => { selected_playlist_item, selected_playlist_item_index, playlist_media_loading, + playlist_media_error, + reloadPlaylistMedia: reload_media, playlist_media_items, playlist_item_schedules, playlist_item_schedule_list, @@ -72,8 +77,29 @@ describe('PlaylistItemsComponent', () => { playlist_item_schedule_list.set([]); can_update.set(true); playlist_media_loading.set(false); + playlist_media_error.set(false); }); + /** Render the real template, without the playlist action buttons */ + async function render() { + await TestBed.configureTestingModule({ + imports: [PlaylistItemsComponent], + providers: [ + { provide: SignageContextService, useValue: context_stub }, + { provide: SignageMediaService, useValue: media_stub }, + { provide: SignagePlaylistService, useValue: playlist_stub }, + ], + }) + .overrideComponent(PlaylistItemsComponent, { + remove: { imports: [PlaylistActionsComponent] }, + add: { schemas: [NO_ERRORS_SCHEMA] }, + }) + .compileComponents(); + const fixture = TestBed.createComponent(PlaylistItemsComponent); + fixture.detectChanges(); + return fixture; + } + it('keeps the items on screen while they reload', async () => { const component = await make(); playlist_media_loading.set(true); @@ -206,6 +232,78 @@ describe('PlaylistItemsComponent', () => { expect(reorder).toHaveBeenCalledWith('pl-1', ['b', 'c', 'a']); }); + it('moves an item one place for keyboard users', async () => { + playlist_media_items.set([media('a'), media('b'), media('c')]); + selected_playlist.set({ id: 'pl-1' }); + const component = await make(); + + await component.moveItem(1, -1); + expect(reorder).toHaveBeenLastCalledWith('pl-1', ['b', 'a', 'c']); + + await component.moveItem(1, 1); + expect(reorder).toHaveBeenLastCalledWith('pl-1', ['a', 'c', 'b']); + + reorder.mockClear(); + await component.moveItem(0, -1); + await component.moveItem(2, 1); + expect(reorder).not.toHaveBeenCalled(); + }); + + it('renders items as list items', async () => { + playlist_media_items.set([media('a'), media('b')]); + selected_playlist.set({ id: 'pl-1' }); + const fixture = await render(); + const element: HTMLElement = fixture.nativeElement; + + const list = element.querySelector('[role="list"]'); + expect(list?.children).toHaveLength(2); + for (const child of Array.from(list?.children || [])) { + expect(child.getAttribute('role')).toBe('listitem'); + } + }); + + it('leaves Enter and Space to the controls inside a row', async () => { + const item = media('a'); + playlist_media_items.set([item]); + selected_playlist.set({ id: 'pl-1' }); + const fixture = await render(); + const element: HTMLElement = fixture.nativeElement; + const press = (target: Element, key: string) => { + const event = new KeyboardEvent('keydown', { + key, + bubbles: true, + cancelable: true, + }); + target.dispatchEvent(event); + return event; + }; + + // A default action that is not prevented lets the browser click + // the actions button, which opens the menu, or toggle the checkbox + const menu_trigger = element.querySelector('.mat-mdc-menu-trigger'); + expect(press(menu_trigger, 'Enter').defaultPrevented).toBe(false); + const checkbox = element.querySelector('mat-checkbox input'); + expect(press(checkbox, ' ').defaultPrevented).toBe(false); + expect(selected_playlist_item()).toBeNull(); + + const row = element.querySelector('[role="button"]'); + expect(press(row, 'Enter').defaultPrevented).toBe(true); + expect(selected_playlist_item()).toBe(item); + }); + + it('shows a load error with retry when the items fail to load', async () => { + selected_playlist.set({ id: 'pl-1' }); + playlist_media_error.set(true); + const fixture = await render(); + const element: HTMLElement = fixture.nativeElement; + + expect(element.textContent).not.toContain('No items'); + const retry = + element.querySelector('load-error button'); + retry?.click(); + expect(reload_media).toHaveBeenCalled(); + }); + it('does not reorder distribution playlists or without update rights', async () => { playlist_media_items.set([media('a'), media('b')]); selected_playlist.set({ id: 'pl-1', distribution: true }); diff --git a/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts b/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts index 0aeaba16d56..7f5b888c5d6 100644 --- a/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts +++ b/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts @@ -1,8 +1,11 @@ import { 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 { PlaylistsSectionComponent } from '../../app/playlists/playlists.component'; import { SignagePlaylistService } from '../../app/playlists/signage-playlist.service'; +import { SignageContextService } from '../../app/signage-context.service'; describe('PlaylistsSectionComponent', () => { const selected_playlist = signal(null); @@ -10,8 +13,14 @@ describe('PlaylistsSectionComponent', () => { const selected_playlist_item_index = signal(null); const playlists = signal([]); const playlist_media_items = signal([]); + const playlists_loading = signal(false); + const can_query = signal(true); const navigate = vi.fn(); const load_playlist = vi.fn(); + const notify_open = vi.fn(() => ({ + onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }), + dismiss: vi.fn(), + })); const playlist_stub = { selected_playlist, @@ -19,6 +28,7 @@ describe('PlaylistsSectionComponent', () => { selected_playlist_item_index, playlists, playlist_media_items, + playlists_loading, loadPlaylist: load_playlist, }; @@ -29,6 +39,10 @@ describe('PlaylistsSectionComponent', () => { imports: [PlaylistsSectionComponent], providers: [ { provide: SignagePlaylistService, useValue: playlist_stub }, + { + provide: SignageContextService, + useValue: { canQueryLists: () => can_query() }, + }, { provide: Router, useValue: { navigate } }, { provide: ActivatedRoute, useValue: {} }, ], @@ -48,6 +62,9 @@ describe('PlaylistsSectionComponent', () => { selected_playlist_item_index.set(null); playlists.set([]); playlist_media_items.set([]); + playlists_loading.set(false); + can_query.set(true); + setNotifyOutlet({ open: notify_open } as unknown as MatSnackBar, true); }); it('syncs the active view tab from the route', async () => { @@ -85,6 +102,64 @@ describe('PlaylistsSectionComponent', () => { expect(selected_playlist()).toBe(linked); }); + it('fetches a linked playlist when the loaded pages are empty', async () => { + const linked = { id: 'pl-9', name: 'Linked' }; + load_playlist.mockImplementationOnce(async () => { + playlists.set([linked]); + return linked; + }); + await make(); + fixture.componentRef.setInput('id', 'pl-9'); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(load_playlist).toHaveBeenCalledWith('pl-9'); + expect(selected_playlist()).toBe(linked); + }); + + it('waits for the first page before fetching a linked playlist', async () => { + playlists_loading.set(true); + await make(); + fixture.componentRef.setInput('id', 'pl-9'); + fixture.detectChanges(); + expect(load_playlist).not.toHaveBeenCalled(); + + playlists_loading.set(false); + fixture.detectChanges(); + expect(load_playlist).toHaveBeenCalledWith('pl-9'); + }); + + it('warns when a linked playlist cannot be loaded', async () => { + load_playlist.mockResolvedValueOnce(null); + await make(); + fixture.componentRef.setInput('id', 'missing'); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(notify_open).toHaveBeenCalledWith( + expect.stringContaining('Could not open the playlist'), + expect.anything(), + expect.anything(), + ); + }); + + it('clears the selection when a linked playlist cannot be loaded', async () => { + const open = { id: 'pl-1' }; + playlists.set([open]); + await make(); + fixture.componentRef.setInput('id', 'pl-1'); + fixture.detectChanges(); + expect(selected_playlist()).toBe(open); + + load_playlist.mockResolvedValueOnce(null); + fixture.componentRef.setInput('id', 'deleted'); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(selected_playlist()).toBeNull(); + }); + it('clears the selection once the route id is removed', async () => { const match = { id: 'pl-1' }; playlists.set([match]); diff --git a/apps/signage-manager/src/tests/playlists/signage-playlist.service.spec.ts b/apps/signage-manager/src/tests/playlists/signage-playlist.service.spec.ts index c3d00c5f9de..da39435b82f 100644 --- a/apps/signage-manager/src/tests/playlists/signage-playlist.service.spec.ts +++ b/apps/signage-manager/src/tests/playlists/signage-playlist.service.spec.ts @@ -1,4 +1,4 @@ -import { signal } from '@angular/core'; +import { ApplicationRef, signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { MatDialog } from '@angular/material/dialog'; import { @@ -9,6 +9,7 @@ import { import { addSignagePlaylist, listSignagePlaylistMedia, + MediaAnimation, PlaceSystem, removeSignagePlaylist, scheduleSignagePlaylistMedia, @@ -149,6 +150,7 @@ describe('SignagePlaylistService', () => { }), ); test_service['_playlist_list'].update(() => loaded_playlists); + vi.mocked(showSignagePlaylist).mockResolvedValue(loaded_playlists[200]); (updateSignagePlaylistMedia as any).mockResolvedValue({}); await service.addMediaToPlaylist('playlist-200', 'media-1'); @@ -463,6 +465,28 @@ describe('SignagePlaylistService', () => { ); }); + it('keeps the default animation on a copy', async () => { + vi.mocked(addSignagePlaylist).mockResolvedValue( + new SignagePlaylist({ id: 'copy-1' }), + ); + vi.mocked(updateSignagePlaylistMedia).mockResolvedValue( + new SignagePlaylistMedia({}), + ); + const service = createService(); + + // ts-client turns the saved index 0 into `cut` + await service.duplicatePlaylist( + new SignagePlaylist({ + id: 'pl-1', + default_animation: 0 as unknown as MediaAnimation, + }), + ); + + expect(vi.mocked(addSignagePlaylist).mock.calls[0][0]).toMatchObject({ + default_animation: MediaAnimation.Default, + }); + }); + it('ignores a second duplicate request while the first runs', async () => { let listed: (value: unknown) => void = () => {}; (listSignagePlaylistMedia as any).mockReturnValue( @@ -684,4 +708,129 @@ describe('SignagePlaylistService', () => { ]); }); }); + + describe('adding media', () => { + it('checks a playlist outside the loaded pages for distribution', async () => { + const service = createService(); + vi.mocked(showSignagePlaylist).mockResolvedValue( + new SignagePlaylist({ id: 'found-1', distribution: true }), + ); + + await service.addMediaToPlaylist('found-1', 'media-1'); + + expect(showSignagePlaylist).toHaveBeenCalledWith( + 'found-1', + expect.anything(), + ); + expect(dialog.open).toHaveBeenCalledWith( + PlaylistItemScheduleModalComponent, + expect.anything(), + ); + expect(updateSignagePlaylistMedia).not.toHaveBeenCalled(); + }); + + it('reports an error, not a rejection, when adding media fails', async () => { + const service = createService(); + vi.mocked(showSignagePlaylist).mockResolvedValue( + new SignagePlaylist({ id: 'pl-1' }), + ); + vi.mocked(listSignagePlaylistMedia).mockRejectedValue( + new Error('Offline'), + ); + + await service.addMediaToPlaylist('pl-1', 'media-1'); + const added = await service.addMediaItemsToPlaylist('pl-1', [ + 'media-2', + ]); + + expect(added).toBe(false); + expect(updateSignagePlaylistMedia).not.toHaveBeenCalled(); + expect(notify_open).toHaveBeenCalledTimes(2); + expect(notify_open).toHaveBeenCalledWith( + 'Error adding media to the playlist', + expect.anything(), + expect.objectContaining({ panelClass: ['error'] }), + ); + }); + }); + + describe('selected playlist items', () => { + /** Select a playlist by ID without the selection debounce */ + function selectPlaylistId(service: SignagePlaylistService) { + const id = signal(''); + Object.defineProperty(service, '_selected_playlist_id', { + value: id, + }); + return async (playlist_id: string) => { + id.set(playlist_id); + TestBed.tick(); + await TestBed.inject(ApplicationRef).whenStable(); + }; + } + + it('keeps a pending approval request when the items load', async () => { + const service = createService(); + const select = selectPlaylistId(service); + vi.mocked(listSignagePlaylistMedia).mockResolvedValue( + new SignagePlaylistMedia({ + items: ['media-1'], + approved: false, + approval_requested: true, + }), + ); + + await select('pl-1'); + + expect(service.playlist_approval_status()['pl-1']).toBe(false); + expect(service.playlist_approval_requested_status()['pl-1']).toBe( + true, + ); + }); + + it('shows a load error and loads the items again on retry', async () => { + const service = createService(); + const select = selectPlaylistId(service); + vi.mocked(listSignagePlaylistMedia).mockRejectedValue( + new Error('Offline'), + ); + + await select('pl-1'); + + expect(service.playlist_media_error()).toBe(true); + expect(service.playlist_media_loading()).toBe(false); + expect(service.playlist_media_items()).toEqual([]); + + vi.mocked(listSignagePlaylistMedia).mockResolvedValue( + new SignagePlaylistMedia({ + items: ['media-1'], + media: [new SignageMedia({ id: 'media-1' })], + }), + ); + service.reloadPlaylistMedia(); + TestBed.tick(); + await TestBed.inject(ApplicationRef).whenStable(); + + expect(service.playlist_media_error()).toBe(false); + expect(playlistMediaIdsShown(service)).toEqual(['media-1']); + }); + + it('shows only the load of the playlist that is selected now', async () => { + const service = createService(); + const select = selectPlaylistId(service); + let resolveFirst: (value: SignagePlaylistMedia) => void = () => {}; + vi.mocked(listSignagePlaylistMedia) + .mockReturnValueOnce( + new Promise((resolve) => (resolveFirst = resolve)), + ) + .mockReturnValueOnce(new Promise(() => {})); + + void select('pl-1'); + void select('pl-2'); + resolveFirst(new SignagePlaylistMedia({ items: [] })); + await new Promise((resolve) => setTimeout(resolve)); + TestBed.tick(); + + expect(service.playlist_media_loading()).toBe(true); + }); + }); }); diff --git a/apps/signage-manager/src/tests/shared/playlist-approve-modal.component.spec.ts b/apps/signage-manager/src/tests/shared/playlist-approve-modal.component.spec.ts index 79416d4f50a..91d5f74dc51 100644 --- a/apps/signage-manager/src/tests/shared/playlist-approve-modal.component.spec.ts +++ b/apps/signage-manager/src/tests/shared/playlist-approve-modal.component.spec.ts @@ -5,6 +5,7 @@ import { setNotifyOutlet } from '@placeos/common'; import { approveSignagePlaylist, listSignagePlaylistMediaRevisions, + SignagePlaylistMedia, updateSignagePlaylistMedia, } from '@placeos/ts-client'; import { SignageMediaService } from '../../app/media/signage-media.service'; @@ -114,6 +115,46 @@ describe('PlaylistApproveModalComponent', () => { ); }); + it('approves when the shown version is still the latest', async () => { + vi.mocked(approveSignagePlaylist).mockResolvedValue({}); + const fixture = await render(); + + await fixture.componentInstance.approve(); + + expect(listSignagePlaylistMediaRevisions).toHaveBeenLastCalledWith( + 'playlist-1', + { limit: 1 }, + ); + expect(approveSignagePlaylist).toHaveBeenCalledWith('playlist-1'); + expect(dialog_ref.close).toHaveBeenCalledWith(true); + }); + + it('does not approve a version that changed after it was shown', async () => { + const fixture = await render(); + vi.mocked(listSignagePlaylistMediaRevisions).mockResolvedValue([ + new SignagePlaylistMedia({ + id: 'newer-version', + items: ['media-4'], + updated_at: 3, + }), + ]); + + await fixture.componentInstance.approve(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(approveSignagePlaylist).not.toHaveBeenCalled(); + expect(dialog_ref.close).not.toHaveBeenCalled(); + expect(notify_open).toHaveBeenCalledWith( + expect.stringContaining('The playlist changed'), + expect.anything(), + expect.anything(), + ); + expect(fixture.componentInstance.playlist_versions()[0].id).toBe( + 'newer-version', + ); + }); + it('shows undo changes when user has update permissions', async () => { const fixture = TestBed.createComponent(PlaylistApproveModalComponent); fixture.detectChanges(); diff --git a/apps/signage-manager/src/tests/shared/playlist-edit-modal.component.spec.ts b/apps/signage-manager/src/tests/shared/playlist-edit-modal.component.spec.ts index b416b6ac533..430c261796c 100644 --- a/apps/signage-manager/src/tests/shared/playlist-edit-modal.component.spec.ts +++ b/apps/signage-manager/src/tests/shared/playlist-edit-modal.component.spec.ts @@ -1,13 +1,15 @@ -import { signal } from '@angular/core'; +import { NO_ERRORS_SCHEMA, signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { form } from '@angular/forms/signals'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { HotkeysService, setNotifyOutlet } from '@placeos/common'; +import { MediaAnimation, SignagePlaylist } from '@placeos/ts-client'; import { PlaylistEditModalComponent } from '../../app/shared/playlist-edit-modal.component'; import { createPlaylistScheduleModel, PlaylistScheduleFormComponent, } from '../../app/shared/playlist-schedule-form.component'; +import { SignageSharedWithComponent } from '../../app/shared/signage-shared-with.component'; const notify_open = vi.fn(() => ({ onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }), @@ -87,6 +89,153 @@ describe('PlaylistEditModalComponent', () => { expect(component.loading()).toBe(false); }); + it('sends null for cleared validity dates, so the patch clears them', async () => { + const component = TestBed.createComponent( + PlaylistEditModalComponent, + ).componentInstance; + component.model.update((value) => ({ + ...value, + valid_from: null, + valid_until: null, + })); + + await component.savePlaylist(); + + expect(onEdit).toHaveBeenCalledWith( + 'playlist-1', + expect.objectContaining({ valid_from: null, valid_until: null }), + ); + }); + + it('shows an error and resolves when the save fails', async () => { + onEdit.mockRejectedValue(new Error('Offline')); + const component = TestBed.createComponent( + PlaylistEditModalComponent, + ).componentInstance; + + await expect(component.savePlaylist()).resolves.toBeUndefined(); + + expect(dialog_ref.close).not.toHaveBeenCalled(); + expect(component.loading()).toBe(false); + expect(notify_open).toHaveBeenCalledWith( + 'Error saving playlist', + expect.anything(), + expect.objectContaining({ panelClass: ['error'] }), + ); + }); + + /** + * Render the modal for a distribution playlist with a saved animation + * @returns The fixture and the shown values of the selects + */ + async function renderWithAnimation( + default_animation: MediaAnimation | number, + ) { + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [PlaylistEditModalComponent], + providers: [ + { + provide: MAT_DIALOG_DATA, + useValue: { + playlist: { + id: 'playlist-1', + name: 'Playlist 1', + distribution: true, + default_animation, + }, + onEdit, + }, + }, + { provide: MatDialogRef, useValue: dialog_ref }, + { + provide: HotkeysService, + useValue: { listen: hotkey_listen }, + }, + ], + }) + .overrideComponent(PlaylistEditModalComponent, { + remove: { imports: [SignageSharedWithComponent] }, + add: { schemas: [NO_ERRORS_SCHEMA] }, + }) + .compileComponents(); + const fixture = TestBed.createComponent(PlaylistEditModalComponent); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + const element: HTMLElement = fixture.nativeElement; + const values = Array.from( + element.querySelectorAll('.mat-mdc-select-value'), + (value) => value.textContent?.trim(), + ); + return { fixture, values }; + } + + it('keeps the default animation when saving other changes', async () => { + // ts-client turns the saved index 0 into `cut` + const { fixture, values } = await renderWithAnimation( + new SignagePlaylist({ + default_animation: 0 as unknown as MediaAnimation, + }).default_animation, + ); + const component = fixture.componentInstance; + component.model.update((model) => ({ ...model, name: 'Renamed' })); + + await component.savePlaylist(); + + expect(values).toContain('Default'); + expect(onEdit.mock.calls[0][1]).not.toHaveProperty('default_animation'); + }); + + it('sends the animation when the user changes it', async () => { + const { fixture } = await renderWithAnimation(2); + const component = fixture.componentInstance; + component.model.update((model) => ({ + ...model, + default_animation: MediaAnimation.SlideTop, + })); + + await component.savePlaylist(); + + expect(onEdit.mock.calls[0][1]).toMatchObject({ + default_animation: MediaAnimation.SlideTop, + }); + }); + + it('shows a saved animation index as its animation', async () => { + const { fixture, values } = await renderWithAnimation(2); + + expect(values).toContain('Cross Fade'); + expect(fixture.componentInstance.model().default_animation).toBe( + MediaAnimation.CrossFade, + ); + }); + + it('selects the saved animation and stores the picked enum value', async () => { + const { fixture, values } = await renderWithAnimation( + MediaAnimation.CrossFade, + ); + const element: HTMLElement = fixture.nativeElement; + + expect(values).toContain('Cross Fade'); + + const triggers = element.querySelectorAll( + '.mat-mdc-select-trigger', + ); + triggers[triggers.length - 1].click(); + fixture.detectChanges(); + await fixture.whenStable(); + const cut = Array.from( + document.querySelectorAll('mat-option'), + ).find((option) => option.textContent?.trim() === 'Cut'); + cut?.click(); + fixture.detectChanges(); + + expect(fixture.componentInstance.model().default_animation).toBe( + MediaAnimation.Cut, + ); + }); + it('blocks saving a schedule with reversed validity limits', async () => { const component = TestBed.createComponent( PlaylistEditModalComponent, @@ -204,6 +353,23 @@ describe('PlaylistEditModalComponent', () => { expect(save).toHaveBeenCalled(); }); + it('ignores the S hotkey while a select has focus', () => { + const fixture = TestBed.createComponent(PlaylistEditModalComponent); + const save = vi + .spyOn(fixture.componentInstance, 'savePlaylist') + .mockResolvedValue(); + const select = document.createElement('div'); + select.setAttribute('role', 'combobox'); + select.tabIndex = 0; + document.body.appendChild(select); + select.focus(); + + hotkey_callback(); + + expect(save).not.toHaveBeenCalled(); + select.remove(); + }); + it('starts blank validity dates as empty values', () => { const fixture = TestBed.createComponent(PlaylistEditModalComponent); const component = fixture.componentInstance; diff --git a/apps/signage-manager/src/tests/shared/playlist-item-schedule-modal.component.spec.ts b/apps/signage-manager/src/tests/shared/playlist-item-schedule-modal.component.spec.ts index 98c0e6a797e..761bdc3617b 100644 --- a/apps/signage-manager/src/tests/shared/playlist-item-schedule-modal.component.spec.ts +++ b/apps/signage-manager/src/tests/shared/playlist-item-schedule-modal.component.spec.ts @@ -322,7 +322,7 @@ describe('PlaylistItemScheduleModalComponent', () => { save.mockRejectedValue(new Error('nope')); const component = await createComponent(); - await component.saveSchedule().catch(() => undefined); + await expect(component.saveSchedule()).resolves.toBeUndefined(); expect(component.loading()).toBe(false); expect(dialog_ref.disableClose).toBe(false); diff --git a/apps/signage-manager/src/tests/signage-playlist.util.spec.ts b/apps/signage-manager/src/tests/signage-playlist.util.spec.ts index 175b7077d99..6503e78c8d7 100644 --- a/apps/signage-manager/src/tests/signage-playlist.util.spec.ts +++ b/apps/signage-manager/src/tests/signage-playlist.util.spec.ts @@ -1,7 +1,10 @@ +import { MediaAnimation } from '@placeos/ts-client'; import { getUnixTime } from 'date-fns'; import { createScheduleMaskFilter, + mediaAnimation, playEndTime, + playlistAnimation, playlistItemScheduleMap, playlistLoopDuration, playlistMediaIds, @@ -11,6 +14,7 @@ import { playlistScheduleExpiryTooltip, playlistScheduleLabel, playlistScheduleNextPlayLabels, + playlistStatus, playOnceStart, } from '../app/signage-playlist.util'; @@ -335,3 +339,98 @@ describe('schedule masks', () => { expect(playlistLoopDuration(items)).toBe(67_000); }); }); + +describe('playlist status', () => { + const now = Date.UTC(2026, 0, 10); + const seconds = (time: number) => Math.floor(time / 1000); + const day = 86_400_000; + + it('marks a playlist expired when it or all its schedules have ended', () => { + expect( + playlistStatus( + { id: 'a', valid_until: seconds(now - day) }, + {}, + {}, + now, + ), + ).toBe('expired'); + expect( + playlistStatus( + { + id: 'b', + schedules: [{ valid_until: seconds(now - day) }], + }, + {}, + {}, + now, + ), + ).toBe('expired'); + }); + + it('marks a playlist pending before it starts', () => { + expect( + playlistStatus( + { id: 'a', valid_from: seconds(now + day) }, + {}, + {}, + now, + ), + ).toBe('pending'); + }); + + it('separates approval required from awaiting review', () => { + const approvals = { a: false, b: false, c: true }; + const requests = { b: true }; + + expect(playlistStatus({ id: 'a' }, approvals, requests, now)).toBe( + 'awaiting_approval', + ); + expect(playlistStatus({ id: 'b' }, approvals, requests, now)).toBe( + 'awaiting_review', + ); + expect(playlistStatus({ id: 'c' }, approvals, requests, now)).toBe( + null, + ); + expect(playlistStatus({ id: 'd' }, approvals, requests, now)).toBe( + null, + ); + }); +}); + +describe('media animation', () => { + it('maps a saved index to its animation', () => { + expect(mediaAnimation(0)).toBe(MediaAnimation.Default); + expect(mediaAnimation(2)).toBe(MediaAnimation.CrossFade); + expect(mediaAnimation(6)).toBe(MediaAnimation.SlideBottom); + }); + + it('keeps animation names', () => { + expect(mediaAnimation(MediaAnimation.SlideTop)).toBe( + MediaAnimation.SlideTop, + ); + }); + + it('uses the default for an index out of range or no value', () => { + for (const value of [-1, 7, 1.5, Number.NaN, null, undefined]) { + expect(mediaAnimation(value)).toBe(MediaAnimation.Default); + } + }); +}); + +describe('playlist animation', () => { + it('reads the cut that ts-client puts in place of index 0 as the default', () => { + expect( + playlistAnimation({ default_animation: MediaAnimation.Cut }), + ).toBe(MediaAnimation.Default); + expect(playlistAnimation({})).toBe(MediaAnimation.Default); + }); + + it('reads a saved cut and other indexes as their animation', () => { + expect(playlistAnimation({ default_animation: 1 })).toBe( + MediaAnimation.Cut, + ); + expect(playlistAnimation({ default_animation: 2 })).toBe( + MediaAnimation.CrossFade, + ); + }); +}); 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 e4c48db2c30..da20c7e3a93 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 @@ -37,6 +37,7 @@ describe('ZoneContentComponent', () => { playlistsById: (ids: readonly string[]) => playlists().filter(({ id }) => ids.includes(id)), playlist_approval_status, + playlist_approval_requested_status: signal>({}), playlist_thumbnail_media, playlists_loading, playlists_error, diff --git a/shared/assets/locale/ar.json b/shared/assets/locale/ar.json index 786d8fcebe7..5b619b0cf0a 100644 --- a/shared/assets/locale/ar.json +++ b/shared/assets/locale/ar.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "الشاشة", "SOURCE_VIA": "عبر {{ source }}", "START": "البدء", - "STATUS_AWAITING_APPROVAL": "بانتظار الاعتماد", "STATUS_AWAITING_REVIEW": "بانتظار المراجعة", "STATUS_EXPIRED": "منتهي الصلاحية", "SUMMARY_ADVANCED": "جدول متكرر متقدم. اختر نمط تكرار لتعديله.", diff --git a/shared/assets/locale/en-AU.json b/shared/assets/locale/en-AU.json index 202284be758..2a9c91754f2 100644 --- a/shared/assets/locale/en-AU.json +++ b/shared/assets/locale/en-AU.json @@ -464,14 +464,18 @@ "PLAYLISTS_PAGE_TITLE": "Signage Playlists", "PLAYLIST_APPROVED": "Playlist approved", "PLAYLIST_APPROVE_ERROR": "Error approving playlist", + "PLAYLIST_CHANGED_BEFORE_APPROVAL": "The playlist changed while you reviewed it. Check the new version, then approve it.", "PLAYLIST_COUNT_LABEL": "{{ count }} playlists", "PLAYLIST_COUNT_LABEL_1": "{{ count }} playlist", "PLAYLIST_DESCRIPTION_ARIA": "Playlist description", "PLAYLIST_DETAILS_TABS": "Playlist details tabs", "PLAYLIST_DISTRIBUTION": "Distribution Playlist", "PLAYLIST_EDIT": "Edit Playlist", + "PLAYLIST_ITEM_MOVE_DOWN": "Move down", + "PLAYLIST_ITEM_MOVE_UP": "Move up", "PLAYLIST_LABEL": "Playlist", "PLAYLIST_NAME_ARIA": "Playlist name", + "PLAYLIST_NOT_FOUND": "Could not open the playlist. It may have been deleted.", "PLAYLIST_ORIENTATION_ARIA": "Playlist orientation", "PLAYLIST_REVERTED": "Playlist reverted to previous version", "PLAYLIST_REVERT_ERROR": "Error reverting playlist changes", @@ -579,7 +583,6 @@ "SOURCE_DISPLAY": "Display", "SOURCE_VIA": "via {{ source }}", "START": "Start", - "STATUS_AWAITING_APPROVAL": "Awaiting Approval", "STATUS_AWAITING_REVIEW": "Awaiting Review", "STATUS_EXPIRED": "Expired", "SUMMARY_ADVANCED": "Advanced recurring schedule. Choose a repeat pattern to edit it.", @@ -612,6 +615,7 @@ "SVC_DISPLAY_REMOVE_ERROR": "Could not remove the display. Please try again.", "SVC_DISPLAY_SAVED": "Display saved", "SVC_DISPLAY_SAVE_ERROR": "Could not save the display. Please try again.", + "SVC_ERR_ADD_PLAYLIST_ITEMS": "Error adding media to the playlist", "SVC_ERR_ADD_USER": "Error adding group user", "SVC_ERR_ADD_ZONE": "Error adding group zone", "SVC_ERR_CONVERT_IMAGE": "Unable to convert image", diff --git a/shared/assets/locale/en-GB.json b/shared/assets/locale/en-GB.json index e34556e9cb8..e0ca5c0f8d0 100644 --- a/shared/assets/locale/en-GB.json +++ b/shared/assets/locale/en-GB.json @@ -2443,14 +2443,18 @@ "PLAYLISTS_PAGE_TITLE": "Signage Playlists", "PLAYLIST_APPROVED": "Playlist approved", "PLAYLIST_APPROVE_ERROR": "Error approving playlist", + "PLAYLIST_CHANGED_BEFORE_APPROVAL": "The playlist changed while you reviewed it. Check the new version, then approve it.", "PLAYLIST_COUNT_LABEL": "{{ count }} playlists", "PLAYLIST_COUNT_LABEL_1": "{{ count }} playlist", "PLAYLIST_DESCRIPTION_ARIA": "Playlist description", "PLAYLIST_DETAILS_TABS": "Playlist details tabs", "PLAYLIST_DISTRIBUTION": "Distribution Playlist", "PLAYLIST_EDIT": "Edit Playlist", + "PLAYLIST_ITEM_MOVE_DOWN": "Move down", + "PLAYLIST_ITEM_MOVE_UP": "Move up", "PLAYLIST_LABEL": "Playlist", "PLAYLIST_NAME_ARIA": "Playlist name", + "PLAYLIST_NOT_FOUND": "Could not open the playlist. It may have been deleted.", "PLAYLIST_ORIENTATION_ARIA": "Playlist orientation", "PLAYLIST_REVERTED": "Playlist reverted to previous version", "PLAYLIST_REVERT_ERROR": "Error reverting playlist changes", @@ -2549,7 +2553,6 @@ "SOURCE_DISPLAY": "Display", "SOURCE_VIA": "via {{ source }}", "START": "Start", - "STATUS_AWAITING_APPROVAL": "Awaiting Approval", "STATUS_AWAITING_REVIEW": "Awaiting Review", "STATUS_EXPIRED": "Expired", "SUMMARY_ADVANCED": "Advanced recurring schedule. Choose a repeat pattern to edit it.", @@ -2579,6 +2582,7 @@ "SVC_DISPLAY_REMOVED_ZONE": "Display removed from zone", "SVC_DISPLAY_REMOVE_ERROR": "Could not remove the display. Please try again.", "SVC_DISPLAY_SAVE_ERROR": "Could not save the display. Please try again.", + "SVC_ERR_ADD_PLAYLIST_ITEMS": "Error adding media to the playlist", "SVC_ERR_ADD_USER": "Error adding group user", "SVC_ERR_ADD_ZONE": "Error adding group zone", "SVC_ERR_CONVERT_IMAGE": "Unable to convert image", diff --git a/shared/assets/locale/en-US.json b/shared/assets/locale/en-US.json index 96ffa627ac1..84f36392694 100644 --- a/shared/assets/locale/en-US.json +++ b/shared/assets/locale/en-US.json @@ -2464,14 +2464,18 @@ "PLAYLISTS_PAGE_TITLE": "Signage Playlists", "PLAYLIST_APPROVED": "Playlist approved", "PLAYLIST_APPROVE_ERROR": "Error approving playlist", + "PLAYLIST_CHANGED_BEFORE_APPROVAL": "The playlist changed while you reviewed it. Check the new version, then approve it.", "PLAYLIST_COUNT_LABEL": "{{ count }} playlists", "PLAYLIST_COUNT_LABEL_1": "{{ count }} playlist", "PLAYLIST_DESCRIPTION_ARIA": "Playlist description", "PLAYLIST_DETAILS_TABS": "Playlist details tabs", "PLAYLIST_DISTRIBUTION": "Distribution Playlist", "PLAYLIST_EDIT": "Edit Playlist", + "PLAYLIST_ITEM_MOVE_DOWN": "Move down", + "PLAYLIST_ITEM_MOVE_UP": "Move up", "PLAYLIST_LABEL": "Playlist", "PLAYLIST_NAME_ARIA": "Playlist name", + "PLAYLIST_NOT_FOUND": "Could not open the playlist. It may have been deleted.", "PLAYLIST_ORIENTATION_ARIA": "Playlist orientation", "PLAYLIST_REVERTED": "Playlist reverted to previous version", "PLAYLIST_REVERT_ERROR": "Error reverting playlist changes", @@ -2576,7 +2580,6 @@ "SOURCE_DISPLAY": "Display", "SOURCE_VIA": "via {{ source }}", "START": "Start", - "STATUS_AWAITING_APPROVAL": "Awaiting Approval", "STATUS_AWAITING_REVIEW": "Awaiting Review", "STATUS_EXPIRED": "Expired", "SUMMARY_ADVANCED": "Advanced recurring schedule. Choose a repeat pattern to edit it.", @@ -2608,6 +2611,7 @@ "SVC_DISPLAY_REMOVED_ZONE": "Display removed from zone", "SVC_DISPLAY_SAVED": "Display saved", "SVC_DISPLAY_SAVE_ERROR": "Could not save the display. Please try again.", + "SVC_ERR_ADD_PLAYLIST_ITEMS": "Error adding media to the playlist", "SVC_ERR_ADD_USER": "Error adding group user", "SVC_ERR_ADD_ZONE": "Error adding group zone", "SVC_ERR_CONVERT_IMAGE": "Unable to convert image", diff --git a/shared/assets/locale/es.json b/shared/assets/locale/es.json index 56b45c7d660..89063ca4c2f 100644 --- a/shared/assets/locale/es.json +++ b/shared/assets/locale/es.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "Pantalla", "SOURCE_VIA": "mediante {{ source }}", "START": "Inicio", - "STATUS_AWAITING_APPROVAL": "Pendiente de aprobación", "STATUS_AWAITING_REVIEW": "Pendiente de revisión", "STATUS_EXPIRED": "Vencido", "SUMMARY_ADVANCED": "Programación recurrente avanzada. Elige un patrón de repetición para editarla.", diff --git a/shared/assets/locale/fr-CA.json b/shared/assets/locale/fr-CA.json index 61188af61b7..461314082d7 100644 --- a/shared/assets/locale/fr-CA.json +++ b/shared/assets/locale/fr-CA.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "Écran", "SOURCE_VIA": "via {{ source }}", "START": "Début", - "STATUS_AWAITING_APPROVAL": "En attente d'approbation", "STATUS_AWAITING_REVIEW": "En attente de révision", "STATUS_EXPIRED": "Expiré", "SUMMARY_ADVANCED": "Planification récurrente avancée. Choisissez un modèle de répétition pour la modifier.", diff --git a/shared/assets/locale/fr.json b/shared/assets/locale/fr.json index 20ba0c1054f..4b6d9554e5a 100644 --- a/shared/assets/locale/fr.json +++ b/shared/assets/locale/fr.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "Écran", "SOURCE_VIA": "via {{ source }}", "START": "Début", - "STATUS_AWAITING_APPROVAL": "En attente d'approbation", "STATUS_AWAITING_REVIEW": "En attente de révision", "STATUS_EXPIRED": "Expiré", "SUMMARY_ADVANCED": "Planification récurrente avancée. Choisissez un modèle de répétition pour la modifier.", diff --git a/shared/assets/locale/it.json b/shared/assets/locale/it.json index 729e2aef16a..acb317667ad 100644 --- a/shared/assets/locale/it.json +++ b/shared/assets/locale/it.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "Schermo", "SOURCE_VIA": "tramite {{ source }}", "START": "Inizio", - "STATUS_AWAITING_APPROVAL": "In attesa di approvazione", "STATUS_AWAITING_REVIEW": "In attesa di revisione", "STATUS_EXPIRED": "Scaduto", "SUMMARY_ADVANCED": "Pianificazione ricorrente avanzata. Scegli un modello di ripetizione per modificarla.", diff --git a/shared/assets/locale/ja-JP.json b/shared/assets/locale/ja-JP.json index ebe0a75c349..8d759f782c0 100644 --- a/shared/assets/locale/ja-JP.json +++ b/shared/assets/locale/ja-JP.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "ディスプレイ", "SOURCE_VIA": "{{ source }} 経由", "START": "開始", - "STATUS_AWAITING_APPROVAL": "承認待ち", "STATUS_AWAITING_REVIEW": "レビュー待ち", "STATUS_EXPIRED": "期限切れ", "SUMMARY_ADVANCED": "高度な繰り返しスケジュールです。編集するには繰り返しパターンを選択してください。", diff --git a/shared/assets/locale/pt.json b/shared/assets/locale/pt.json index 7d5f0789cec..87bc83f5e32 100644 --- a/shared/assets/locale/pt.json +++ b/shared/assets/locale/pt.json @@ -2312,7 +2312,6 @@ "SOURCE_DISPLAY": "Ecrã", "SOURCE_VIA": "através de {{ source }}", "START": "Início", - "STATUS_AWAITING_APPROVAL": "A aguardar aprovação", "STATUS_AWAITING_REVIEW": "A aguardar revisão", "STATUS_EXPIRED": "Expirado", "SUMMARY_ADVANCED": "Agenda recorrente avançada. Escolha um padrão de repetição para a editar.", diff --git a/shared/assets/locale/zh.json b/shared/assets/locale/zh.json index 6fca8080cba..ff4bbd95e24 100644 --- a/shared/assets/locale/zh.json +++ b/shared/assets/locale/zh.json @@ -2313,7 +2313,6 @@ "SOURCE_DISPLAY": "显示屏", "SOURCE_VIA": "通过 {{ source }}", "START": "开始", - "STATUS_AWAITING_APPROVAL": "等待审批", "STATUS_AWAITING_REVIEW": "等待审核", "STATUS_EXPIRED": "已过期", "SUMMARY_ADVANCED": "高级重复计划。请选择一个重复模式进行编辑。", From 7f032085b012b6c22efa5cc6c10d9bb1cb808769 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 2 Oct 2026 13:22:28 +1000 Subject: [PATCH 2/2] fix(signage-manager): retry the failed playlist page and skip stale link warnings - reloadPlaylists() retries the failed page first and reloads from page one only when the first page failed, so loaded pages are kept. - A linked playlist that fails to load after the user moved on no longer warns or clears the selection. --- .../src/app/playlists/playlists.component.ts | 5 ++-- .../app/playlists/signage-playlist.service.ts | 9 ++++-- .../playlists/playlists.component.spec.ts | 20 +++++++++++++ .../src/tests/signage-playlist-search.spec.ts | 29 +++++++++++++++++++ 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/apps/signage-manager/src/app/playlists/playlists.component.ts b/apps/signage-manager/src/app/playlists/playlists.component.ts index 7bfa861c60d..8d48916dead 100644 --- a/apps/signage-manager/src/app/playlists/playlists.component.ts +++ b/apps/signage-manager/src/app/playlists/playlists.component.ts @@ -283,12 +283,13 @@ export class PlaylistsSectionComponent { /** * Fetch a linked playlist that the loaded pages lack. When it cannot - * load, clear the selection, so no other playlist shows under its link. + * load, warn and clear the selection, so no other playlist shows under + * its link. Does nothing when the user has opened another link since. */ private async _loadLinkedPlaylist(id: string) { if (await this._playlist_service.loadPlaylist(id)) return; - notifyWarn(i18n('SIGNAGE_MANAGER.PLAYLIST_NOT_FOUND')); if (this.id() !== id) return; + notifyWarn(i18n('SIGNAGE_MANAGER.PLAYLIST_NOT_FOUND')); this._playlist_service.selected_playlist.set(null); this._playlist_service.selected_playlist_item.set(null); this._playlist_service.selected_playlist_item_index.set(null); diff --git a/apps/signage-manager/src/app/playlists/signage-playlist.service.ts b/apps/signage-manager/src/app/playlists/signage-playlist.service.ts index c7d2d5c4512..da4af0a7d1d 100644 --- a/apps/signage-manager/src/app/playlists/signage-playlist.service.ts +++ b/apps/signage-manager/src/app/playlists/signage-playlist.service.ts @@ -199,9 +199,14 @@ export class SignagePlaylistService { }); }); - /** Load the playlist list again from the first page, e.g. after an error */ + /** + * Load the playlist page that failed again: the next page when some + * pages are loaded, so they stay, otherwise the first page. + */ public reloadPlaylists() { - this._playlists_retry.update((count) => count + 1); + if (!this._playlist_list.retry()) { + this._playlists_retry.update((count) => count + 1); + } } public loadMorePlaylists() { diff --git a/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts b/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts index 7f5b888c5d6..ac3c090cb56 100644 --- a/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts +++ b/apps/signage-manager/src/tests/playlists/playlists.component.spec.ts @@ -160,6 +160,26 @@ describe('PlaylistsSectionComponent', () => { expect(selected_playlist()).toBeNull(); }); + it('does not warn about a link the user has already left', async () => { + let resolveLoad: (value: null) => void = () => {}; + load_playlist.mockReturnValueOnce( + new Promise((resolve) => (resolveLoad = resolve)), + ); + const open = { id: 'pl-1' }; + playlists.set([open]); + await make(); + fixture.componentRef.setInput('id', 'deleted'); + fixture.detectChanges(); + + fixture.componentRef.setInput('id', 'pl-1'); + fixture.detectChanges(); + resolveLoad(null); + await fixture.whenStable(); + + expect(notify_open).not.toHaveBeenCalled(); + expect(selected_playlist()).toBe(open); + }); + it('clears the selection once the route id is removed', async () => { const match = { id: 'pl-1' }; playlists.set([match]); diff --git a/apps/signage-manager/src/tests/signage-playlist-search.spec.ts b/apps/signage-manager/src/tests/signage-playlist-search.spec.ts index 6f175f076be..dd4771c3cd6 100644 --- a/apps/signage-manager/src/tests/signage-playlist-search.spec.ts +++ b/apps/signage-manager/src/tests/signage-playlist-search.spec.ts @@ -128,6 +128,35 @@ describe('SignagePlaylistService playlist search', () => { ]); }); + it('loads a later page again on retry and keeps the loaded pages', async () => { + let fail = true; + (querySignagePlaylists as any).mockResolvedValue({ + ...pageOf(['events'], 2), + next: () => + fail + ? Promise.reject(new Error('offline')) + : Promise.resolve(pageOf(['news'], 2)), + }); + const service = TestBed.inject(SignagePlaylistService); + TestBed.tick(); + await flush(); + service.loadMorePlaylists(); + await flush(); + expect(service.playlists_error()).toBe(true); + + fail = false; + service.reloadPlaylists(); + TestBed.tick(); + await flush(); + + expect(querySignagePlaylists).toHaveBeenCalledTimes(1); + expect(service.playlists_error()).toBe(false); + expect(service.filtered_playlists().map(({ id }) => id)).toEqual([ + 'events', + 'news', + ]); + }); + it('keeps loaded playlists available for id lookups while searching', async () => { const service = await init(); expect(service.playlists().map((item: any) => item.id)).toEqual([