From 9bb4362434921b33be7f492d5d8c8ddd79de77e7 Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Fri, 27 Jun 2025 13:48:43 +0300 Subject: [PATCH] Clear active roster if admin removes a member in it Closes #2427 --- app/features/scrims/routes/scrims.new.test.ts | 2 +- .../routes/to.$id.matches.$mid.test.ts | 242 ++++++++++++++++++ .../TournamentTeamRepository.server.ts | 10 +- .../tournament/actions/to.$id.admin.server.ts | 7 + .../tournament/tournament-test-utils.ts | 139 ++++++++++ app/utils/Test.ts | 18 +- 6 files changed, 411 insertions(+), 7 deletions(-) create mode 100644 app/features/tournament-bracket/routes/to.$id.matches.$mid.test.ts create mode 100644 app/features/tournament/tournament-test-utils.ts diff --git a/app/features/scrims/routes/scrims.new.test.ts b/app/features/scrims/routes/scrims.new.test.ts index 28a0bc51f..bd972a243 100644 --- a/app/features/scrims/routes/scrims.new.test.ts +++ b/app/features/scrims/routes/scrims.new.test.ts @@ -13,7 +13,7 @@ import type { scrimsNewActionSchema } from "../scrims-schemas"; const newScrimAction = wrappedAction({ action, - isNewForm: true, + isJsonSubmission: true, }); const scrimPostsLoader = wrappedLoader>({ diff --git a/app/features/tournament-bracket/routes/to.$id.matches.$mid.test.ts b/app/features/tournament-bracket/routes/to.$id.matches.$mid.test.ts new file mode 100644 index 000000000..89231ae60 --- /dev/null +++ b/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({ + action, + isJsonSubmission: true, +}); +const tournamentAdminAction = wrappedAction({ + action: adminAction, + isJsonSubmission: true, +}); + +const tournamentMatchLoader = wrappedLoader>({ + 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); + }); + }); +}); diff --git a/app/features/tournament/TournamentTeamRepository.server.ts b/app/features/tournament/TournamentTeamRepository.server.ts index 9882cef6b..eb1b1074f 100644 --- a/app/features/tournament/TournamentTeamRepository.server.ts +++ b/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; }); } diff --git a/app/features/tournament/actions/to.$id.admin.server.ts b/app/features/tournament/actions/to.$id.admin.server.ts index e20d1b9db..7c0052c6d 100644 --- a/app/features/tournament/actions/to.$id.admin.server.ts +++ b/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, diff --git a/app/features/tournament/tournament-test-utils.ts b/app/features/tournament/tournament-test-utils.ts new file mode 100644 index 000000000..d1785a183 --- /dev/null +++ b/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", + })), + }, + ]); +} diff --git a/app/utils/Test.ts b/app/utils/Test.ts index 5f11881f9..08bff7842 100644 --- a/app/utils/Test.ts +++ b/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(arr1: T[], arr2: T[]) { return ( @@ -28,11 +29,11 @@ export function arrayContainsSameItems(arr1: T[], arr2: T[]) { export function wrappedAction({ 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, @@ -41,7 +42,7 @@ export function wrappedAction({ params = {}, }: { user?: "admin" | "regular"; params?: Params } = {}, ) => { - 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({ ...(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({ 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({ * @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);