From 0ac605fbe889ede1b7cbe52793c2f21aeaa4322c Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 16 Aug 2026 07:52:36 +0200 Subject: [PATCH] fix(portals): address review findings on the Discover pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The matching indicator was keyed on the request's subject, so an obsolete response skipped clearing it while no replacement request ever ran — the results grid then sat under the spinner forever. `createLatestRequestGuard()` now owns the indicator: the newest request always clears it, and the subject check keeps deciding whether the RESULT is still wanted. The actor pages carried the same latent bug and use the same guard now. - Country chips came from `production_countries` while Discover filters by `with_origin_country`, so clicking a co-production partner returned titles originating there instead of titles it produced. Chips are now built from `origin_country` and labelled from `production_countries`; a code TMDB does not name is dropped rather than shown as a bare code. - A cold load of an `actor` or `discover` route never initialized the catalog, so every result claimed to be missing from a library that actually holds it. Both are import-driven sections now. Co-Authored-By: Claude Fable 5 --- docs/architecture/tmdb-metadata-enrichment.md | 16 ++++-- libs/portal/shared/util/src/index.ts | 1 + .../util/src/lib/latest-request-guard.spec.ts | 50 +++++++++++++++++++ .../util/src/lib/latest-request-guard.ts | 28 +++++++++++ .../src/lib/stalker-actor-route.component.ts | 13 ++++- .../lib/stalker-discover-route.component.ts | 13 ++++- .../lib/actor/xtream-actor-route.component.ts | 13 ++++- .../xtream-discover-route.component.ts | 13 ++++- .../xtream-workspace-route-session.service.ts | 16 +++++- .../src/lib/tmdb/tmdb-merge-stalker.spec.ts | 1 + libs/services/src/lib/tmdb/tmdb-merge.spec.ts | 33 ++++++++++++ libs/services/src/lib/tmdb/tmdb-merge.ts | 25 +++++++--- libs/services/src/lib/tmdb/tmdb.types.ts | 7 +++ 13 files changed, 209 insertions(+), 20 deletions(-) create mode 100644 libs/portal/shared/util/src/lib/latest-request-guard.spec.ts create mode 100644 libs/portal/shared/util/src/lib/latest-request-guard.ts diff --git a/docs/architecture/tmdb-metadata-enrichment.md b/docs/architecture/tmdb-metadata-enrichment.md index adc78a468..f653fae8a 100644 --- a/docs/architecture/tmdb-metadata-enrichment.md +++ b/docs/architecture/tmdb-metadata-enrichment.md @@ -352,8 +352,13 @@ else search" pattern as actor pages, generalized to metadata facets **Structured facets from the merge.** All three merge functions in `tmdb-merge.ts` additionally emit `tmdb_genres: {id, name}[]` (from -`details.genres`) and `tmdb_countries: {code, name}[]` (from -`details.production_countries`, ISO 3166-1 alpha-2 codes); +`details.genres`) and `tmdb_countries: {code, name}[]`. Countries come +from `details.origin_country`, NOT the fuller `production_countries`: +Discover filters by `with_origin_country`, so offering a co-production's +other partners would promise "titles from here" and return a different +set. Names are read out of `production_countries` (the only place TMDB +states them) and a code it does not name is dropped rather than rendered +as a bare `FR`; `mergeStalkerInfoWithTmdb` also emits `tmdb_media_type`, because Stalker embedded-VOD series route as movies structurally and the shared detail view needs the real media type for the Discover link. Cached details @@ -394,7 +399,12 @@ scope toggle, same portal/global matching (Xtream in-memory index by the facet's media type; Stalker search-prefill only, global scope matching Xtream playlists), same navigation on click. Because facets change via query params on the SAME route instance, async results are -staleness-guarded by `discoverFacetKey()` rather than by instance. Both +staleness-guarded by `discoverFacetKey()` rather than by instance — +but the in-flight INDICATOR is owned by `createLatestRequestGuard()` +(`libs/portal/shared/util`), because a request whose subject changed +must not clear a spinner a replacement request now owns, and a request +with no replacement (the user left the scope) must still clear it. The +actor pages use the same guard for the same reason. Both render the shared `DiscoverViewComponent`, whose grid is the `TitleResultsComponent` extracted from `ActorViewComponent` (`libs/ui/shared-portals/src/lib/title-results/`) — one grid, filter diff --git a/libs/portal/shared/util/src/index.ts b/libs/portal/shared/util/src/index.ts index adcf5eec4..0771bfead 100644 --- a/libs/portal/shared/util/src/index.ts +++ b/libs/portal/shared/util/src/index.ts @@ -1,3 +1,4 @@ +export * from './lib/latest-request-guard'; export * from './lib/logger'; export * from './lib/category-sort'; export * from './lib/channel-sort'; diff --git a/libs/portal/shared/util/src/lib/latest-request-guard.spec.ts b/libs/portal/shared/util/src/lib/latest-request-guard.spec.ts new file mode 100644 index 000000000..b8ebefb72 --- /dev/null +++ b/libs/portal/shared/util/src/lib/latest-request-guard.spec.ts @@ -0,0 +1,50 @@ +import { createLatestRequestGuard } from './latest-request-guard'; + +describe('createLatestRequestGuard', () => { + it('treats the only started request as latest', () => { + const guard = createLatestRequestGuard(); + + expect(guard.isLatest(guard.start())).toBe(true); + }); + + it('retires every earlier request when a new one starts', () => { + const guard = createLatestRequestGuard(); + + const first = guard.start(); + const second = guard.start(); + + expect(guard.isLatest(first)).toBe(false); + expect(guard.isLatest(second)).toBe(true); + }); + + it('keeps the newest owner even when responses settle out of order', () => { + const guard = createLatestRequestGuard(); + + const first = guard.start(); + const second = guard.start(); + + // The stale response settling last must not reclaim ownership + expect(guard.isLatest(second)).toBe(true); + expect(guard.isLatest(first)).toBe(false); + }); + + it('never reports an unclaimed token as latest', () => { + const guard = createLatestRequestGuard(); + + expect(guard.isLatest(0)).toBe(false); + + guard.start(); + expect(guard.isLatest(99)).toBe(false); + }); + + it('isolates guards from each other', () => { + const one = createLatestRequestGuard(); + const other = createLatestRequestGuard(); + + const token = one.start(); + other.start(); + other.start(); + + expect(one.isLatest(token)).toBe(true); + }); +}); diff --git a/libs/portal/shared/util/src/lib/latest-request-guard.ts b/libs/portal/shared/util/src/lib/latest-request-guard.ts new file mode 100644 index 000000000..ff0de43f2 --- /dev/null +++ b/libs/portal/shared/util/src/lib/latest-request-guard.ts @@ -0,0 +1,28 @@ +/** + * Ownership token for a "only the newest request may commit" sequence. + * + * Pages that re-issue the same request as the user navigates (catalog + * matching on the actor and Discover pages) cannot key their in-flight + * indicator on the SUBJECT of the request: when a replacement request is + * never issued — the user switched away from the scope that needs it — + * the obsolete response finds a changed subject and leaves the spinner + * up forever. The guard separates the two questions: `isLatest` says + * whether this response still owns the shared state, and the caller + * keeps whatever subject check decides if the RESULT is still wanted. + */ +export interface LatestRequestGuard { + /** Claims ownership for a new request and returns its token */ + start(): number; + /** Whether `token` still belongs to the most recently started request */ + isLatest(token: number): boolean; +} + +export function createLatestRequestGuard(): LatestRequestGuard { + // Tokens start at 1 so the "nothing started yet" state cannot be + // mistaken for ownership by a caller holding a zeroed token + let latest = 0; + return { + start: () => ++latest, + isLatest: (token: number) => token > 0 && token === latest, + }; +} diff --git a/libs/portal/stalker/feature/src/lib/stalker-actor-route.component.ts b/libs/portal/stalker/feature/src/lib/stalker-actor-route.component.ts index 760ef4f3b..e25f1c68b 100644 --- a/libs/portal/stalker/feature/src/lib/stalker-actor-route.component.ts +++ b/libs/portal/stalker/feature/src/lib/stalker-actor-route.component.ts @@ -20,6 +20,7 @@ import { pickTitleMatch, } from '@iptvnator/services'; import { CatalogTitleMatch } from '@iptvnator/shared/interfaces'; +import { createLatestRequestGuard } from '@iptvnator/portal/shared/util'; import { ActorViewComponent, ActorViewItem, @@ -71,6 +72,7 @@ export class StalkerActorRouteComponent { readonly scope = signal('portal'); readonly isMatchingGlobal = signal(false); private readonly globalMatches = signal(null); + private readonly matchRequest = createLatestRequestGuard(); private readonly globalIndex = computed(() => groupTitleMatchesByKey(this.globalMatches() ?? []) ); @@ -167,14 +169,21 @@ export class StalkerActorRouteComponent { // previous person must not overwrite the current one's results const requestedPersonId = this.personId(); const titles = this.filmography().map((credit) => credit.title); + const matchToken = this.matchRequest.start(); this.isMatchingGlobal.set(true); try { const matches = await this.titleMatch.matchTitles(titles); - if (this.personId() === requestedPersonId) { + if ( + this.matchRequest.isLatest(matchToken) && + this.personId() === requestedPersonId + ) { this.globalMatches.set(matches); } } finally { - if (this.personId() === requestedPersonId) { + // Only the newest request may clear the indicator. Keying this + // on the subject instead strands the spinner when the user + // leaves the scope and no replacement request ever runs. + if (this.matchRequest.isLatest(matchToken)) { this.isMatchingGlobal.set(false); } } diff --git a/libs/portal/stalker/feature/src/lib/stalker-discover-route.component.ts b/libs/portal/stalker/feature/src/lib/stalker-discover-route.component.ts index f93a1ab41..d5d8cb6b3 100644 --- a/libs/portal/stalker/feature/src/lib/stalker-discover-route.component.ts +++ b/libs/portal/stalker/feature/src/lib/stalker-discover-route.component.ts @@ -17,6 +17,7 @@ import { pickTitleMatch, } from '@iptvnator/services'; import { CatalogTitleMatch } from '@iptvnator/shared/interfaces'; +import { createLatestRequestGuard } from '@iptvnator/portal/shared/util'; import { DiscoverViewComponent, TitleResultsScope, @@ -76,6 +77,7 @@ export class StalkerDiscoverRouteComponent { readonly scope = signal('portal'); readonly isMatchingGlobal = signal(false); private readonly globalMatches = signal(null); + private readonly matchRequest = createLatestRequestGuard(); private readonly globalIndex = computed(() => groupTitleMatchesByKey(this.globalMatches() ?? []) ); @@ -166,14 +168,21 @@ export class StalkerDiscoverRouteComponent { private async loadGlobalMatches(): Promise { const requestedKey = this.facetKey(); const titles = this.results().map((title) => title.title); + const matchToken = this.matchRequest.start(); this.isMatchingGlobal.set(true); try { const matches = await this.titleMatch.matchTitles(titles); - if (this.facetKey() === requestedKey) { + if ( + this.matchRequest.isLatest(matchToken) && + this.facetKey() === requestedKey + ) { this.globalMatches.set(matches); } } finally { - if (this.facetKey() === requestedKey) { + // Only the newest request may clear the indicator. Keying this + // on the subject instead strands the spinner when the user + // leaves the scope and no replacement request ever runs. + if (this.matchRequest.isLatest(matchToken)) { this.isMatchingGlobal.set(false); } } diff --git a/libs/portal/xtream/feature/src/lib/actor/xtream-actor-route.component.ts b/libs/portal/xtream/feature/src/lib/actor/xtream-actor-route.component.ts index 6a940bfd4..1cc02c73c 100644 --- a/libs/portal/xtream/feature/src/lib/actor/xtream-actor-route.component.ts +++ b/libs/portal/xtream/feature/src/lib/actor/xtream-actor-route.component.ts @@ -21,6 +21,7 @@ import { pickTitleMatch, } from '@iptvnator/services'; import { CatalogTitleMatch } from '@iptvnator/shared/interfaces'; +import { createLatestRequestGuard } from '@iptvnator/portal/shared/util'; import { ActorViewComponent, ActorViewItem, @@ -78,6 +79,7 @@ export class XtreamActorRouteComponent { readonly scope = signal('portal'); readonly isMatchingGlobal = signal(false); private readonly globalMatches = signal(null); + private readonly matchRequest = createLatestRequestGuard(); private readonly vodIndex = computed(() => buildCatalogTitleIndex(this.xtreamStore.vodStreams()) @@ -195,14 +197,21 @@ export class XtreamActorRouteComponent { // previous person must not overwrite the current one's results const requestedPersonId = this.personId(); const titles = this.filmography().map((credit) => credit.title); + const matchToken = this.matchRequest.start(); this.isMatchingGlobal.set(true); try { const matches = await this.titleMatch.matchTitles(titles); - if (this.personId() === requestedPersonId) { + if ( + this.matchRequest.isLatest(matchToken) && + this.personId() === requestedPersonId + ) { this.globalMatches.set(matches); } } finally { - if (this.personId() === requestedPersonId) { + // Only the newest request may clear the indicator. Keying this + // on the subject instead strands the spinner when the user + // leaves the scope and no replacement request ever runs. + if (this.matchRequest.isLatest(matchToken)) { this.isMatchingGlobal.set(false); } } diff --git a/libs/portal/xtream/feature/src/lib/discover/xtream-discover-route.component.ts b/libs/portal/xtream/feature/src/lib/discover/xtream-discover-route.component.ts index 5a09c2a7e..54221a42a 100644 --- a/libs/portal/xtream/feature/src/lib/discover/xtream-discover-route.component.ts +++ b/libs/portal/xtream/feature/src/lib/discover/xtream-discover-route.component.ts @@ -18,6 +18,7 @@ import { pickTitleMatch, } from '@iptvnator/services'; import { CatalogTitleMatch } from '@iptvnator/shared/interfaces'; +import { createLatestRequestGuard } from '@iptvnator/portal/shared/util'; import { DiscoverViewComponent, TitleResultsScope, @@ -84,6 +85,7 @@ export class XtreamDiscoverRouteComponent { readonly scope = signal('portal'); readonly isMatchingGlobal = signal(false); private readonly globalMatches = signal(null); + private readonly matchRequest = createLatestRequestGuard(); private readonly vodIndex = computed(() => buildCatalogTitleIndex(this.xtreamStore.vodStreams()) @@ -195,14 +197,21 @@ export class XtreamDiscoverRouteComponent { private async loadGlobalMatches(): Promise { const requestedKey = this.facetKey(); const titles = this.results().map((title) => title.title); + const matchToken = this.matchRequest.start(); this.isMatchingGlobal.set(true); try { const matches = await this.titleMatch.matchTitles(titles); - if (this.facetKey() === requestedKey) { + if ( + this.matchRequest.isLatest(matchToken) && + this.facetKey() === requestedKey + ) { this.globalMatches.set(matches); } } finally { - if (this.facetKey() === requestedKey) { + // Only the newest request may clear the indicator. Keying this + // on the subject instead strands the spinner when the user + // leaves the scope and no replacement request ever runs. + if (this.matchRequest.isLatest(matchToken)) { this.isMatchingGlobal.set(false); } } diff --git a/libs/portal/xtream/feature/src/lib/xtream-workspace-route-session.service.ts b/libs/portal/xtream/feature/src/lib/xtream-workspace-route-session.service.ts index 0b48352a7..dcd15b41b 100644 --- a/libs/portal/xtream/feature/src/lib/xtream-workspace-route-session.service.ts +++ b/libs/portal/xtream/feature/src/lib/xtream-workspace-route-session.service.ts @@ -56,13 +56,25 @@ function toXtreamPlaylistData( }; } -function isImportDrivenSection(section: PortalRailSection | null): boolean { +/** + * Sections whose content comes from the imported catalog, so a cold load + * of one has to initialize it first. + * + * The parameter is a raw URL segment, not a rail section: `actor` and + * `discover` have no rail entry, yet both read the catalog to decide + * whether a TMDB title is "In your library". Without initialization a + * direct load of either answers "no" for everything the user owns — + * a wrong claim rather than a missing one. + */ +function isImportDrivenSection(section: string | null): boolean { return ( section === 'vod' || section === 'live' || section === 'series' || section === 'search' || - section === 'recently-added' + section === 'recently-added' || + section === 'actor' || + section === 'discover' ); } diff --git a/libs/services/src/lib/tmdb/tmdb-merge-stalker.spec.ts b/libs/services/src/lib/tmdb/tmdb-merge-stalker.spec.ts index a13781ce1..3e8b9cc87 100644 --- a/libs/services/src/lib/tmdb/tmdb-merge-stalker.spec.ts +++ b/libs/services/src/lib/tmdb/tmdb-merge-stalker.spec.ts @@ -103,6 +103,7 @@ describe('mergeStalkerInfoWithTmdb', () => { providerStalkerInfo, { ...tmdbMovieRu, + origin_country: ['RU'], production_countries: [ { iso_3166_1: 'RU', name: 'Russia' }, ], diff --git a/libs/services/src/lib/tmdb/tmdb-merge.spec.ts b/libs/services/src/lib/tmdb/tmdb-merge.spec.ts index 83e6e5b61..9513e96fe 100644 --- a/libs/services/src/lib/tmdb/tmdb-merge.spec.ts +++ b/libs/services/src/lib/tmdb/tmdb-merge.spec.ts @@ -51,6 +51,7 @@ const tmdbMovie: TmdbMovieDetails = { poster_path: '/matrix-poster.jpg', backdrop_path: '/matrix-backdrop.jpg', production_countries: [{ iso_3166_1: 'US', name: 'United States' }], + origin_country: ['US'], credits: { cast: [ { @@ -247,6 +248,36 @@ describe('mergeVodInfoWithTmdb', () => { ]); }); + it('keeps country facets to the origin countries the filter can query', () => { + // A co-production lists every partner in production_countries, but + // Discover filters by origin — offering "Canada" here would return + // Canadian-origin films rather than ones Canada co-produced + const merged = mergeVodInfoWithTmdb(providerVodInfo(), { + id: 476299, + origin_country: ['FR'], + production_countries: [ + { iso_3166_1: 'CA', name: 'Canada' }, + { iso_3166_1: 'FR', name: 'France' }, + ], + }); + + expect(merged.tmdb_countries).toEqual([ + { code: 'FR', name: 'France' }, + ]); + }); + + it('drops an origin country TMDB does not name', () => { + const merged = mergeVodInfoWithTmdb(providerVodInfo(), { + id: 603, + origin_country: ['US', 'XX'], + production_countries: [{ iso_3166_1: 'US', name: 'United States' }], + }); + + expect(merged.tmdb_countries).toEqual([ + { code: 'US', name: 'United States' }, + ]); + }); + it('omits facet fields when TMDB states none', () => { const merged = mergeVodInfoWithTmdb(providerVodInfo(), { id: 603 }); @@ -261,6 +292,7 @@ describe('mergeVodInfoWithTmdb', () => { { id: 28, name: 'Action' }, { id: 12, name: ' ' }, ], + origin_country: ['US', 'ZZ'], production_countries: [ { iso_3166_1: 'US', name: 'United States' }, { iso_3166_1: '', name: 'Nowhere' }, @@ -361,6 +393,7 @@ describe('mergeSerieInfoWithTmdb', () => { it('emits structured genre and country facets for Discover chips', () => { const merged = mergeSerieInfoWithTmdb(providerSerieInfo, { ...tmdbTv, + origin_country: ['DE'], production_countries: [{ iso_3166_1: 'DE', name: 'Germany' }], }); diff --git a/libs/services/src/lib/tmdb/tmdb-merge.ts b/libs/services/src/lib/tmdb/tmdb-merge.ts index c94bf0dc1..aacd1be90 100644 --- a/libs/services/src/lib/tmdb/tmdb-merge.ts +++ b/libs/services/src/lib/tmdb/tmdb-merge.ts @@ -75,14 +75,25 @@ function genreFacets(details: TmdbDetails): TmdbGenreFacet[] { .map((genre) => ({ id: genre.id, name: genre.name })); } -/** Structured production countries for clickable Discover chips */ +/** + * Structured countries for clickable Discover chips. + * + * Built from `origin_country`, never from the fuller + * `production_countries`: the Discover page filters by + * `with_origin_country`, so a co-production's other countries would + * promise "titles from here" and deliver a different set. Names come + * from `production_countries` (the only place TMDB states them), and a + * code it does not name is dropped rather than rendered as a bare "FR". + */ function countryFacets(details: TmdbDetails): TmdbCountryFacet[] { - return (details.production_countries ?? []) - .filter((entry) => !!entry.iso_3166_1 && !!entry.name) - .map((entry) => ({ - code: entry.iso_3166_1 as string, - name: entry.name as string, - })); + const namesByCode = new Map( + (details.production_countries ?? []) + .filter((entry) => !!entry.iso_3166_1 && !!entry.name) + .map((entry) => [entry.iso_3166_1 as string, entry.name as string]) + ); + return (details.origin_country ?? []) + .filter((code) => namesByCode.has(code)) + .map((code) => ({ code, name: namesByCode.get(code) as string })); } function tmdbRating(details: TmdbDetails): number | null { diff --git a/libs/services/src/lib/tmdb/tmdb.types.ts b/libs/services/src/lib/tmdb/tmdb.types.ts index dd30dd45e..7fecc3389 100644 --- a/libs/services/src/lib/tmdb/tmdb.types.ts +++ b/libs/services/src/lib/tmdb/tmdb.types.ts @@ -103,6 +103,13 @@ interface TmdbDetailsBase { recommendations?: { results?: TmdbSearchResult[] }; /** Present on both /movie and /tv details payloads */ production_countries?: { iso_3166_1?: string; name?: string }[]; + /** + * ISO 3166-1 codes of the countries the title ORIGINATES from — a + * subset of `production_countries` for co-productions. This is the + * dimension `/discover`'s `with_origin_country` filters on, so it is + * what a clickable country facet must be built from. + */ + origin_country?: string[]; } export interface TmdbMovieDetails extends TmdbDetailsBase {