Clear active roster if admin removes a member in it Closes #2427

app/features/scrims/routes/scrims.new.test.ts

@@ -13,7 +13,7 @@ import type { scrimsNewActionSchema } from "../scrims-schemas";
 
 const newScrimAction = wrappedAction<typeof scrimsNewActionSchema>({
 	action,
-	isNewForm: true,
+	isJsonSubmission: true,
 });
 
 const scrimPostsLoader = wrappedLoader<SerializeFrom<typeof loader>>({

app/features/tournament-bracket/routes/to.$id.matches.$mid.test.ts

@@ -0,0 +1,242 @@
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import type { adminActionSchema } from "~/features/tournament/actions/to.$id.admin.server";
+import {
+	dbInsertTournament,
+	dbInsertTournamentTeam,
+	dbStartTournament,
+} from "~/features/tournament/tournament-test-utils";
+import type { SerializeFrom } from "~/utils/remix";
+import {
+	assertResponseErrored,
+	dbInsertUsers,
+	dbReset,
+	wrappedAction,
+	wrappedLoader,
+} from "~/utils/Test";
+import { action as adminAction } from "../../tournament/routes/to.$id.admin";
+import type { matchSchema } from "../tournament-bracket-schemas.server";
+import { action, loader } from "./to.$id.matches.$mid";
+
+const tournamentMatchAction = wrappedAction<typeof matchSchema>({
+	action,
+	isJsonSubmission: true,
+});
+const tournamentAdminAction = wrappedAction<typeof adminActionSchema>({
+	action: adminAction,
+	isJsonSubmission: true,
+});
+
+const tournamentMatchLoader = wrappedLoader<SerializeFrom<typeof loader>>({
+	loader,
+});
+
+const loadMatchData = () =>
+	tournamentMatchLoader({
+		params: { id: "1", mid: "1" },
+	});
+
+const reportScoreAction = ({
+	position,
+	params = { id: "1", mid: "1" },
+	winnerTeamId = 1,
+}: {
+	position: number;
+	params?: { id: string; mid: string };
+	winnerTeamId?: number;
+}) =>
+	tournamentMatchAction(
+		{
+			_action: "REPORT_SCORE",
+			position,
+			winnerTeamId,
+		},
+		{ user: "admin", params },
+	);
+
+const setActiveRosterAction = (teamId = 1, roster = [1, 2, 3, 4]) =>
+	tournamentMatchAction(
+		{
+			_action: "SET_ACTIVE_ROSTER",
+			roster: roster,
+			teamId,
+		},
+		{ user: "admin", params: { id: "1", mid: "1" } },
+	);
+
+const removeMemberAction = ({
+	userId,
+	teamId,
+}: {
+	userId: number;
+	teamId: number;
+}) =>
+	tournamentAdminAction(
+		{
+			_action: "REMOVE_MEMBER",
+			memberId: userId,
+			teamId,
+		},
+		{ user: "admin", params: { id: "1" } },
+	);
+
+describe("Tournament match page", () => {
+	beforeEach(async () => {
+		dbInsertUsers(10);
+		await dbInsertTournament();
+		await dbInsertTournamentTeam({
+			membersCount: 6,
+			ownerId: 1,
+		});
+		await dbInsertTournamentTeam({
+			membersCount: 4,
+			ownerId: 7,
+		});
+		await dbStartTournament([1, 2]);
+	});
+
+	afterEach(() => {
+		dbReset();
+	});
+
+	describe("results", () => {
+		it("is empty array for new match", async () => {
+			const data = await loadMatchData();
+
+			expect(data.results).toBeDefined();
+			expect(data.results.length).toBe(0);
+		});
+
+		it("returns results for an in-progress match with correct fields", async () => {
+			await setActiveRosterAction();
+			await reportScoreAction({ position: 0 });
+
+			const data = await loadMatchData();
+
+			expect(data.results.length).toBe(1);
+
+			const result = data.results[0];
+
+			expect(result.stageId).toBe(1);
+			expect(result.mode).toBe("SZ");
+			expect(
+				result.participants.every((participant) =>
+					[1, 2, 3, 4, 7, 8, 9, 10].includes(participant.userId),
+				),
+				"Result participants should only include active roster user ids",
+			).toBeTruthy();
+			expect(result.opponentOnePoints).toBe(null);
+			expect(result.opponentTwoPoints).toBe(null);
+			expect(result.winnerTeamId).toBe(1);
+		});
+
+		it("returns results for a completed match", async () => {
+			await setActiveRosterAction();
+			await reportScoreAction({ position: 0 });
+			await reportScoreAction({ position: 1 });
+
+			const data = await loadMatchData();
+
+			expect(data.results.length).toBe(2);
+		});
+	});
+
+	describe("mapList", () => {
+		it("returns TO picked map list for match", async () => {
+			const data = await loadMatchData();
+
+			expect(data.mapList).toBeDefined();
+			expect(data.mapList?.length).toBe(3);
+			expect(data.mapList?.[0].source).toBe("TO");
+			expect(data.mapList?.[0].mode).toBe("SZ");
+			expect(data.mapList?.[0].stageId).toBe(1);
+		});
+	});
+
+	describe("matchIsOver", () => {
+		it("is false for new match", async () => {
+			const data = await loadMatchData();
+
+			expect(data.matchIsOver).toBe(false);
+		});
+
+		it("is true for a completed match", async () => {
+			await setActiveRosterAction();
+			await reportScoreAction({ position: 0 });
+			await reportScoreAction({ position: 1 });
+
+			const data = await loadMatchData();
+
+			expect(data.matchIsOver).toBe(true);
+		});
+	});
+
+	describe("active roster", () => {
+		it("should return error if submitted active roster contains user id not in the team", async () => {
+			const res = await setActiveRosterAction(1, [1, 2, 3, 7]);
+
+			assertResponseErrored(res, "Invalid roster");
+		});
+
+		it("should return error if submitted active roster is not of correct length", async () => {
+			const res = await setActiveRosterAction(1, [1, 2, 3]);
+
+			assertResponseErrored(res, "Invalid roster length");
+		});
+
+		it("should return error if trying to report score without active roster", async () => {
+			const res = await reportScoreAction({ position: 0 });
+
+			assertResponseErrored(res, "Team one has no active roster");
+		});
+
+		it("should wipe active roster if member in it removed by tournament admin", async () => {
+			await setActiveRosterAction();
+
+			await removeMemberAction({
+				teamId: 1,
+				userId: 2,
+			});
+
+			const res = await reportScoreAction({ position: 0 });
+			assertResponseErrored(res, "Team one has no active roster");
+		});
+
+		it("should retain active roster if member removed by tournament admin was not in it", async () => {
+			await setActiveRosterAction();
+			await removeMemberAction({
+				teamId: 1,
+				userId: 5,
+			});
+
+			const res = await reportScoreAction({ position: 0 });
+
+			expect(res).toBe(null);
+		});
+
+		it("should not require setting active roster if both teams have no subs", async () => {
+			await dbInsertTournament();
+			await dbInsertTournamentTeam({
+				membersCount: 4,
+				ownerId: 1,
+				tournamentId: 2,
+			});
+			await dbInsertTournamentTeam({
+				membersCount: 4,
+				ownerId: 5,
+				tournamentId: 2,
+			});
+			await dbStartTournament([3, 4], 2);
+
+			const res = await reportScoreAction({
+				position: 0,
+				params: {
+					id: "2",
+					mid: "2",
+				},
+				winnerTeamId: 3,
+			});
+
+			expect(res).toBe(null);
+		});
+	});
+});

app/features/tournament/TournamentTeamRepository.server.ts

@@ -13,11 +13,15 @@ export function setActiveRoster({
 	activeRosterUserIds,
 }: {
 	teamId: number;
-	activeRosterUserIds: number[];
+	activeRosterUserIds: number[] | null;
 }) {
 	return db
 		.updateTable("TournamentTeam")
-		.set({ activeRosterUserIds: JSON.stringify(activeRosterUserIds) })
+		.set({
+			activeRosterUserIds: activeRosterUserIds
+				? JSON.stringify(activeRosterUserIds)
+				: null,
+		})
 		.where("TournamentTeam.id", "=", teamId)
 		.execute();
 }
@@ -150,6 +154,8 @@ export function create({
 				inGameName: ownerInGameName,
 			})
 			.execute();
+
+		return tournamentTeam;
 	});
 }
 

