From 4fa18729642dcc3e5e6610478ff5f58c4e6178e2 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 7 Jul 2026 08:26:19 +0200 Subject: [PATCH] fix(favorites): scope reorder position writes by playlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global favorites reorder wrote the new position filtering only by content_id, so two Xtream playlists holding a favorite with the same content_id would clobber each other's persisted order (greptile P1). Thread playlist_id through the whole reorder path — the renderer builder (UnifiedCollectionItem already carries playlistId), the IPC contract (ElectronBridgeFavoriteReorderUpdate + inline payload types), the worker op — and scope the prepared UPDATE by (contentId, playlistId), matching the favorites composite unique index. Tests: favorites.operations.spec asserts the playlistId placeholder and per-row playlistId payload; preload contract fixture updated. Co-Authored-By: Claude Opus 4.8 --- .../src/app/api/main.preload.spec-data.ts | 4 +++- apps/electron-backend/src/app/api/main.preload.ts | 2 +- .../operations/favorites.operations.spec.ts | 13 ++++++++++--- .../database/operations/favorites.operations.ts | 15 ++++++++++++--- .../src/app/events/database/favorites.events.ts | 2 +- .../src/app/workers/database.worker.ts | 2 +- .../collection/unified-favorites-data.service.ts | 7 ++++++- .../interfaces/src/lib/electron-api.interface.ts | 2 ++ 8 files changed, 36 insertions(+), 11 deletions(-) diff --git a/apps/electron-backend/src/app/api/main.preload.spec-data.ts b/apps/electron-backend/src/app/api/main.preload.spec-data.ts index 229524cf8..3b8b5cfab 100644 --- a/apps/electron-backend/src/app/api/main.preload.spec-data.ts +++ b/apps/electron-backend/src/app/api/main.preload.spec-data.ts @@ -34,7 +34,9 @@ const streams = [{ stream_id: 42, name: 'Channel' }]; const favorites = [{ contentId: 1, playlistId }]; const recentlyViewed = [{ contentId: 2, playlistId }]; const categoryIds = [10, 11]; -const reorderUpdates = [{ content_id: 12, position: 1 }]; +const reorderUpdates = [ + { content_id: 12, playlist_id: 'playlist-1', position: 1 }, +]; const recentItemsBatch = [{ contentId: 13, playlistId }]; const playbackData = { contentXtreamId: 42, diff --git a/apps/electron-backend/src/app/api/main.preload.ts b/apps/electron-backend/src/app/api/main.preload.ts index 743220410..0110962a5 100644 --- a/apps/electron-backend/src/app/api/main.preload.ts +++ b/apps/electron-backend/src/app/api/main.preload.ts @@ -710,7 +710,7 @@ const electronApi: ElectronBridgeApi = { dbGetAllGlobalFavorites: () => ipcRenderer.invoke('DB_GET_ALL_GLOBAL_FAVORITES'), dbReorderGlobalFavorites: ( - updates: { content_id: number; position: number }[] + updates: { content_id: number; playlist_id: string; position: number }[] ) => ipcRenderer.invoke('DB_REORDER_GLOBAL_FAVORITES', updates), // Recently viewed (playlist-specific) dbGetRecentItems: (playlistId: string) => diff --git a/apps/electron-backend/src/app/database/operations/favorites.operations.spec.ts b/apps/electron-backend/src/app/database/operations/favorites.operations.spec.ts index 0024409f1..90b65e4de 100644 --- a/apps/electron-backend/src/app/database/operations/favorites.operations.spec.ts +++ b/apps/electron-backend/src/app/database/operations/favorites.operations.spec.ts @@ -111,15 +111,19 @@ describe('favorites.operations', () => { await expect( reorderGlobalFavorites(db, [ - { content_id: 30, position: 0 }, - { content_id: 10, position: 1 }, - { content_id: 20, position: 2 }, + { content_id: 30, playlist_id: 'p1', position: 0 }, + { content_id: 10, playlist_id: 'p1', position: 1 }, + { content_id: 20, playlist_id: 'p2', position: 2 }, ]) ).resolves.toEqual({ success: true }); expect(updatePrepare).toHaveBeenCalledTimes(1); expect(placeholderMock).toHaveBeenCalledWith('position'); expect(placeholderMock).toHaveBeenCalledWith('contentId'); + // Regression: favorites are playlist-scoped, so the UPDATE must + // filter by playlistId too — otherwise a same-contentId favorite + // in another playlist gets its position silently rewritten. + expect(placeholderMock).toHaveBeenCalledWith('playlistId'); expect(transaction).toHaveBeenCalledTimes(1); // Regression (issue #1137): the prepared UPDATE must be dispatched @@ -131,14 +135,17 @@ describe('favorites.operations', () => { expect(updateRun).toHaveBeenNthCalledWith(1, { position: 0, contentId: 30, + playlistId: 'p1', }); expect(updateRun).toHaveBeenNthCalledWith(2, { position: 1, contentId: 10, + playlistId: 'p1', }); expect(updateRun).toHaveBeenNthCalledWith(3, { position: 2, contentId: 20, + playlistId: 'p2', }); }); }); diff --git a/apps/electron-backend/src/app/database/operations/favorites.operations.ts b/apps/electron-backend/src/app/database/operations/favorites.operations.ts index ebf5dba17..8bb66e98d 100644 --- a/apps/electron-backend/src/app/database/operations/favorites.operations.ts +++ b/apps/electron-backend/src/app/database/operations/favorites.operations.ts @@ -140,7 +140,7 @@ function selectGlobalFavoriteRows( export async function reorderGlobalFavorites( db: AppDatabase, - updates: { content_id: number; position: number }[], + updates: { content_id: number; playlist_id: string; position: number }[], control?: OperationControl ): Promise<{ success: boolean }> { if (!Array.isArray(updates) || updates.length === 0) { @@ -152,17 +152,25 @@ export async function reorderGlobalFavorites( // Drizzle's .set() doesn't accept a bare Placeholder — wrap it in an // sql template so the value resolves to SQL at compile time. + // Scope by (contentId, playlistId): the favorites table is + // playlist-scoped, so filtering by contentId alone would also rewrite + // the position of a same-contentId favorite in another playlist. const updateFavoritePosition = db .update(schema.favorites) .set({ position: sql`${sql.placeholder('position')}` }) - .where(eq(schema.favorites.contentId, sql.placeholder('contentId'))) + .where( + and( + eq(schema.favorites.contentId, sql.placeholder('contentId')), + eq(schema.favorites.playlistId, sql.placeholder('playlistId')) + ) + ) .prepare(); for (const chunk of chunkValues(updates, DEFAULT_BATCH_SIZE)) { await checkpointOperation(control); await db.transaction(() => { - for (const { content_id, position } of chunk) { + for (const { content_id, playlist_id, position } of chunk) { // Must be .run() (synchronous), NOT .execute(): on the // better-sqlite3 driver .execute() defers the write to a // resolved promise, which never settles inside this @@ -172,6 +180,7 @@ export async function reorderGlobalFavorites( updateFavoritePosition.run({ position, contentId: content_id, + playlistId: playlist_id, }); } }); diff --git a/apps/electron-backend/src/app/events/database/favorites.events.ts b/apps/electron-backend/src/app/events/database/favorites.events.ts index a0c77ae28..2daef247b 100644 --- a/apps/electron-backend/src/app/events/database/favorites.events.ts +++ b/apps/electron-backend/src/app/events/database/favorites.events.ts @@ -39,7 +39,7 @@ handleWorkerRequest('DB_GET_ALL_GLOBAL_FAVORITES', () => ({})); handleWorkerRequest( 'DB_REORDER_GLOBAL_FAVORITES', - (updates: { content_id: number; position: number }[]) => ({ + (updates: { content_id: number; playlist_id: string; position: number }[]) => ({ updates, }) ); diff --git a/apps/electron-backend/src/app/workers/database.worker.ts b/apps/electron-backend/src/app/workers/database.worker.ts index 868a856d5..3cfd67144 100644 --- a/apps/electron-backend/src/app/workers/database.worker.ts +++ b/apps/electron-backend/src/app/workers/database.worker.ts @@ -770,7 +770,7 @@ async function executeRequest(message: DbWorkerRequestMessage) { case 'DB_REORDER_GLOBAL_FAVORITES': { const payload = message.payload as { - updates: { content_id: number; position: number }[]; + updates: { content_id: number; playlist_id: string; position: number }[]; }; return reorderGlobalFavorites(db, payload.updates); } diff --git a/libs/portal/shared/data-access/src/lib/collection/unified-favorites-data.service.ts b/libs/portal/shared/data-access/src/lib/collection/unified-favorites-data.service.ts index 096fdc6a8..4dbf97134 100644 --- a/libs/portal/shared/data-access/src/lib/collection/unified-favorites-data.service.ts +++ b/libs/portal/shared/data-access/src/lib/collection/unified-favorites-data.service.ts @@ -807,12 +807,17 @@ export class UnifiedFavoritesDataService { } private buildXtreamPositionUpdates(items: UnifiedCollectionItem[]) { - const updates: { content_id: number; position: number }[] = []; + const updates: { + content_id: number; + playlist_id: string; + position: number; + }[] = []; for (const item of items) { if (item.sourceType === 'xtream' && item.contentId != null) { updates.push({ content_id: item.contentId, + playlist_id: item.playlistId, position: updates.length, }); } diff --git a/libs/shared/interfaces/src/lib/electron-api.interface.ts b/libs/shared/interfaces/src/lib/electron-api.interface.ts index 73d222fcb..3ad7c5b5e 100644 --- a/libs/shared/interfaces/src/lib/electron-api.interface.ts +++ b/libs/shared/interfaces/src/lib/electron-api.interface.ts @@ -414,6 +414,8 @@ export interface ElectronBridgeGlobalRecentlyAddedItem extends ElectronBridgeXtr export interface ElectronBridgeFavoriteReorderUpdate { content_id: number; + /** Favorites are playlist-scoped — scope the position write per playlist */ + playlist_id: string; position: number; }