From 4ac4bb4f3909e7af704c2c22047fdd8b07c6ca46 Mon Sep 17 00:00:00 2001 From: hfcRed Date: Fri, 31 Jul 2026 10:34:52 -0400 Subject: [PATCH] Fix trophy user migration/merging handling --- .../admin/AdminRepository.server.test.ts | 133 ++++++++++++++++++ app/features/admin/AdminRepository.server.ts | 86 +++++++++++ .../trophies/TrophyRepository.server.test.ts | 38 +++++ migrations/163-trophies.js | 6 +- 4 files changed, 260 insertions(+), 3 deletions(-) diff --git a/app/features/admin/AdminRepository.server.test.ts b/app/features/admin/AdminRepository.server.test.ts index e759c1e4b..55eacbdb4 100644 --- a/app/features/admin/AdminRepository.server.test.ts +++ b/app/features/admin/AdminRepository.server.test.ts @@ -1,8 +1,12 @@ import { beforeEach, describe, expect, test } from "vitest"; import * as BuildFactory from "~/db/seed/factories/BuildFactory"; +import * as TournamentFactory from "~/db/seed/factories/TournamentFactory"; +import * as TournamentOrganizationFactory from "~/db/seed/factories/TournamentOrganizationFactory"; +import * as TrophyFactory from "~/db/seed/factories/TrophyFactory"; import * as UserFactory from "~/db/seed/factories/UserFactory"; import { db } from "~/db/sql"; import * as UserRepository from "~/features/user-page/UserRepository.server"; +import { databaseTimestampNow } from "~/utils/dates"; import * as AdminRepository from "./AdminRepository.server"; const users = UserFactory.pool(); @@ -12,6 +16,135 @@ const users = UserFactory.pool(); const createUsers = (count: number) => users.create(count, (index) => ({ discordId: String(index) })); +describe("migrate", () => { + let oldUserId: number; + let newUserId: number; + + beforeEach(async () => { + oldUserId = (await UserFactory.create()).id; + newUserId = (await UserFactory.create()).id; + }); + + const insertOwnership = async (args: { + trophyId: number; + tournamentId: number; + userId: number; + }) => { + // biome-ignore lint/plugin: awarding a trophy requires playing out a whole tournament; migrate only cares that the rows exist + await db.insertInto("TrophyOwner").values(args).execute(); + // biome-ignore lint/plugin: special trophies are synced from X rank placements; migrate only cares that the rows exist + await db + .insertInto("SpecialTrophyOwner") + .values({ + trophyId: args.trophyId, + userId: args.userId, + createdAt: databaseTimestampNow(), + }) + .execute(); + }; + + const createPendingWithApprovals = async (args: { + submitterUserId: number; + managerId: number; + approverUserIds: number[]; + }) => { + const organization = await TournamentOrganizationFactory.create({ + ownerId: oldUserId, + }); + + return TrophyFactory.createPending( + { + organizationId: organization.id, + submitterUserId: args.submitterUserId, + managerId: args.managerId, + }, + { approverUserIds: args.approverUserIds }, + ); + }; + + test("re-points trophy data to the remaining user", async () => { + const trophy = await TrophyFactory.create({ + creatorId: newUserId, + managerId: newUserId, + }); + const tournament = await TournamentFactory.create({ authorId: oldUserId }); + await insertOwnership({ + trophyId: trophy.id, + tournamentId: tournament.id, + userId: newUserId, + }); + await createPendingWithApprovals({ + submitterUserId: newUserId, + managerId: newUserId, + approverUserIds: [newUserId], + }); + + expect(await AdminRepository.migrate({ newUserId, oldUserId })).toBe(null); + + const migrated = await db + .selectFrom("Trophy") + .select(["creatorId", "managerId"]) + .where("id", "=", trophy.id) + .executeTakeFirstOrThrow(); + expect(migrated).toEqual({ + creatorId: oldUserId, + managerId: oldUserId, + }); + + expect( + await db.selectFrom("TrophyOwner").select("userId").execute(), + ).toEqual([{ userId: oldUserId }]); + expect( + await db.selectFrom("SpecialTrophyOwner").select("userId").execute(), + ).toEqual([{ userId: oldUserId }]); + + const pending = await db + .selectFrom("PendingTrophy") + .select(["submitterUserId", "managerId"]) + .executeTakeFirstOrThrow(); + expect(pending).toEqual({ + submitterUserId: oldUserId, + managerId: oldUserId, + }); + + expect( + await db.selectFrom("PendingTrophyApproval").select("userId").execute(), + ).toEqual([{ userId: oldUserId }]); + }); + + test("drops the migrated account's duplicate rows when both accounts own the same trophy", async () => { + const trophy = await TrophyFactory.create({ + creatorId: oldUserId, + managerId: oldUserId, + }); + const tournament = await TournamentFactory.create({ authorId: oldUserId }); + for (const userId of [oldUserId, newUserId]) { + await insertOwnership({ + trophyId: trophy.id, + tournamentId: tournament.id, + userId, + }); + } + await createPendingWithApprovals({ + submitterUserId: oldUserId, + managerId: oldUserId, + approverUserIds: [oldUserId, newUserId], + }); + + expect(await AdminRepository.migrate({ newUserId, oldUserId })).toBe(null); + + expect( + await db.selectFrom("TrophyOwner").select("userId").execute(), + ).toEqual([{ userId: oldUserId }]); + expect( + await db.selectFrom("SpecialTrophyOwner").select("userId").execute(), + ).toEqual([{ userId: oldUserId }]); + expect( + await db.selectFrom("PendingTrophyApproval").select("userId").execute(), + ).toEqual([{ userId: oldUserId }]); + }); +}); + describe("findAllBannedUsers", () => { let admin: { id: number }; diff --git a/app/features/admin/AdminRepository.server.ts b/app/features/admin/AdminRepository.server.ts index 59fced297..b663c8fc8 100644 --- a/app/features/admin/AdminRepository.server.ts +++ b/app/features/admin/AdminRepository.server.ts @@ -126,6 +126,92 @@ export function migrate(args: { newUserId: number; oldUserId: number }) { .set({ userId: args.oldUserId }) .execute(); + // If both accounts own the same trophy, drop the + // migrated account's duplicate rows + await trx + .deleteFrom("TrophyOwner") + .where("userId", "=", args.newUserId) + .where((eb) => + eb.exists( + eb + .selectFrom("TrophyOwner as existing") + .select("existing.trophyId") + .where("existing.userId", "=", args.oldUserId) + .whereRef("existing.trophyId", "=", "TrophyOwner.trophyId") + .whereRef("existing.tournamentId", "=", "TrophyOwner.tournamentId"), + ), + ) + .execute(); + await trx + .deleteFrom("SpecialTrophyOwner") + .where("userId", "=", args.newUserId) + .where((eb) => + eb( + "SpecialTrophyOwner.trophyId", + "in", + eb + .selectFrom("SpecialTrophyOwner") + .select("trophyId") + .where("userId", "=", args.oldUserId), + ), + ) + .execute(); + await trx + .deleteFrom("PendingTrophyApproval") + .where("userId", "=", args.newUserId) + .where((eb) => + eb( + "PendingTrophyApproval.pendingTrophyId", + "in", + eb + .selectFrom("PendingTrophyApproval") + .select("pendingTrophyId") + .where("userId", "=", args.oldUserId), + ), + ) + .execute(); + + await trx + .updateTable("TrophyOwner") + .where("userId", "=", args.newUserId) + .set({ userId: args.oldUserId }) + .execute(); + await trx + .updateTable("SpecialTrophyOwner") + .where("userId", "=", args.newUserId) + .set({ userId: args.oldUserId }) + .execute(); + await trx + .updateTable("Trophy") + .where("creatorId", "=", args.newUserId) + .set({ creatorId: args.oldUserId }) + .execute(); + await trx + .updateTable("Trophy") + .where("managerId", "=", args.newUserId) + .set({ managerId: args.oldUserId }) + .execute(); + await trx + .updateTable("PendingTrophy") + .where("submitterUserId", "=", args.newUserId) + .set({ submitterUserId: args.oldUserId }) + .execute(); + await trx + .updateTable("PendingTrophy") + .where("managerId", "=", args.newUserId) + .set({ managerId: args.oldUserId }) + .execute(); + await trx + .updateTable("PendingTrophy") + .where("declinedByUserId", "=", args.newUserId) + .set({ declinedByUserId: args.oldUserId }) + .execute(); + await trx + .updateTable("PendingTrophyApproval") + .where("userId", "=", args.newUserId) + .set({ userId: args.oldUserId }) + .execute(); + const deletedUser = await trx .deleteFrom("User") .where("User.id", "=", args.newUserId) diff --git a/app/features/trophies/TrophyRepository.server.test.ts b/app/features/trophies/TrophyRepository.server.test.ts index 7419179b8..96b835e5a 100644 --- a/app/features/trophies/TrophyRepository.server.test.ts +++ b/app/features/trophies/TrophyRepository.server.test.ts @@ -252,6 +252,44 @@ describe("trophy list tiers", () => { } }); +describe("user deletion", () => { + test("keeps their trophies and drops their approvals", async () => { + const submitter = await UserFactory.create(); + const deleted = await UserFactory.create(); + + const trophy = await TrophyFactory.create({ + name: "Orphaned Trophy", + creatorId: deleted.id, + managerId: deleted.id, + }); + + const organization = await TournamentOrganizationFactory.create({ + ownerId: submitter.id, + }); + await TrophyFactory.createPending( + { + organizationId: organization.id, + submitterUserId: submitter.id, + }, + { approverUserIds: [deleted.id] }, + ); + + // biome-ignore lint/plugin: no production code path deletes users, the test pins the schemas on-delete behavior + await db.deleteFrom("User").where("id", "=", deleted.id).execute(); + + const orphaned = await db + .selectFrom("Trophy") + .select(["creatorId", "managerId"]) + .where("id", "=", trophy.id) + .executeTakeFirstOrThrow(); + expect(orphaned).toEqual({ creatorId: null, managerId: null }); + + expect( + await db.selectFrom("PendingTrophyApproval").selectAll().execute(), + ).toEqual([]); + }); +}); + async function findTrophyByName(name: string) { return (await TrophyRepository.all()).find((row) => row.name === name); } diff --git a/migrations/163-trophies.js b/migrations/163-trophies.js index 312b92f71..9346accea 100644 --- a/migrations/163-trophies.js +++ b/migrations/163-trophies.js @@ -11,8 +11,8 @@ export function up(db) { "creatorId" integer, "managerId" integer, foreign key ("organizationId") references "TournamentOrganization"("id") on delete set null, - foreign key ("creatorId") references "User"("id"), - foreign key ("managerId") references "User"("id") + foreign key ("creatorId") references "User"("id") on delete set null, + foreign key ("managerId") references "User"("id") on delete set null ) strict `, ).run(); @@ -91,7 +91,7 @@ export function up(db) { "userId" integer not null, "createdAt" integer not null, foreign key ("pendingTrophyId") references "PendingTrophy"("id") on delete cascade, - foreign key ("userId") references "User"("id"), + foreign key ("userId") references "User"("id") on delete cascade, unique("pendingTrophyId", "userId") ) strict `,