diff --git a/app/features/admin/AdminRepository.server.ts b/app/features/admin/AdminRepository.server.ts index 27b309d2f..4169c73e3 100644 --- a/app/features/admin/AdminRepository.server.ts +++ b/app/features/admin/AdminRepository.server.ts @@ -1,5 +1,6 @@ +import type { Transaction } from "kysely"; import { db, sql } from "~/db/sql"; -import type { Tables } from "~/db/tables"; +import type { DB, Tables } from "~/db/tables"; import { dateToDatabaseTimestamp } from "~/utils/dates"; import invariant from "~/utils/invariant"; import { syncXPBadges } from "../badges/queries/syncXPBadges.server"; @@ -33,8 +34,21 @@ export const cleanUp = () => { cleanUpStm.run(); }; +/** + * Migrates user-related data. Takes data from the "old user" and remaps it to the Discord ID of the "new user". Used when user switches their Discord accounts. + * + * @param args - An object containing: + * - `newUserId`: The ID of the user whose data will be migrated and then deleted. + * - `oldUserId`: The ID of the user who will receive the migrated data. + * @returns A promise that resolves to `null` if the migration succeeds, or an error message if validation fails. + */ export function migrate(args: { newUserId: number; oldUserId: number }) { return db.transaction().execute(async (trx) => { + const error = await validateMigration(trx, args); + if (error) { + return error; + } + // delete some limited data from the target user // idea is to make the migration a bit more smooth // since it won't fail if some small thing has been added @@ -53,6 +67,24 @@ export function migrate(args: { newUserId: number; oldUserId: number }) { .where("userId", "=", args.newUserId) .set({ userId: args.oldUserId }) .execute(); + await trx + .updateTable("UnvalidatedUserSubmittedImage") + .where("submitterUserId", "=", args.newUserId) + .set({ submitterUserId: args.oldUserId }) + .execute(); + + // delete past team membership data (not user visible) + await trx + .deleteFrom("AllTeamMember") + .where("userId", "=", args.newUserId) + .where("leftAt", "is not", null) + .execute(); + // existing team membership will stay + await trx + .updateTable("AllTeamMember") + .where("userId", "=", args.newUserId) + .set({ userId: args.oldUserId }) + .execute(); const deletedUser = await trx .deleteFrom("User") @@ -65,9 +97,34 @@ export function migrate(args: { newUserId: number; oldUserId: number }) { .set({ discordId: deletedUser.discordId }) .where("User.id", "=", args.oldUserId) .execute(); + + return null; }); } +async function validateMigration( + trx: Transaction, + args: { newUserId: number; oldUserId: number }, +) { + const oldUserCurrentTeam = await trx + .selectFrom("TeamMember") + .select(["teamId"]) + .where("userId", "=", args.oldUserId) + .executeTakeFirst(); + + const newUserCurrentTeam = await trx + .selectFrom("TeamMember") + .select(["teamId"]) + .where("userId", "=", args.newUserId) + .executeTakeFirst(); + + if (oldUserCurrentTeam && newUserCurrentTeam) { + return "both old and new user are in teams"; + } + + return null; +} + export function replacePlusTiers( plusTiers: Array<{ userId: number; plusTier: number }>, ) { diff --git a/app/features/admin/actions/admin.server.ts b/app/features/admin/actions/admin.server.ts index d0b456605..297c34548 100644 --- a/app/features/admin/actions/admin.server.ts +++ b/app/features/admin/actions/admin.server.ts @@ -7,7 +7,12 @@ import { refreshBannedCache } from "~/features/ban/core/banned.server"; import * as UserRepository from "~/features/user-page/UserRepository.server"; import { requireRole } from "~/modules/permissions/guards.server"; import { logger } from "~/utils/logger"; -import { parseRequestPayload, successToast } from "~/utils/remix.server"; +import { + errorToast, + parseRequestPayload, + successToast, +} from "~/utils/remix.server"; +import { errorIsSqliteForeignKeyConstraintFailure } from "~/utils/sql"; import { assertUnreachable } from "~/utils/types"; import { _action, actualNumber, friendCode } from "~/utils/zod"; import { plusTiersFromVotingAndLeaderboard } from "../core/plus-tier.server"; @@ -24,13 +29,27 @@ export const action = async ({ request }: ActionFunctionArgs) => { case "MIGRATE": { requireRole(user, "STAFF"); - await AdminRepository.migrate({ - oldUserId: data["old-user"], - newUserId: data["new-user"], - }); + try { + const errorMessage = await AdminRepository.migrate({ + oldUserId: data["old-user"], + newUserId: data["new-user"], + }); - message = "Account migrated"; - break; + if (errorMessage) { + errorToast(`Migration failed. Reason: ${errorMessage}`); + } + + message = "Account migrated"; + break; + } catch (err) { + if (errorIsSqliteForeignKeyConstraintFailure(err)) { + errorToast( + "New user has data preventing the migration (e.g. member of tournament teams or SendouQ played)", + ); + } + + throw err; + } } case "REFRESH": { requireRole(user, "ADMIN"); diff --git a/app/features/admin/routes/admin.test.ts b/app/features/admin/routes/admin.test.ts index 2cea5d31f..04a38b3bb 100644 --- a/app/features/admin/routes/admin.test.ts +++ b/app/features/admin/routes/admin.test.ts @@ -1,7 +1,14 @@ -import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, test, vi } from "vitest"; import { db } from "~/db/sql"; import * as PlusVotingRepository from "~/features/plus-voting/PlusVotingRepository.server"; -import { dbInsertUsers, dbReset, wrappedAction } from "~/utils/Test"; +import * as TeamRepository from "~/features/team/TeamRepository.server"; +import * as UserRepository from "~/features/user-page/UserRepository.server"; +import { + assertResponseErrored, + dbInsertUsers, + dbReset, + wrappedAction, +} from "~/utils/Test"; import { dateToDatabaseTimestamp } from "~/utils/dates"; import type { adminActionSchema } from "../actions/admin.server"; import { action } from "./admin"; @@ -266,3 +273,79 @@ describe("Plus voting", () => { expect(await countPlusTierMembers(2)).toBe(1); }); }); + +const migrateUserAction = () => + adminAction( + { + _action: "MIGRATE", + "old-user": 1, + "new-user": 2, + }, + { user: "admin" }, + ); + +describe("Account migration", () => { + beforeEach(async () => { + await dbInsertUsers(2); + }); + + afterEach(() => { + dbReset(); + }); + + it("migrates a blank account", async () => { + expect(await UserRepository.findProfileByIdentifier("0")).toBeDefined(); + expect(await UserRepository.findProfileByIdentifier("1")).toBeDefined(); + + await migrateUserAction(); + + const oldUser = await UserRepository.findProfileByIdentifier("0"); // these are discord ids + const newUser = await UserRepository.findProfileByIdentifier("1"); + + expect(oldUser).toBeNull(); + expect(newUser?.id).toBe(1); // took the old user's id + }); + + it("two accounts with teams results in an error", async () => { + await TeamRepository.create({ + customUrl: "team-1", + name: "Team 1", + ownerUserId: 1, + isMainTeam: true, + }); + await TeamRepository.create({ + customUrl: "team-2", + name: "Team 2", + ownerUserId: 2, + isMainTeam: true, + }); + + const response = await migrateUserAction(); + + assertResponseErrored(response, "both old and new user are in teams"); + }); + + it("deletes past team membership status of the new user", async () => { + await TeamRepository.create({ + customUrl: "team-1", + name: "Team 1", + ownerUserId: 2, + isMainTeam: true, + }); + await TeamRepository.del(1); + + const membershipQuery = db + .selectFrom("AllTeamMember") + .select(["userId"]) + .where("userId", "=", 2); + + const membershipBeforeMigration = await membershipQuery.executeTakeFirst(); + expect(membershipBeforeMigration).toBeDefined(); + + await migrateUserAction(); + + const membershipAfterMigration = await membershipQuery.executeTakeFirst(); + + expect(membershipAfterMigration).toBeUndefined(); + }); +}); diff --git a/app/features/sendouq/actions/q.looking.server.ts b/app/features/sendouq/actions/q.looking.server.ts index e84448b1c..06cc05db0 100644 --- a/app/features/sendouq/actions/q.looking.server.ts +++ b/app/features/sendouq/actions/q.looking.server.ts @@ -67,7 +67,6 @@ export const action: ActionFunction = async ({ request }) => { targetGroupId: data.targetGroupId, }); } catch (e) { - if (!(e instanceof Error)) throw e; // the group disbanded before we could like it if (errorIsSqliteForeignKeyConstraintFailure(e)) return null; diff --git a/app/utils/Test.ts b/app/utils/Test.ts index f8fef70b3..9f7514aa6 100644 --- a/app/utils/Test.ts +++ b/app/utils/Test.ts @@ -90,9 +90,15 @@ export function wrappedLoader({ /** * Asserts that the given response errored out (with a toast message, via `errorToastIfFalsy(cond)` call) + * + * @param response - The HTTP response object to check. + * @param message - Optional. The expected error toast message shown to the user. */ -export function assertResponseErrored(response: Response) { +export function assertResponseErrored(response: Response, message?: string) { expect(response.headers.get("Location")).toContain("?__error="); + if (message) { + expect(response.headers.get("Location")).toContain(message); + } } async function authHeader(user?: "admin" | "regular"): Promise { @@ -105,6 +111,23 @@ async function authHeader(user?: "admin" | "regular"): Promise { return [["Cookie", await authSessionStorage.commitSession(session)]]; } +/** + * Resets all data in the database by deleting all rows from every table, + * except for SQLite system tables and the 'migrations' table. + * + * @example + * describe("My integration test", () => { + * beforeEach(async () => { + * await dbInsertUsers(2); + * }); + * + * afterEach(() => { + * dbReset(); + * }); + * + * // tests go here + * }); + */ export const dbReset = () => { const tables = sql .prepare( @@ -119,12 +142,26 @@ export const dbReset = () => { sql.prepare("PRAGMA foreign_keys = ON").run(); }; -export const dbInsertUsers = (count?: number) => +/** + * Inserts a specified number of user records into the "User" table in the database for integration testing. + * 1) id: 1, discordName: "user1", discordId: "0" + * 2) id: 2, discordName: "user2", discordId: "1" + * 3) etc. + * + * @param count - The number of users to insert. Defaults to 2 if not provided. + * + * @example + * // Inserts 5 users into the database + * await dbInsertUsers(5); + * + * // Inserts 2 users (default) + * await dbInsertUsers(); + */ +export const dbInsertUsers = (count = 2) => db .insertInto("User") .values( - // defaults to 2 = admin & regular "NZAP" - Array.from({ length: count ?? 2 }).map((_, i) => ({ + Array.from({ length: count }).map((_, i) => ({ id: i + 1, discordName: `user${i + 1}`, discordId: String(i), diff --git a/app/utils/sql.ts b/app/utils/sql.ts index c0dc0ed1d..0e7d128f9 100644 --- a/app/utils/sql.ts +++ b/app/utils/sql.ts @@ -2,8 +2,11 @@ export function errorIsSqliteUniqueConstraintFailure(error: any) { return error?.code === "SQLITE_CONSTRAINT_UNIQUE"; } -export function errorIsSqliteForeignKeyConstraintFailure(error: Error) { - return error?.message?.includes("FOREIGN KEY constraint failed"); +export function errorIsSqliteForeignKeyConstraintFailure(error: unknown) { + return ( + error instanceof Error && + error?.message?.includes("FOREIGN KEY constraint failed") + ); } export function parseDBJsonArray(value: any) {