fix(portals): keep a pin readable under every identity of its film

Two defects in the pin/position subsystem, both reported in review.

A pin was stored under the movie's most-trusted key alone and its other
keys retired. But a movie's identity GROWS: the film keyed `tmdb:438631`
today was `title:dune:2021` before enrichment, and reopening it cold asks
for the poorer key first. The preference was therefore ignored until
enrichment landed — and permanently when enrichment is off or never
answers. The decision is now written under every key in `write` (never
the yearless form, which every remake shares), one upsert per key plus
the leftover retirement in the same transaction. `setVodSourcePin` also
reports failure for a pin with no usable key instead of claiming a write
it never made.

The primary button asked whether the pinned copy's position had loaded
by testing presence rather than identity, so re-pinning left it wearing
the previous copy's timecode until the new lookup returned.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-07-29 19:32:14 +02:00
1 parent e0ebbeafed
commit 45fca81504
16 files changed
+326 -78

No files matched your search

@@ -18,6 +18,7 @@ import {
ALT_TWO,
CURRENT_A_ID,
MOVIE_A,
MOVIE_B,
PROBE_OK,
createDeferred,
resolveWith,
@@ -129,9 +130,9 @@ describe('VodMultiSourceHostService — pin persistence', () => {
expect(rowFor(ALT_TWO.id)?.isPinned).toBe(false);
});
it('retires its own aliases but never another remake’s', async () => {
it('stores itself under its own keys, never under a remake’s', async () => {
// With a TMDB id there are two keys naming this film and one shared
// with every remake, so the retire set is worth asserting on.
// with every remake, so both lists are worth asserting on.
await loadMovie([ALT_TWO], { ...MOVIE_A, tmdbId: 603 });
expect(pins.get.mock.calls[0][0]).toEqual([
'tmdb:603',
@@ -143,20 +144,79 @@ describe('VodMultiSourceHostService — pin persistence', () => {
expect(pins.set).toHaveBeenCalledWith(
expect.objectContaining({ matchKey: 'tmdb:603' }),
expect.any(Array),
expect.any(Array)
);
// The aliases ride along with the write, so the two cannot half-apply.
const [, retired] = pins.set.mock.calls[0] as [unknown, string[]];
// This film's other key goes, so a reopen before enrichment cannot
// read a row still pointing at the source just replaced.
expect(retired).toContain('title:the matrix:1999');
// Both lists ride along with the write, so none of it can half-apply.
const [, retired, aliases] = pins.set.mock.calls[0] as [
unknown,
string[],
string[],
];
// This film's pre-enrichment key takes the SAME decision instead of
// being deleted: a reopen that has only a title and a year still finds
// the pin, and finds this source rather than the one just replaced.
expect(aliases).toContain('title:the matrix:1999');
expect(retired).not.toContain('title:the matrix:1999');
// The yearless form is shared by every remake: a Dune (2021) pin must
// not delete — or answer for — a row that may be Dune (1984)'s.
// not answer for — or delete — a row that may be Dune (1984)'s.
expect(aliases).not.toContain('title:the matrix:');
expect(retired).not.toContain('title:the matrix:');
// And never the row just written.
expect(retired).not.toContain('tmdb:603');
});
it('survives a reopen that has not been enriched yet', async () => {
// The whole point of the pin is that it outlives the page. Enrichment
// is asynchronous and optional, so the movie that owned a `tmdb:` key
// when the user pinned it opens again as nothing but a title and a
// year — and asks for the pin under THAT identity.
const rows = new Map<string, unknown>();
pins.set.mockImplementation(
async (
pin: { matchKey: string },
retire: string[] = [],
aliases: string[] = []
) => {
for (const key of [pin.matchKey, ...aliases]) {
rows.set(key, { ...pin, matchKey: key });
}
for (const key of retire) {
rows.delete(key);
}
return true;
}
);
pins.get.mockImplementation(async (keys: string[]) => {
for (const key of keys) {
if (rows.has(key)) {
return rows.get(key);
}
}
return null;
});
await loadMovie([ALT_TWO, ALT_THREE], { ...MOVIE_A, tmdbId: 603 });
await service.togglePin(ALT_THREE.id);
expect(rowFor(ALT_THREE.id)?.isPinned).toBe(true);
// Away to another film and back. That is what makes this a REOPEN:
// the session key is (playlist, content), so a different movie is what
// drops the in-memory controller and forces the pin to be re-read from
// storage — the same thing reopening the page does.
await loadMovie([], MOVIE_B);
await loadMovie([ALT_TWO, ALT_THREE], MOVIE_A);
// MOVIE_A carries no TMDB id, so this lookup asks only for the title
// forms. Stored under the enriched key alone the preference would be
// invisible here, and Play would quietly start the route source.
expect(pins.get).toHaveBeenLastCalledWith([
'title:the matrix:1999',
'title:the matrix:',
]);
expect(rowFor(ALT_THREE.id)?.isPinned).toBe(true);
});
it('keeps the stored pin when the replacement write fails', async () => {
await loadMovie([ALT_TWO], { ...MOVIE_A, tmdbId: 603 });
pins.set.mockResolvedValue(false);
@@ -208,8 +268,12 @@ describe('VodMultiSourceHostService — pin persistence', () => {
// half-outcome had an honest answer, so there is only one call now.
expect(pins.set).toHaveBeenCalledTimes(1);
expect(pins.clear).not.toHaveBeenCalled();
const [, retired] = pins.set.mock.calls[0] as [unknown, string[]];
expect(retired).toContain('title:the matrix:1999');
const [, , aliases] = pins.set.mock.calls[0] as [
unknown,
string[],
string[],
];
expect(aliases).toContain('title:the matrix:1999');
});
it('keeps the pin when clearing it fails', async () => {
@@ -322,8 +322,10 @@ describe('VodMultiSourceHostService — pinning', () => {
await service.togglePin(ALT_TWO.id);
// The aliases to retire travel with the write, in one transaction.
expect(pins.set).toHaveBeenCalledWith(pin, []);
// Both key lists travel with the write, in one transaction. This film
// has no TMDB id, so its title key IS the canonical one and there is
// nothing further to store it under or retire.
expect(pins.set).toHaveBeenCalledWith(pin, [], []);
expect(rowFor(ALT_TWO.id)?.isPinned).toBe(true);
await service.togglePin(ALT_TWO.id);
@@ -68,28 +68,32 @@ export async function readPin(
}
/**
* Persist the pin for `candidate` under the movie's most-trusted key, having
* first cleared every alias it could otherwise be found under.
* Persist the pin for `candidate` under EVERY key that names exactly this
* film, having first cleared any remaining alias it could otherwise be found
* under.
*
* Both halves are load-bearing, and each rules out the other's obvious
* shortcut:
* Storing it once, under the most-trusted key alone, is not enough: a movie's
* identity grows. Pin Dune while `tmdb:438631` is known and the decision goes
* only there — then reopen it, and the page starts out with nothing but a
* title and a year, so its lookup asks for `title:dune:2021` and finds
* nothing. The pin is ignored until enrichment lands, and if enrichment is off
* or never answers, permanently. Writing both keys makes the preference
* readable at every stage of the same film's identity.
*
* - Writing ONLY the top key leaves the lower-trust aliases pointing at
* whatever was pinned before, and a reopen that reads one of those —
* because enrichment has not landed yet — starts the source the user just
* replaced. Hence the clear.
* - Writing the decision INTO every alias is not the fix either: the yearless
* `title:{base}:` form is shared by every remake, so a known-year pin stored
* there would answer for a different film — pin Dune (2021), open Dune
* (1984) before its year arrives, and it would start the 2021 source. That
* alias stays readable, for genuinely pre-enrichment pins, and unwritten.
* `keys.write` is exactly the right set to spread across, because it holds
* only keys that name ONE film. The yearless `title:{base}:` form is
* deliberately not among them — every remake shares it, so a known-year pin
* stored there would answer for a different film: pin Dune (2021), open Dune
* (1984) before its year arrives, and it would start the 2021 source. That
* alias stays readable, for genuinely pre-enrichment pins, and unwritten.
*/
export async function writePin(
pins: Pick<VodSourcePinService, 'set' | 'clear'>,
keys: PinKeySets,
candidate: VodSourceCandidate
): Promise<boolean> {
const matchKey = keys.write[0] ?? buildVodSourceMatchKey(candidate);
const [primary, ...aliases] = keys.write;
const matchKey = primary ?? buildVodSourceMatchKey(candidate);
if (!matchKey) {
return false;
}
@@ -100,6 +104,7 @@ export async function writePin(
// source the user just replaced — while reporting failure leaves the
// canonical row durable and the UI showing a pin that is no longer the
// stored one.
const written = new Set([matchKey, ...aliases]);
return pins.set(
{
matchKey,
@@ -107,7 +112,10 @@ export async function writePin(
contentId: candidate.contentId,
portalType: candidate.portalType,
},
retirablePinKeys(keys).filter((key) => key !== matchKey)
// Whatever is left is the ambiguous row this session actually read —
// the only one an unpin may take, and one this write must not keep.
retirablePinKeys(keys).filter((key) => !written.has(key)),
aliases
);
}
@@ -195,6 +195,43 @@ describe('createPrimaryActionPosition', () => {
expect(load).not.toHaveBeenCalled();
});
it('stops describing the old pin the moment a new one is chosen', async () => {
const load = jest
.fn()
.mockResolvedValueOnce(position(4200))
.mockImplementationOnce(
() => new Promise<PlaybackPositionData | null>(() => undefined)
);
const { api, sourcesSignal } = setup(
[source(), ALT],
position(2538),
load
);
TestBed.tick();
await Promise.resolve();
TestBed.tick();
expect(api.position()?.positionSeconds).toBe(4200);
sourcesSignal.set([
source(),
source({
id: 'playlist-3:xtream:77',
playlistId: 'playlist-3',
contentId: 77,
isPinned: true,
isActive: false,
}),
]);
TestBed.tick();
// A row IS loaded — the one belonging to the copy that was pinned a
// moment ago. Treating "loaded something" as "loaded this" makes the
// button offer the old copy's timecode for the new one.
expect(api.position()).toBeNull();
expect(api.hasPosition()).toBe(false);
});
it('drops a lookup the pin outran', async () => {
let resolveFirst: (value: PlaybackPositionData | null) => void = () => {
/* replaced below */
@@ -113,7 +113,11 @@ export function createPrimaryActionPosition(
// button is not going to start — and a click across this window looks
// the pinned position up separately and begins somewhere else. Say
// nothing until the answer is in; empty beats wrong.
if (!pinnedLoadedFor()) {
//
// Compared by identity rather than mere presence: re-pinning leaves
// the PREVIOUS copy's row loaded, and "something was loaded once" would
// let the button wear that copy's timecode until the new lookup lands.
if (pinnedLoadedFor() !== pinned.id) {
return null;
}
@@ -87,10 +87,19 @@ export class VodSourcePinService {
}
/**
* Store the pin, retiring `retireKeys` in the same transaction — a
* half-applied change has no honest outcome to report.
* Store the pin, also under `aliasKeys`, retiring `retireKeys` — all in
* the same transaction, because a half-applied change has no honest
* outcome to report.
*
* @param aliasKeys further keys naming exactly this film. A movie's
* identity grows as enrichment lands, and a pin recorded only under the
* richest key is invisible to the next reopen that has not been enriched.
*/
async set(pin: VodSourcePin, retireKeys: string[] = []): Promise<boolean> {
async set(
pin: VodSourcePin,
retireKeys: string[] = [],
aliasKeys: string[] = []
): Promise<boolean> {
if (!this.isAvailable) {
return false;
}
@@ -98,7 +107,8 @@ export class VodSourcePinService {
try {
const result = await window.electron.dbSetVodSourcePin(
pin,
retireKeys
retireKeys,
aliasKeys
);
return result?.success === true;
} catch (error) {
@@ -913,10 +913,15 @@ export interface ElectronBridgeApi {
dbClearVodSourcePinsForPlaylist: (
playlistId: string
) => Promise<ElectronBridgeResult>;
/** `retireKeys` are removed in the SAME transaction as the write. */
/**
* `aliasKeys` receive the same pin, and `retireKeys` are removed, all in
* the SAME transaction as the write. Aliases keep a pin readable under the
* poorer key forms the movie had before enrichment.
*/
dbSetVodSourcePin: (
pin: VodSourcePin,
retireKeys?: string[]
retireKeys?: string[],
aliasKeys?: string[]
) => Promise<ElectronBridgeResult>;
dbClearVodSourcePin: (
matchKeys: string[]