From 85e6cb38167afc1f00a2554be465363caaa0a907 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 18:15:09 +0200 Subject: [PATCH] fix(sources): sort by the shown name; start the benchmark's Undo window in its trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local review round 3 (Greptile; Codex was clean): - Name order compared the stored title, while the rows show a URL-named or credential-bearing source by its host: a source stored as get.php?username=… sorted by its hidden account details. sortPlaylists takes the list's name function, and Sources and the switcher pass sourceSortName, the name they display. - The Xtream delete benchmark clicked Remove while preparing, which starts the four-second Undo window; capture setup before the trigger could outlast it, so the delete committed on its own before the measurement and the close button was gone. Preparation now stops at the open menu, and the trigger clicks Remove and then the toast's close button, with the trigger time taken right before the commit click as before. Co-Authored-By: Claude Opus 5.5 --- .../xtream-scenario-driver.spec.ts | 4 +++ .../src/performance/xtream-scenario-driver.ts | 7 +++++- .../src/performance/xtream-scenario-ui.ts | 18 ++++++++++--- .../playlist-switcher-panel.component.ts | 4 ++- .../recent-playlists.component.spec.ts | 16 ++++++++++++ .../recent-playlists.component.ts | 10 ++++++-- .../lib/source-display/source-display.spec.ts | 10 ++++++++ .../src/lib/source-display/source-display.ts | 5 ++++ libs/services/src/lib/sort.service.spec.ts | 25 +++++++++++++++++++ libs/services/src/lib/sort.service.ts | 13 +++++++--- 10 files changed, 102 insertions(+), 10 deletions(-) diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts index e8bf5c2e9..2f7b68f72 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts @@ -207,7 +207,11 @@ describe('Xtream Playwright scenario driver', () => { fake.asPage(), scenarioOptions(XTREAM_SCENARIO_ID.DELETE_LARGE) ); + // Preparation stops at the open menu: Remove would start the Undo + // window, which commits on its own before a slow capture setup ends. + assert.equal(fake.clicksMatching('filter(Remove playlist)'), 0); await prepared.trigger(); + assert.equal(fake.clicksMatching('filter(Remove playlist)'), 1); const armIndex = fake.log.indexOf('evaluate:install-operation'); const clickIndex = fake.log.findIndex( (entry) => diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts index 2846e6034..9acba2d41 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts @@ -19,6 +19,7 @@ import { prepareXtreamSourceActionTrigger, waitForXtreamCancellationTerminal, waitForXtreamCatalogTerminal, + removeXtreamSource, waitForXtreamSourceRowGone, } from './xtream-scenario-ui'; import { assertXtreamMockOrigin } from './xtream-control-client'; @@ -202,8 +203,12 @@ function createPreparedScenario(state: PreparedState): PreparedXtreamScenario { 'delete-playlist' ); } + let commit = state.trigger; + if (state.options.scenarioId === XTREAM_SCENARIO_ID.DELETE_LARGE) { + commit = await removeXtreamSource(state.page, state.trigger); + } const triggerEpochMs = Date.now(); - await state.trigger.click(); + await commit.click(); return { scenarioId: state.options.scenarioId, triggerEpochMs, diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts index aa254c241..460d8bbcf 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts @@ -87,9 +87,9 @@ export async function prepareXtreamSourceActionTrigger( return confirmation; } - // Delete leaves the list at once and offers Undo; the toast's close - // button commits it without waiting out the Undo window, so the - // measurement starts when the delete does. + // Remove starts the Undo window, which commits on its own after four + // seconds: preparation stops at the open menu, and the trigger removes + // and commits at once (removeXtreamSource). const more = row.locator('.source-row__more'); await assertTriggerReady(more, 'xtream-source-delete-menu'); await more.click(); @@ -98,6 +98,18 @@ export async function prepareXtreamSourceActionTrigger( .filter({ hasText: 'Remove playlist' }) .first(); await assertTriggerReady(remove, 'xtream-source-delete'); + return remove; +} + +/** + * Clicks Remove and returns the Undo toast's close button, which commits the + * delete without waiting out the Undo window: the measured delete starts + * with that click. + */ +export async function removeXtreamSource( + page: Page, + remove: Locator +): Promise { await remove.click(); const commit = page.locator( 'mat-snack-bar-container.app-toast .source-removal-toast__close' diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts index fdf23f8f7..0bb26d390 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts @@ -28,6 +28,7 @@ import { PlaylistContextFacade, PlaylistRecentHistoryService, SourceBucketOf, + sourceSortName, SourcesTypeFilter, } from '@iptvnator/playlist/shared/util'; import { SortBy, SortService } from '@iptvnator/services'; @@ -138,7 +139,8 @@ export class PlaylistSwitcherPanelComponent { const options = this.sortOptions(); const sorted = this.sortService.sortPlaylists( this.playlists(), - options + options, + sourceSortName ); return options.failingFirst && options.by !== SortBy.CUSTOM ? orderFailingFirst(sorted, this.bucketOf) diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts index ca4904d02..2753485de 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts @@ -190,6 +190,22 @@ describe('RecentPlaylistsComponent', () => { restarted.destroy(); }); + it('sorts by the name it shows: the host for a URL-named source', () => { + store.overrideSelector(selectAllPlaylistsMeta, [ + M3U, + XTREAM, + FILE, + source('url', { + title: 'get.php?username=a&password=b', + url: 'http://zzz.example/get.php?username=a&password=b', + }), + ]); + store.refreshState(); + sort.next({ by: SortBy.NAME, order: SortOrder.ASC }); + fixture.detectChanges(); + expect(ids()).toEqual(['file', 'm3u', 'xtream', 'url']); + }); + it('applies the type and status filters together', () => { viewState.setType('m3u'); expect(ids()).toEqual(['m3u', 'file']); diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts index 5d7d8d04a..bc602026e 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts @@ -32,6 +32,7 @@ import { orderFailingFirst, PlaylistContextFacade, SourceBucketOf, + sourceSortName, SourcesViewStateService, } from '@iptvnator/playlist/shared/util'; import { @@ -128,7 +129,11 @@ export class RecentPlaylistsComponent { this.searchQueryInput(), bucketOf ); - const sorted = this.sortService.sortPlaylists(filtered, options); + const sorted = this.sortService.sortPlaylists( + filtered, + options, + sourceSortName + ); return options.failingFirst && options.by !== SortBy.CUSTOM ? orderFailingFirst(sorted, bucketOf) : sorted; @@ -155,7 +160,8 @@ export class RecentPlaylistsComponent { const visibleIds = new Set(visible.map((item) => item._id)); const ordered = this.sortService.sortPlaylists( this.allPlaylists(), - this.sortOptions() + this.sortOptions(), + sourceSortName ); let shown = 0; const merged = ordered.map((item) => diff --git a/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts b/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts index 74696d6a5..d4a9bda7f 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts @@ -1,6 +1,7 @@ import { sourceCopyUrl, sourceDisplayName, + sourceSortName, sourceDisplayUrl, sourceHost, } from './source-display'; @@ -83,6 +84,15 @@ describe('sourceDisplayName', () => { }); }); +describe('sourceSortName', () => { + it('sorts a source by the name it shows', () => { + expect(sourceSortName({ title: 'get.php?x=1', url: M3U_URL })).toBe( + sourceDisplayName({ title: 'get.php?x=1', url: M3U_URL }) + ); + expect(sourceSortName({ title: '' })).toBe(''); + }); +}); + describe('sourceDisplayUrl', () => { it('shows an M3U URL without credentials', () => { expect(sourceDisplayUrl({ url: M3U_URL })).toBe( diff --git a/libs/playlist/shared/util/src/lib/source-display/source-display.ts b/libs/playlist/shared/util/src/lib/source-display/source-display.ts index 25012e48d..9af934e79 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-display.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-display.ts @@ -135,6 +135,11 @@ export function sourceDisplayName( return containsSourceSecret(raw, playlist) ? host : raw; } +/** The name a list shows and sorts a source by; empty when it has none. */ +export function sourceSortName(playlist: SourceDisplayFields): string { + return sourceDisplayName(playlist) ?? ''; +} + /** `origin/…/last` once a path has more than two segments. */ function compactPath(pathname: string): string { const segments = pathname.split('/').filter(Boolean); diff --git a/libs/services/src/lib/sort.service.spec.ts b/libs/services/src/lib/sort.service.spec.ts index b6358b3d2..fff83b199 100644 --- a/libs/services/src/lib/sort.service.spec.ts +++ b/libs/services/src/lib/sort.service.spec.ts @@ -41,6 +41,31 @@ describe('SortService', () => { expect(ids).toEqual(['refreshed', 'new', 'old']); }); + it('sorts by name as the list shows it, when told how', () => { + const sources = [ + playlist('zed', { title: 'zed' }), + playlist('url', { title: 'get.php?username=zzz&password=x' }), + playlist('mid', { title: 'mid' }), + ]; + const service = create(); + const byTitle = service + .sortPlaylists(sources, { by: SortBy.NAME, order: SortOrder.ASC }) + .map((item) => item._id); + expect(byTitle).toEqual(['url', 'mid', 'zed']); + + // The URL-named source shows as its host, "abc.test". + const shown = (item: PlaylistMeta) => + item._id === 'url' ? 'abc.test' : item.title; + const byShownName = service + .sortPlaylists( + sources, + { by: SortBy.NAME, order: SortOrder.DESC }, + shown + ) + .map((item) => item._id); + expect(byShownName).toEqual(['zed', 'mid', 'url']); + }); + it('sorts by channel count, with no count as zero', () => { const ids = create() .sortPlaylists( diff --git a/libs/services/src/lib/sort.service.ts b/libs/services/src/lib/sort.service.ts index 522569b62..59035911f 100644 --- a/libs/services/src/lib/sort.service.ts +++ b/libs/services/src/lib/sort.service.ts @@ -103,9 +103,16 @@ export class SortService { } } + /** + * @param nameOf The name a list shows for a source, for Name order: a + * list that shows a source by its host must sort it by its host, not by + * a stored title such as `get.php?username=…`. + */ sortPlaylists( playlists: PlaylistMeta[], - options: SortOptions + options: SortOptions, + nameOf: (playlist: PlaylistMeta) => string = (playlist) => + playlist.title ): PlaylistMeta[] { const { by, order } = options; @@ -113,9 +120,9 @@ export class SortService { let comparison = 0; if (by === SortBy.NAME) { - comparison = a.title + comparison = nameOf(a) .toLowerCase() - .localeCompare(b.title.toLowerCase()); + .localeCompare(nameOf(b).toLowerCase()); } else if (by === SortBy.DATE_ADDED) { const dateA = new Date(a.importDate).getTime(); const dateB = new Date(b.importDate).getTime();