fix(favorites): scope reorder position writes by playlist

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 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 4.8 committed 2026-07-07 08:26:19 +02:00
1 parent 5cc69d72b1
commit 4fa1872964
8 files changed
+36 -11

No files matched your search

@@ -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,
@@ -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) =>
@@ -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',
});
});
});
@@ -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<number> 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<number>`${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,
});
}
});
@@ -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,
})
);
@@ -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);
}
@@ -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,
});
}
@@ -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;
}