Handle new account having a team in account merge Closes #2308

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<DB>,
+	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 }>,
 ) {

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");

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();
+	});
+});

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;
 

app/utils/Test.ts

@@ -90,9 +90,15 @@ export function wrappedLoader<T>({
 
 /**
  * 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<HeadersInit> {
@@ -105,6 +111,23 @@ async function authHeader(user?: "admin" | "regular"): Promise<HeadersInit> {
 	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),

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) {