fix(portals): stop a remake matching, and let a pin survive its own playlist

Three findings from the latest review pass. `normalizeTitleKeys` strips bracketed segments as tag noise, so "Dune (1984)" normalizes to exactly "dune" — an EXACT match for the 2021 film, ranked above every fuzzy one, with the year never consulted because that tier skipped the gate. Auto-failover could switch the user to the other film entirely. The year is now read out of brackets too, and a stated disagreement rejects the row on either tier. Playback positions are keyed by (playlist, stream), so watching through a pinned alternative stores progress under ITS ids while the page loads the route copy's row. Starting the pin therefore resumed from a position belonging to a different copy — usually zero. It now loads its own. And a pin can point at another copy of the film inside the playlist being viewed, which discovery excludes wholesale: the pinned row was absent from the list, so nothing showed as pinned and Play ignored the preference. The pin is now read before discovery, which keeps that one row. Moves the pin-shaped decisions into the pin module, where the persistence helpers already live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-12 12:06:16 -08:00 · 2026-07-28 02:28:26 +02:00
1 parent ed442da3ca
commit f0c99fac87
@@ -22,6 +22,12 @@ export interface VodSourceDiscoveryRequest {
    year?: number | null;
    /** The playlist being viewed; never offered back as an alternative. */
    currentPlaylistId: string;
+    /**
+     * One stream id inside it to keep anyway — a pin can point at another copy
+     * of the film in the playlist the user is currently on, and excluding it
+     * would silently ignore an explicit preference.
+     */
+    keepContentId?: number | null;
 }

 export interface VodSourceDiscoveryResult {
@@ -61,6 +67,7 @@ export class VodSourceDiscoveryService {
                title: request.title,
                year: request.year ?? null,
                excludePlaylistId: request.currentPlaylistId,
+                keepContentId: request.keepContentId ?? null,
            });

            return {
@@ -23,7 +23,10 @@ import {
    VodSourcesChipComponent,
 } from '@iptvnator/ui/components';
 import { SafePipe } from '@iptvnator/pipes';
-import { createLogger } from '@iptvnator/portal/shared/util';
+import {
+    createLogger,
+    PORTAL_PLAYBACK_POSITIONS,
+} from '@iptvnator/portal/shared/util';
 import { XtreamStore } from '@iptvnator/portal/xtream/data-access';
 import {
    type PlaybackFallbackRequest,
@@ -45,6 +48,7 @@ import {
    XtreamVodInfo,
    XtreamVodStream,
    youtubeEmbedUrl,
+    type VodSourceCandidate,
 } from '@iptvnator/shared/interfaces';
 import {
    SimilarCatalogItem,
@@ -90,6 +94,7 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy {
    private readonly snackBar = inject(MatSnackBar);
    private readonly translateService = inject(TranslateService);
    private readonly playback = inject(VodDetailsPlaybackService);
+    private readonly playbackPositions = inject(PORTAL_PLAYBACK_POSITIONS);
    /** Alternative sources for this movie in the user's other playlists */
    readonly multiSource = inject(VodMultiSourceHostService);
    private readonly logger = createLogger('VodDetailsRoute');
@@ -461,13 +466,30 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy {
        // A pinned source is an explicit "play this movie from here", so it
        // outranks the playlist the route happens to be on. Falls through to
        // the normal path when nothing is pinned or the pin cannot resolve.
-        if (await this.multiSource.playPinnedSource()) {
+        if (await this.multiSource.playPinnedSource(this.resumeSecondsFor)) {
            return;
        }

        this.playback.onPrimaryAction(vodItem);
    }

+    /**
+     * Where a specific source was last watched.
+     *
+     * Positions are keyed by (playlist, stream), so a pinned alternative has
+     * its own row — the one this page loaded belongs to the route's copy.
+     */
+    private readonly resumeSecondsFor = async (
+        source: VodSourceCandidate
+    ): Promise<number | null> => {
+        const position = await this.playbackPositions.getPlaybackPosition(
+            source.playlistId,
+            source.contentId,
+            'vod'
+        );
+        return position?.positionSeconds ?? null;
+    };
+
    stopExternalPlayback(): Promise<void> {
        return this.playback.stopExternalPlayback();
    }
@@ -125,6 +125,87 @@ describe('VodMultiSourceHostService — pinning', () => {
        );
    });

+    it('asks discovery to keep a copy pinned inside the current playlist', async () => {
+        // The pin was set on another copy of the film in the playlist the user
+        // is now opening. Excluding that playlist wholesale would leave the pin
+        // pointing at a row that is not in the list, so Play would ignore it.
+        pins.get.mockResolvedValue({
+            matchKey: 'title:the matrix:1999',
+            playlistId: MOVIE_A.playlistId,
+            contentId: 4242,
+            portalType: 'xtream',
+        });
+
+        await loadMovie([]);
+
+        expect(discovery.discover).toHaveBeenCalledWith(
+            expect.objectContaining({
+                currentPlaylistId: MOVIE_A.playlistId,
+                keepContentId: 4242,
+            })
+        );
+    });
+
+    it('asks for no exception when the pin is in another playlist', async () => {
+        pins.get.mockResolvedValue({
+            matchKey: 'title:the matrix:1999',
+            playlistId: ALT_TWO.playlistId,
+            contentId: ALT_TWO.contentId,
+            portalType: 'xtream',
+        });
+
+        await loadMovie([ALT_TWO]);
+
+        expect(discovery.discover).toHaveBeenCalledWith(
+            expect.objectContaining({ keepContentId: null })
+        );
+    });
+
+    it('resumes a pinned alternative from its OWN stored position', async () => {
+        pins.get.mockResolvedValue({
+            matchKey: 'title:the matrix:1999',
+            playlistId: ALT_TWO.playlistId,
+            contentId: ALT_TWO.contentId,
+            portalType: 'xtream',
+        });
+        await loadMovie([ALT_TWO]);
+
+        // What the page loaded belongs to the ROUTE's copy — a different row
+        // entirely from the one the user has been watching through the pin.
+        service.seedResumePosition(12);
+
+        const resumeFor = jest.fn().mockResolvedValue(3600);
+        await expect(service.playPinnedSource(resumeFor)).resolves.toBe(true);
+
+        expect(resumeFor).toHaveBeenCalledWith(
+            expect.objectContaining({ id: ALT_TWO.id })
+        );
+        expect(resolver.resolve).toHaveBeenCalledWith(
+            expect.objectContaining({ id: ALT_TWO.id }),
+            { startTime: 3600 }
+        );
+    });
+
+    it('keeps the page position when the pinned source has none', async () => {
+        pins.get.mockResolvedValue({
+            matchKey: 'title:the matrix:1999',
+            playlistId: ALT_TWO.playlistId,
+            contentId: ALT_TWO.contentId,
+            portalType: 'xtream',
+        });
+        await loadMovie([ALT_TWO]);
+        service.seedResumePosition(2538);
+
+        await expect(
+            service.playPinnedSource(jest.fn().mockResolvedValue(null))
+        ).resolves.toBe(true);
+
+        expect(resolver.resolve).toHaveBeenCalledWith(
+            expect.objectContaining({ id: ALT_TWO.id }),
+            { startTime: 2538 }
+        );
+    });
+
    it('retires every stale alias and writes only the canonical key', async () => {
        await loadMovie([ALT_TWO]);
        const matchKeys: string[] = pins.get.mock.calls[0][0];
@@ -186,6 +186,9 @@ describe('VodMultiSourceHostService — stale resolutions', () => {
        vodAutoFailover.set(true);

        const loading = service.load(MOVIE_A);
+        while (discovery.discover.mock.calls.length === 0) {
+            await Promise.resolve();
+        }

        // The stream died faster than the database answered. Concluding
        // "nowhere to go" here would strand the user on the error screen with
@@ -207,6 +210,11 @@ describe('VodMultiSourceHostService — stale resolutions', () => {
        vodAutoFailover.set(true);

        const loadingA = service.load(MOVIE_A);
+        // The pin lookup runs first now, so wait until A is genuinely inside
+        // discovery before the failure arrives.
+        while (discovery.discover.mock.calls.length === 0) {
+            await Promise.resolve();
+        }
        const failingOver = service.failover();

        // The user navigates while A's discovery is still out. Running against
@@ -107,6 +107,7 @@ describe('VodMultiSourceHostService — session lifecycle', () => {
            title: 'The Matrix',
            year: 1999,
            currentPlaylistId: 'playlist-1',
+            keepContentId: null,
        });

        movie.set({ ...MOVIE_A });
@@ -187,25 +188,21 @@ describe('VodMultiSourceHostService — session lifecycle', () => {

    it('discards a discovery that resolves after the movie changed', async () => {
        const first = createDeferred<DiscoveryResult>();
-        const second = createDeferred<DiscoveryResult>();
-        discovery.discover
-            .mockReturnValueOnce(first.promise)
-            .mockReturnValueOnce(second.promise);
+        discovery.discover.mockReturnValueOnce(first.promise);

        const loadA = service.load(MOVIE_A);
-        const loadB = service.load(MOVIE_B);
+        // A's pin lookup runs before its discovery, so wait for the call that
+        // actually matters rather than guessing at microtask counts.
+        while (discovery.discover.mock.calls.length === 0) {
+            await Promise.resolve();
+        }

+        const loadB = loadMovie([ALT_THREE], MOVIE_B);
        first.resolve({ sources: [ALT_TWO], matchKind: 'title-year' });
        await loadA;
-
-        expect(service.sources()).toEqual([]);
-        // Abandoned on arrival: it never even looks the stale pin up.
-        expect(pins.get).not.toHaveBeenCalled();
-
-        second.resolve({ sources: [ALT_THREE], matchKind: 'title-year' });
        await loadB;

-        expect(pins.get).toHaveBeenCalledTimes(1);
+        // A's alternatives must not appear anywhere in B's session.
        expect(service.sources().map((source) => source.id)).toEqual([
            'playlist-1:xtream:202',
            ALT_THREE.id,
@@ -31,7 +31,10 @@ import type { VodMultiSourceSwitchNotice } from './vod-multi-source-notice';
 import { currentSourceRow } from './vod-multi-source-current-row';
 import { probeSource } from './vod-multi-source-probe';
 import {
+    pinnedCopyInPlaylist,
+    pinnedSourceAwaitingPlay,
    pinnedSourceIdOf,
+    playPinned,
    readPin,
    togglePinnedSource,
 } from './vod-multi-source-pin';
@@ -75,10 +78,7 @@ export class VodMultiSourceHostService {
    private discoveryToken = 0;
    /**
     * Bumped when the FILM changes, and only then — a rediscovery of the same
-     * film (enrichment described it better) must not cancel a switch or probe
-     * in flight for it. Without this guard the continuation would activate the
-     * old film's source in the new session and restart it from that session's
-     * zero resume position.
+     * film must not cancel a switch or probe in flight for it.
     */
    private sessionToken = 0;
    /**
@@ -132,8 +132,7 @@ export class VodMultiSourceHostService {
        () => this.settingsStore.vodAutoFailover?.() === true
    );

-    /** Live getter, not a snapshot: `isAvailable` is itself a getter over
-     * the Electron bridge, so copying it once could disagree with it. */
+    /** Live getter: the bridge's own `isAvailable` can change under us. */
    get isAvailable(): boolean {
        return this.discovery.isAvailable;
    }
@@ -176,11 +175,9 @@ export class VodMultiSourceHostService {
     * the tried-source set that makes failover terminate.
     *
     * The SAME film arriving again is a refresh, not a new session: enrichment
-     * supplies its year and TMDB id after the page opened, which changes the
-     * movie key on purpose (discovery gets the year, a `tmdb:` pin becomes
-     * findable) but changes nothing about what is playing. Resetting there
-     * would take the film back off the source the user switched to and hand
-     * failover a clean tried-set for sources it has already burned.
+     * changes the movie key on purpose, but nothing about what is playing.
+     * Resetting there would take the film off the source the user switched to
+     * and hand failover a clean tried-set for sources it has already burned.
     */
    async load(movie: VodMultiSourceMovie): Promise<void> {
        const finished = this.discover(movie);
@@ -214,10 +211,17 @@ export class VodMultiSourceHostService {
            return;
        }

+        // Read FIRST, because discovery has to be told which copy to keep.
+        const pin = await readPin(this.pins, this.matchKeys);
+        if (token !== this.discoveryToken) {
+            return;
+        }
+
        const result = await this.discovery.discover({
            title: movie.title,
            year: movie.year,
            currentPlaylistId: movie.playlistId,
+            keepContentId: pinnedCopyInPlaylist(pin, movie.playlistId),
        });
        if (token !== this.discoveryToken) {
            return;
@@ -229,11 +233,6 @@ export class VodMultiSourceHostService {
            result.sources,
            result.matchKind
        );
-
-        const pin = await readPin(this.pins, this.matchKeys);
-        if (token !== this.discoveryToken) {
-            return;
-        }
        if (pin) {
            this.controller.setPinnedSource(pinnedSourceIdOf(pin));
        }
@@ -241,32 +240,30 @@ export class VodMultiSourceHostService {
        this.publish();
    }

-    /**
-     * The pinned source, when it is not the one the route already plays.
-     *
-     * "Make this the main source" has to survive reopening the movie, or the
-     * persisted preference is just an icon. The host consults this before its
-     * normal Play so the pin decides where playback starts.
-     */
-    readonly pendingPinnedSourceId = computed(() => {
-        const pinned = this._sources().find((source) => source.isPinned);
-        return pinned && !pinned.isActive ? pinned.id : null;
-    });
+    /** The pinned source, when it is not the one the route already plays. */
+    readonly pendingPinnedSourceId = computed(() =>
+        pinnedSourceAwaitingPlay(this._sources())
+    );

    /**
     * Start from the pinned source if there is one. Returns false when there is
     * nothing pinned to honour, leaving the caller's own Play path in charge.
     */
-    async playPinnedSource(): Promise<boolean> {
-        // The pin arrives with discovery. Concluding "nothing is pinned"
-        // before the lookup returns would start the route's own source and
-        // make the persisted preference a coin toss on worker latency.
+    async playPinnedSource(
+        resumeFor?: (source: VodSourceCandidate) => Promise<number | null>
+    ): Promise<boolean> {
+        // The pin arrives with discovery; answering before the lookup lands
+        // would make a persisted preference lose to worker latency.
        if (!(await this.stillOwnsScreen())) {
            return false;
        }

-        const pinnedId = this.pendingPinnedSourceId();
-        return pinnedId ? this.play(pinnedId) : false;
+        return playPinned({
+            controller: this.controller,
+            pinnedSourceId: this.pendingPinnedSourceId(),
+            resumeFor,
+            play: (sourceId) => this.play(sourceId),
+        });
    }

    /**
@@ -1,7 +1,9 @@
+import type { VodMultiSourceController } from '@iptvnator/portal/shared/data-access';
 import type { VodSourcePinService } from '@iptvnator/services';
 import {
    buildVodSourceMatchKey,
    type VodSourceCandidate,
+    type VodSourceDescriptor,
    type VodSourcePin,
 } from '@iptvnator/shared/interfaces';

@@ -13,6 +15,34 @@ import {
 * host needs to know how a match key is chosen.
 */

+/**
+ * The one copy inside the playlist being viewed that discovery must keep.
+ *
+ * A pin can point at another copy of the film in that same playlist, and
+ * excluding the playlist wholesale would drop the pinned row from the list —
+ * leaving the preference pointing at nothing and silently ignored.
+ */
+export function pinnedCopyInPlaylist(
+    pin: VodSourcePin | null,
+    playlistId: string
+): number | null {
+    return pin && pin.playlistId === playlistId ? pin.contentId : null;
+}
+
+/**
+ * The pinned row, when it is not the one already playing.
+ *
+ * "Make this the main source" has to survive reopening the movie, or the
+ * persisted preference is just an icon. The host consults this before its
+ * normal Play, so the pin decides where playback starts.
+ */
+export function pinnedSourceAwaitingPlay(
+    sources: readonly VodSourceDescriptor[]
+): string | null {
+    const pinned = sources.find((source) => source.isPinned);
+    return pinned && !pinned.isActive ? pinned.id : null;
+}
+
 /** The row id the controller uses, derived from a stored pin. */
 export function pinnedSourceIdOf(pin: VodSourcePin): string {
    return `${pin.playlistId}:${pin.portalType}:${pin.contentId}`;
@@ -103,3 +133,35 @@ export async function togglePinnedSource(
        ? candidate.id
        : undefined;
 }
+
+/** What starting the pinned source needs from the host. */
+export interface PinnedPlayDeps {
+    controller: VodMultiSourceController;
+    pinnedSourceId: string | null;
+    /** Where THAT source was last watched, when the host can look it up. */
+    resumeFor?: (source: VodSourceCandidate) => Promise<number | null>;
+    play: (sourceId: string) => Promise<boolean>;
+}
+
+/**
+ * Start the movie from its pinned source, at the position that source was
+ * last left at.
+ *
+ * Playback positions are keyed by (playlist, stream), so watching through a
+ * pinned alternative stores progress under ITS ids. The page loaded the ROUTE
+ * row, which for this source is stale or missing entirely — resuming from it
+ * would restart the film or jump to where a different copy was left.
+ */
+export async function playPinned(deps: PinnedPlayDeps): Promise<boolean> {
+    const pinned = deps.controller.findSource(deps.pinnedSourceId ?? '');
+    if (!pinned) {
+        return false;
+    }
+
+    const stored = await deps.resumeFor?.(pinned);
+    if (stored !== null && stored !== undefined) {
+        deps.controller.setResumeSeconds(stored);
+    }
+
+    return deps.play(pinned.id);
+}
@@ -875,6 +875,8 @@ export interface ElectronBridgeApi {
        title: string;
        year?: number | null;
        excludePlaylistId?: string | null;
+        /** A stream id inside the excluded playlist to keep anyway (a pin). */
+        keepContentId?: number | null;
    }) => Promise<VodSourceCandidateRow[]>;
    /** Per-movie pinned source; keys are passed most-trusted first */
    dbGetVodSourcePin: (matchKeys: string[]) => Promise<VodSourcePin | null>;