mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-13 04:26:16 -08:00
fix(portals): address review findings on the Discover pages
- 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
ae01203dd0
commit
0ac605fbe8
13 files changed
+209
-20
No files matched your search
@@ -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
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
export * from './lib/latest-request-guard';
|
||||
export * from './lib/logger';
|
||||
export * from './lib/category-sort';
|
||||
export * from './lib/channel-sort';
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
@@ -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<ActorViewScope>('portal');
|
||||
readonly isMatchingGlobal = signal(false);
|
||||
private readonly globalMatches = signal<CatalogTitleMatch[] | null>(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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<TitleResultsScope>('portal');
|
||||
readonly isMatchingGlobal = signal(false);
|
||||
private readonly globalMatches = signal<CatalogTitleMatch[] | null>(null);
|
||||
private readonly matchRequest = createLatestRequestGuard();
|
||||
private readonly globalIndex = computed(() =>
|
||||
groupTitleMatchesByKey(this.globalMatches() ?? [])
|
||||
);
|
||||
@@ -166,14 +168,21 @@ export class StalkerDiscoverRouteComponent {
|
||||
private async loadGlobalMatches(): Promise<void> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<ActorViewScope>('portal');
|
||||
readonly isMatchingGlobal = signal(false);
|
||||
private readonly globalMatches = signal<CatalogTitleMatch[] | null>(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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<TitleResultsScope>('portal');
|
||||
readonly isMatchingGlobal = signal(false);
|
||||
private readonly globalMatches = signal<CatalogTitleMatch[] | null>(null);
|
||||
private readonly matchRequest = createLatestRequestGuard();
|
||||
|
||||
private readonly vodIndex = computed(() =>
|
||||
buildCatalogTitleIndex(this.xtreamStore.vodStreams())
|
||||
@@ -195,14 +197,21 @@ export class XtreamDiscoverRouteComponent {
|
||||
private async loadGlobalMatches(): Promise<void> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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'
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -103,6 +103,7 @@ describe('mergeStalkerInfoWithTmdb', () => {
|
||||
providerStalkerInfo,
|
||||
{
|
||||
...tmdbMovieRu,
|
||||
origin_country: ['RU'],
|
||||
production_countries: [
|
||||
{ iso_3166_1: 'RU', name: 'Russia' },
|
||||
],
|
||||
|
||||
@@ -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' }],
|
||||
});
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in new issue
Block a user