mirror of
https://github.com/jakejarvis/sofa.git
synced 2026-10-16 05:25:42 -04:00
fix(credits): replace title cast on refresh and remove duplicate crew rows
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
5 files changed
+3543
-6
No files matched your search
@@ -1,11 +1,11 @@
|
||||
import type { CastMember } from "@sofa/api/schemas";
|
||||
import {
|
||||
batchUpsertPersonsTransaction,
|
||||
batchUpsertTitleCast,
|
||||
getCastForTitleJoined,
|
||||
getExistingPersonsByTmdbIds,
|
||||
getFallbackPersonsByTmdbIds,
|
||||
getPersonsForTitleCast,
|
||||
replaceTitleCastTransaction,
|
||||
} from "@sofa/db/queries/credits";
|
||||
import { getTitleById } from "@sofa/db/queries/title";
|
||||
import type { titleCast } from "@sofa/db/schema";
|
||||
@@ -164,7 +164,8 @@ export async function refreshCredits(titleId: string) {
|
||||
});
|
||||
crewOrder++;
|
||||
}
|
||||
batchUpsertTitleCast(allCastRows);
|
||||
// Skip empty payloads so a blank TMDB response doesn't wipe existing credits
|
||||
if (allCastRows.length > 0) replaceTitleCastTransaction(titleId, allCastRows);
|
||||
} else {
|
||||
const credits = await getTvAggregateCredits(title.tmdbId);
|
||||
const tvCast = credits.cast ?? [];
|
||||
@@ -244,7 +245,8 @@ export async function refreshCredits(titleId: string) {
|
||||
});
|
||||
crewOrder++;
|
||||
}
|
||||
batchUpsertTitleCast(allCastRows);
|
||||
// Skip empty payloads so a blank TMDB response doesn't wipe existing credits
|
||||
if (allCastRows.length > 0) replaceTitleCastTransaction(titleId, allCastRows);
|
||||
}
|
||||
|
||||
log.debug(`Credits refreshed for "${title.title}"`);
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import { readdirSync, readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
import { beforeEach, describe, expect, test, vi } from "vitest";
|
||||
|
||||
import { persons, titleCast } from "@sofa/db/schema";
|
||||
import { clearAllTables, eq, insertTitle, testDb } from "@sofa/test/db";
|
||||
import { clearAllTables, eq, insertTitle, testClient, testDb } from "@sofa/test/db";
|
||||
|
||||
const { mockGetMovieCredits, mockGetTvAggregateCredits } = vi.hoisted(() => ({
|
||||
mockGetMovieCredits: vi.fn<
|
||||
@@ -191,6 +195,77 @@ describe("refreshCredits", () => {
|
||||
expect(creator).toBeDefined();
|
||||
});
|
||||
|
||||
test("does not duplicate crew rows when refreshed twice", async () => {
|
||||
insertTitle({ id: "m1", tmdbId: 100, type: "movie" });
|
||||
|
||||
mockGetMovieCredits.mockResolvedValue({
|
||||
cast: [{ id: 1, name: "Actor A", profile_path: "/a.jpg", character: "Hero", popularity: 10 }],
|
||||
crew: [
|
||||
{
|
||||
id: 3,
|
||||
name: "Dir Person",
|
||||
profile_path: "/d.jpg",
|
||||
job: "Director",
|
||||
department: "Directing",
|
||||
popularity: 8,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await refreshCredits("m1");
|
||||
await refreshCredits("m1");
|
||||
|
||||
const castRows = testDb.select().from(titleCast).where(eq(titleCast.titleId, "m1")).all();
|
||||
expect(castRows).toHaveLength(2);
|
||||
});
|
||||
|
||||
test("removes cast members no longer returned by TMDB", async () => {
|
||||
insertTitle({ id: "m1", tmdbId: 100, type: "movie" });
|
||||
const actorA = {
|
||||
id: 1,
|
||||
name: "Actor A",
|
||||
profile_path: null,
|
||||
character: "Hero",
|
||||
popularity: 10,
|
||||
};
|
||||
const actorB = {
|
||||
id: 2,
|
||||
name: "Actor B",
|
||||
profile_path: null,
|
||||
character: "Villain",
|
||||
popularity: 5,
|
||||
};
|
||||
|
||||
mockGetMovieCredits.mockResolvedValue({ cast: [actorA, actorB], crew: [] });
|
||||
await refreshCredits("m1");
|
||||
expect(testDb.select().from(titleCast).where(eq(titleCast.titleId, "m1")).all()).toHaveLength(
|
||||
2,
|
||||
);
|
||||
|
||||
mockGetMovieCredits.mockResolvedValue({ cast: [actorA], crew: [] });
|
||||
await refreshCredits("m1");
|
||||
expect(testDb.select().from(titleCast).where(eq(titleCast.titleId, "m1")).all()).toHaveLength(
|
||||
1,
|
||||
);
|
||||
});
|
||||
|
||||
test("keeps existing credits when TMDB returns an empty payload", async () => {
|
||||
insertTitle({ id: "m1", tmdbId: 100, type: "movie" });
|
||||
|
||||
mockGetMovieCredits.mockResolvedValue({
|
||||
cast: [{ id: 1, name: "Actor A", profile_path: null, character: "Hero", popularity: 10 }],
|
||||
crew: [],
|
||||
});
|
||||
await refreshCredits("m1");
|
||||
|
||||
mockGetMovieCredits.mockResolvedValue({ cast: [], crew: [] });
|
||||
await refreshCredits("m1");
|
||||
|
||||
expect(testDb.select().from(titleCast).where(eq(titleCast.titleId, "m1")).all()).toHaveLength(
|
||||
1,
|
||||
);
|
||||
});
|
||||
|
||||
test("does nothing for nonexistent title", async () => {
|
||||
await refreshCredits("nonexistent");
|
||||
expect(mockGetMovieCredits).not.toHaveBeenCalled();
|
||||
@@ -284,3 +359,38 @@ describe("refreshCredits", () => {
|
||||
expect(jobs).toEqual(["Director", "Writer"]);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── dedupe migration ───────────────────────────────────────────────
|
||||
|
||||
describe("dedupe_title_cast migration", () => {
|
||||
test("keeps one row per credit", () => {
|
||||
insertTitle({ id: "m1", tmdbId: 1 });
|
||||
insertPerson("p1", 100, "Director Person");
|
||||
|
||||
for (let i = 0; i < 3; i++) {
|
||||
testDb
|
||||
.insert(titleCast)
|
||||
.values({
|
||||
titleId: "m1",
|
||||
personId: "p1",
|
||||
character: null,
|
||||
department: "Directing",
|
||||
job: "Director",
|
||||
displayOrder: 100,
|
||||
lastFetchedAt: new Date(),
|
||||
})
|
||||
.run();
|
||||
}
|
||||
insertCastEntry("m1", "p1", { character: "Hero" });
|
||||
|
||||
const dir = fileURLToPath(new URL("../../db/drizzle", import.meta.url));
|
||||
const folder = readdirSync(dir).find((d) => d.endsWith("_dedupe_title_cast"));
|
||||
expect(folder).toBeDefined();
|
||||
testClient.exec(readFileSync(join(dir, folder!, "migration.sql"), "utf8"));
|
||||
|
||||
const rows = testDb.select().from(titleCast).where(eq(titleCast.titleId, "m1")).all();
|
||||
expect(rows).toHaveLength(2);
|
||||
expect(rows.filter((r) => r.department === "Directing")).toHaveLength(1);
|
||||
expect(rows.filter((r) => r.department === "Acting")).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,8 @@
|
||||
-- Crew rows store character = NULL and SQLite treats NULLs as distinct in the
|
||||
-- titleCast_unique index, so every credits refresh inserted another copy.
|
||||
-- Keep the most recently inserted row for each logical credit.
|
||||
DELETE FROM titleCast
|
||||
WHERE rowid NOT IN (
|
||||
SELECT MAX(rowid) FROM titleCast
|
||||
GROUP BY titleId, personId, department, COALESCE(character, ''), COALESCE(job, '')
|
||||
);
|
||||
File diff suppressed because it is too large.
Load diff
@@ -99,9 +99,19 @@ export function getPersonsForTitleCast(titleId: string) {
|
||||
.all();
|
||||
}
|
||||
|
||||
export function batchUpsertTitleCast(rows: (typeof titleCast.$inferInsert)[]): void {
|
||||
/**
|
||||
* Atomically replace all cast/crew rows for a title. Upserting is not enough:
|
||||
* crew rows have character = NULL, and SQLite treats NULLs as distinct in the
|
||||
* unique index, so upserts kept inserting duplicates.
|
||||
*/
|
||||
export function replaceTitleCastTransaction(
|
||||
titleId: string,
|
||||
rows: (typeof titleCast.$inferInsert)[],
|
||||
): void {
|
||||
db.transaction((tx) => {
|
||||
tx.delete(titleCast).where(eq(titleCast.titleId, titleId)).run();
|
||||
if (rows.length === 0) return;
|
||||
db.insert(titleCast)
|
||||
tx.insert(titleCast)
|
||||
.values(rows)
|
||||
.onConflictDoUpdate({
|
||||
target: [titleCast.titleId, titleCast.personId, titleCast.department, titleCast.character],
|
||||
@@ -113,6 +123,7 @@ export function batchUpsertTitleCast(rows: (typeof titleCast.$inferInsert)[]): v
|
||||
},
|
||||
})
|
||||
.run();
|
||||
});
|
||||
}
|
||||
|
||||
export function getCastForTitleJoined(titleId: string) {
|
||||
|
||||
Reference in new issue
Block a user