app/features/tournament/actions/to.$id.admin.server.ts

@@ -191,6 +191,13 @@ export const action: ActionFunction = async ({ request, params }) => {
 				"Cannot remove player that has participated in the tournament",
 			);
 
+			if (team.activeRosterUserIds?.includes(data.memberId)) {
+				await TournamentTeamRepository.setActiveRoster({
+					teamId: team.id,
+					activeRosterUserIds: null,
+				});
+			}
+
 			leaveTeam({
 				userId: data.memberId,
 				teamId: team.id,

app/features/tournament/tournament-test-utils.ts

@@ -0,0 +1,139 @@
+import * as CalendarRepository from "~/features/calendar/CalendarRepository.server";
+import { databaseTimestampNow } from "~/utils/dates";
+import invariant from "~/utils/invariant";
+import { getServerTournamentManager } from "../tournament-bracket/core/brackets-manager/manager.server";
+import { tournamentFromDB } from "../tournament-bracket/core/Tournament.server";
+import { joinTeam } from "./queries/joinLeaveTeam.server";
+import { updateRoundMaps } from "./queries/updateRoundMaps.server";
+import * as TournamentRepository from "./TournamentRepository.server";
+import * as TournamentTeamRepository from "./TournamentTeamRepository.server";
+
+/**
+ * Creates a mock tournament with one single elimination bracket.
+ */
+export async function dbInsertTournament() {
+	await CalendarRepository.create({
+		isFullTournament: true,
+		authorId: 1,
+		badges: [],
+		bracketUrl: "https://example.com/bracket",
+		description: null,
+		discordInviteCode: "test-discord",
+		deadlines: "DEFAULT",
+		name: "Test Tournament",
+		organizationId: null,
+		rules: null,
+		startTimes: [databaseTimestampNow()],
+		tags: null,
+		bracketProgression: [
+			{
+				name: "Bracket",
+				type: "single_elimination",
+				requiresCheckIn: false,
+				settings: {
+					thirdPlaceMatch: false,
+				},
+			},
+		],
+		mapPickingStyle: "TO",
+		mapPoolMaps: ([1, 2, 3, 4, 5] as const).map((id) => ({
+			mode: "SZ",
+			stageId: id,
+		})),
+	});
+}
+
+/**
+ * Inserts a tournament team into the database with the specified number of members. Also checks in the team to the tournament.
+ */
+export async function dbInsertTournamentTeam({
+	membersCount,
+	ownerId,
+	tournamentId = 1,
+}: {
+	/** Total number of members in the team, including the owner. */
+	membersCount: number;
+	/** Id of the user who owns the team. The other members are relative to this ID so e.g. if captain has ID of 5 then other members have 6,7,8 etc. */
+	ownerId: number;
+	/** Id of the tournament to associate the team with. Defaults to 1. */
+	tournamentId?: number;
+}) {
+	const tournamentTeam = await TournamentTeamRepository.create({
+		ownerInGameName: null,
+		team: {
+			name: `Test Team ${ownerId}`,
+			noScreen: 0,
+			prefersNotToHost: 0,
+			teamId: null,
+		},
+		userId: ownerId,
+		tournamentId,
+	});
+
+	for (let i = 1; i < membersCount; i++) {
+		const memberId = ownerId + i;
+
+		joinTeam({
+			userId: memberId,
+			newTeamId: tournamentTeam.id,
+			tournamentId,
+			inGameName: null,
+		});
+	}
+
+	await TournamentRepository.checkIn({
+		tournamentTeamId: tournamentTeam.id,
+		// no sources = regular check in
+		bracketIdx: null,
+	});
+}
+
+/**
+ * Starts a tournament with the given seeding and tournament ID.
+ * Assumes that the tournament has only one bracket and one round.
+ */
+export async function dbStartTournament(seeding: number[], tournamentId = 1) {
+	const manager = getServerTournamentManager();
+
+	const tournament = await tournamentFromDB({
+		tournamentId,
+		user: undefined,
+	});
+
+	invariant(
+		seeding.every((id) => tournament.ctx.teams.some((team) => team.id === id)),
+	);
+
+	const bracket = tournament.bracketByIdx(0)!;
+
+	const settings = tournament.bracketManagerSettings(
+		bracket.settings,
+		bracket.type,
+		seeding.length,
+	);
+
+	manager.create({
+		tournamentId: tournament.ctx.id,
+		name: bracket.name,
+		type: bracket.type,
+		seeding,
+		settings,
+	});
+
+	// assuming here every tournament has only one round
+	const roundId = tournamentId === 1 ? 1 : 2;
+
+	updateRoundMaps([
+		{
+			count: 3,
+			roundId,
+			type: "BEST_OF",
+			list: ([1, 2, 3] as const).map((stageId) => ({
+				pickBan: false,
+				mode: "SZ",
+				stageId,
+				source: "TO",
+			})),
+		},
+	]);
+}

app/utils/Test.ts

@@ -7,6 +7,7 @@ import { db, sql } from "~/db/sql";
 import { ADMIN_ID } from "~/features/admin/admin-constants";
 import { SESSION_KEY } from "~/features/auth/core/authenticator.server";
 import { authSessionStorage } from "~/features/auth/core/session.server";
+import { logger } from "./logger";
 
 export function arrayContainsSameItems<T>(arr1: T[], arr2: T[]) {
 	return (
@@ -28,11 +29,11 @@ export function arrayContainsSameItems<T>(arr1: T[], arr2: T[]) {
 export function wrappedAction<T extends z.ZodTypeAny>({
 	action,
 	/** Is this action submitted as json (via SendouForm) */
-	isNewForm = false,
+	isJsonSubmission = false,
 }: {
 	// TODO: strongly type this
 	action: (args: ActionFunctionArgs) => any;
-	isNewForm?: boolean;
+	isJsonSubmission?: boolean;
 }) {
 	return async (
 		args: z.infer<T>,
@@ -41,7 +42,7 @@ export function wrappedAction<T extends z.ZodTypeAny>({
 			params = {},
 		}: { user?: "admin" | "regular"; params?: Params<string> } = {},
 	) => {
-		const body = isNewForm
+		const body = isJsonSubmission
 			? JSON.stringify(args)
 			: new URLSearchParams(args as any);
 		const request = new Request("http://app.com/path", {
@@ -51,7 +52,9 @@ export function wrappedAction<T extends z.ZodTypeAny>({
 				...(await authHeader(user)),
 				[
 					"Content-Type",
-					isNewForm ? "application/json" : "application/x-www-form-urlencoded",
+					isJsonSubmission
+						? "application/json"
+						: "application/x-www-form-urlencoded",
 				],
 			],
 		});
@@ -65,6 +68,9 @@ export function wrappedAction<T extends z.ZodTypeAny>({
 
 			return response;
 		} catch (thrown) {
+			// we only log errors in vitest for failed tests so this is okay (more context)
+			logger.error("Error in wrappedAction:", thrown);
+
 			if (thrown instanceof Response) {
 				// it was a redirect
 				if (thrown.status === 302) return thrown;
@@ -123,6 +129,10 @@ export function wrappedLoader<T>({
  * @param message - Optional. The expected error toast message shown to the user.
  */
 export function assertResponseErrored(response: Response, message?: string) {
+	if (!response) {
+		throw new Error(`Expected a Response, got: ${response}`);
+	}
+
 	expect(response.headers.get("Location")).toContain("?__error=");
 	if (message) {
 		expect(response.headers.get("Location")).toContain(message);