From 10a42612d5d16d9274cb6fb5f9f02ec40f4311bf Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 28 Jul 2026 12:03:35 +0200 Subject: [PATCH] fix(portals): keep the primary button honest across navigation and pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four follow-ups from review, all consequences of splitting the position signals. - Route reuse (the Similar rail) cleared only `vodPlaybackPosition`, so the button kept the previous movie's Resume label — and start point — until the new lookup landed. Both signals and the playback latch now reset together. - The primary button's fall-through past an unresolvable pin reached the service directly, skipping the bookkeeping a route start needs: the controller kept the alternative's timecode and the old stream's timeupdate still vouched for the new one. It now goes through the route's own wrappers, and Resume seeds the controller with the ROUTE copy's position. - `alternativePlaylistCount` counted the playlist being watched whenever it held a second copy, so "also found in 2 other playlists" could mean one. - The pinned copy's stored row went stale the moment the user watched it; its live position now wins while it is the one playing. Co-Authored-By: Claude Opus 5 --- docs/architecture/vod-multi-source.md | 9 ++- .../vod-details-route-playback.spec.ts | 77 ++++++++++++++++++- .../vod-details-route.component.ts | 26 ++++++- .../vod-multi-source-counts.spec.ts | 76 ++++++++++++++++++ .../vod-details/vod-multi-source-counts.ts | 17 +++- .../vod-primary-action-position.spec.ts | 51 +++++++++++- .../vod-primary-action-position.ts | 25 ++++-- 7 files changed, 264 insertions(+), 17 deletions(-) create mode 100644 libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.spec.ts diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index d7c138929..75d3a2ae5 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -430,7 +430,14 @@ seen — whichever copy produced it — and feeds the progress bar and the switc handoff. `routePlaybackPosition` is the route copy's own row, and everything that acts on the route's stream reads that: Resume, its label, its timecode. Collapsing them lets an alternative's progress resume the route copy at a -timecode nobody reached in it. +timecode nobody reached in it. Route reuse (the Similar rail) clears +both, so the button cannot describe the previous movie while the new lookup is +still in flight, and every route start — including the primary button's +fall-through past an unresolvable pin — goes through the route's own wrappers, +which replace whatever timecode an alternative left in the controller. + +While the pinned copy is the one playing, its live position wins over the row +that was stored before this session started. A pin means the button plays a copy the page did not load a position for. `createPrimaryActionPosition` therefore looks that copy's row up and lets it diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts index 02f96c8d9..b6faadfb9 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts @@ -201,6 +201,77 @@ describe('VodDetailsRouteComponent — playback actions', () => { expect(reported).toHaveBeenLastCalledWith(12); }); + it('drops the previous movie’s resume point on a reused route', () => { + currentPlaylist.set({ id: 'playlist-1' }); + const playback = fixture.debugElement.injector.get( + VodDetailsPlaybackService + ); + fixture.detectChanges(); + playback.routePlaybackPosition.set({ + playlistId: 'playlist-1', + contentXtreamId: 650020, + contentType: 'vod', + positionSeconds: 2538, + durationSeconds: 7744, + }); + + // The Similar rail reuses this component. Until the new lookup lands + // the button would offer the PREVIOUS movie's "Resume 42:18" — and + // start the new stream there. + currentPlaylist.set({ id: 'playlist-2' }); + fixture.detectChanges(); + + expect(playback.routePlaybackPosition()).toBeNull(); + expect(playback.hasPlaybackPosition()).toBe(false); + }); + + it('replaces an alternative’s timecode when Resume starts the route copy', async () => { + currentPlaylist.set({ id: 'playlist-1' }); + const component = fixture.componentInstance; + const playback = fixture.debugElement.injector.get( + VodDetailsPlaybackService + ); + withActiveSource('playlist-1', 650020); + playback.routePlaybackPosition.set({ + playlistId: 'playlist-1', + contentXtreamId: 650020, + contentType: 'vod', + positionSeconds: 120, + durationSeconds: 7744, + }); + const reported = jest.spyOn(component.multiSource, 'reportPosition'); + + // An alternative left its own timecode in the controller. Resuming the + // ROUTE copy has to overwrite it, or a failure before the first + // timeupdate resolves the next source at a position from another copy. + component.multiSource.reportPosition(4200); + component.resumeVod({ + movie_data: { stream_id: 650020, name: 'Example' }, + } as never); + + expect(reported).toHaveBeenLastCalledWith(120); + }); + + it('takes the route wrappers when the primary button falls through', () => { + currentPlaylist.set({ id: 'playlist-1' }); + const component = fixture.componentInstance; + withActiveSource('playlist-1', 650020); + jest.spyOn(component.multiSource, 'playPinnedSource').mockResolvedValue( + false + ); + const reported = jest.spyOn(component.multiSource, 'reportPosition'); + + // Reaching the service directly would skip the bookkeeping a route + // start needs — the controller would keep an alternative's timecode. + void component.onPrimaryAction({ + movie_data: { stream_id: 650020, name: 'Example' }, + } as never); + + return Promise.resolve().then(() => { + expect(reported).toHaveBeenCalledWith(0); + }); + }); + describe('auto-failover toggle', () => { beforeEach(() => { selectedPlayer.set(VideoPlayer.Html5Player); @@ -215,9 +286,9 @@ describe('VodDetailsRouteComponent — playback actions', () => { // calls onPlaybackFailed(), so the switch could never happen. selectedPlayer.set(player); - expect( - fixture.componentInstance.autoFailoverSupported() - ).toBe(false); + expect(fixture.componentInstance.autoFailoverSupported()).toBe( + false + ); } ); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts index aa09e489f..67707cb5d 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts @@ -245,6 +245,7 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { routePlaylistId: computed(() => this.xtreamStore.currentPlaylist()?.id), routeContentId: this.selectedVodId, routePosition: this.playback.routePlaybackPosition, + livePosition: this.playback.vodPlaybackPosition, load: (source) => this.positionFor(source), }); readonly hasPlaybackPosition = this.primaryAction.hasPosition; @@ -409,7 +410,13 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { this.lastInitKey.set(initKey); this.inlinePlayback.set(null); + // Both, or the primary button keeps the previous movie's Resume + // label until the new lookup lands — and starts the new stream + // there. `loadPosition` is guarded on the same key, so an older + // lookup cannot repopulate either one. this.vodPlaybackPosition.set(null); + this.playback.routePlaybackPosition.set(null); + this.inlineTimeSeen.set(false); this.initializeVodDetails(playlistId, vodId); }); @@ -494,6 +501,12 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { this.multiSource.markRouteSourceActive(); this.playbackFailed.set(false); this.inlineTimeSeen.set(false); + // The controller can still hold an ALTERNATIVE's timecode. A failure + // before the first timeupdate would otherwise resolve the next source + // at a position that belongs to a different copy. + this.multiSource.reportPosition( + this.playback.routePlaybackPosition()?.positionSeconds ?? 0 + ); this.playback.resumeVod(vodItem); } @@ -513,10 +526,15 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { return; } - // Falling through to the route's own stream: whatever alternative was - // last active is not what will be playing. - this.multiSource.markRouteSourceActive(); - this.playback.onPrimaryAction(vodItem); + // Through the route's OWN wrappers, not the service's: they carry the + // bookkeeping a route start needs — clearing the playback evidence and + // replacing whatever timecode an alternative left in the controller. + if (this.playback.hasPlaybackPosition()) { + this.resumeVod(vodItem); + return; + } + + this.playVod(vodItem); } /** diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.spec.ts new file mode 100644 index 000000000..9bfc2047a --- /dev/null +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.spec.ts @@ -0,0 +1,76 @@ +import { signal } from '@angular/core'; +import type { VodSourceDescriptor } from '@iptvnator/shared/interfaces'; +import { createVodSourceCounts } from './vod-multi-source-counts'; + +function source( + id: string, + playlistId: string, + isActive = false +): VodSourceDescriptor { + return { + id, + playlistId, + playlistName: playlistId, + portalType: 'xtream', + contentId: 1, + rawTitle: 'Example', + matchConfidence: 'exact', + year: null, + isActive, + isPinned: false, + isTried: false, + probe: { status: 'idle' }, + } as VodSourceDescriptor; +} + +describe('createVodSourceCounts', () => { + it('counts alternative streams for the chip', () => { + const counts = createVodSourceCounts( + signal([ + source('a', 'playlist-1', true), + source('b', 'playlist-2'), + source('c', 'playlist-2'), + ]) + ); + + expect(counts.alternativeCount()).toBe(2); + expect(counts.hasAlternatives()).toBe(true); + }); + + it('counts OTHER playlists for the caption', () => { + // The caption says "also found in N other playlists", so a playlist's + // two copies are one playlist. + const counts = createVodSourceCounts( + signal([ + source('a', 'playlist-1', true), + source('b', 'playlist-2'), + source('c', 'playlist-2'), + ]) + ); + + expect(counts.alternativePlaylistCount()).toBe(1); + }); + + it('does not count the playlist being watched as another', () => { + // Playing one of playlist-2's two copies leaves the other listed as an + // alternative, which put playlist-2's own id in the set. + const counts = createVodSourceCounts( + signal([ + source('a', 'playlist-1'), + source('b', 'playlist-2', true), + source('c', 'playlist-2'), + ]) + ); + + expect(counts.alternativePlaylistCount()).toBe(1); + }); + + it('has nothing to offer for a single source', () => { + const counts = createVodSourceCounts( + signal([source('a', 'playlist-1', true)]) + ); + + expect(counts.hasAlternatives()).toBe(false); + expect(counts.alternativePlaylistCount()).toBe(0); + }); +}); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.ts index c95ae76ed..a49f0a4bc 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-counts.ts @@ -29,8 +29,19 @@ export function createVodSourceCounts( alternatives, hasAlternatives: computed(() => alternatives().length > 0), alternativeCount: computed(() => alternatives().length), - alternativePlaylistCount: computed( - () => new Set(alternatives().map((s) => s.playlistId)).size - ), + alternativePlaylistCount: computed(() => { + const playlists = new Set( + alternatives().map((source) => source.playlistId) + ); + // A playlist holding two copies puts its own id in that set via + // the copy that is not playing, and the caption would then count + // the playlist the user is already watching as an "other". + const active = sources().find((source) => source.isActive); + if (active) { + playlists.delete(active.playlistId); + } + + return playlists.size; + }), }; } diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.spec.ts index 7319050bc..b577a2767 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.spec.ts @@ -64,16 +64,18 @@ function setup( load: (source: VodSourceDescriptor) => Promise ) { const sourcesSignal = signal(sources); + const livePosition = signal(null); const api = TestBed.runInInjectionContext(() => createPrimaryActionPosition({ sources: sourcesSignal, routePlaylistId: signal(ROUTE_PLAYLIST), routeContentId: signal(ROUTE_CONTENT), routePosition: signal(routePosition), + livePosition, load, }) ); - return { api, sourcesSignal }; + return { api, sourcesSignal, livePosition }; } describe('createPrimaryActionPosition', () => { @@ -116,6 +118,53 @@ describe('createPrimaryActionPosition', () => { expect(api.hasPosition()).toBe(false); }); + it('follows the pinned copy while it is being watched', async () => { + const { api, livePosition } = setup( + [source(), ALT], + position(2538), + async () => position(600) + ); + TestBed.tick(); + await Promise.resolve(); + TestBed.tick(); + expect(api.position()?.positionSeconds).toBe(600); + + // Playing the pinned copy leaves its stored row behind; the button + // would keep offering the timecode it had before this session. + livePosition.set({ + playlistId: ALT.playlistId, + contentXtreamId: ALT.contentId, + contentType: 'vod', + positionSeconds: 3000, + durationSeconds: 7200, + }); + TestBed.tick(); + + expect(api.position()?.positionSeconds).toBe(3000); + }); + + it('ignores live progress from a copy the button will not play', async () => { + const { api, livePosition } = setup( + [source(), ALT], + position(2538), + async () => position(600) + ); + TestBed.tick(); + await Promise.resolve(); + TestBed.tick(); + + livePosition.set({ + playlistId: 'playlist-9', + contentXtreamId: 5, + contentType: 'vod', + positionSeconds: 3000, + durationSeconds: 7200, + }); + TestBed.tick(); + + expect(api.position()?.positionSeconds).toBe(600); + }); + it('ignores a pin on the route’s own copy', () => { const load = jest.fn(); const { api } = setup( diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.ts index ed212b65b..2dc63c62a 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-primary-action-position.ts @@ -26,6 +26,12 @@ export interface PrimaryActionPositionDeps { routeContentId: Signal; /** The route copy's position, as loaded by the page. */ routePosition: Signal; + /** + * The last position seen from any copy. The pinned copy's stored row goes + * stale the moment the user watches it, and re-reading the database on + * every update would be a round-trip per timeupdate. + */ + livePosition: Signal; load: (source: VodSourceDescriptor) => Promise; } @@ -80,11 +86,20 @@ export function createPrimaryActionPosition( }); }); - const position = computed(() => - foreignPin() && pinnedLoadedFor() - ? pinnedPosition() - : deps.routePosition() - ); + const position = computed(() => { + const pinned = foreignPin(); + if (!pinned || !pinnedLoadedFor()) { + return deps.routePosition(); + } + + // What just played beats what was stored before it did. + const live = deps.livePosition(); + const isPinnedCopy = + live?.playlistId === pinned.playlistId && + live?.contentXtreamId === pinned.contentId; + + return isPinnedCopy ? live : pinnedPosition(); + }); const hasPosition = computed(() => { const progress = getPortalPlaybackProgressPercent(position());