From c945ba0152e70f4c90230090af5ea21a17a74610 Mon Sep 17 00:00:00 2001 From: Sabine Maennel <5292683+sabinem@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:27:52 +0100 Subject: [PATCH] refactor(submissions): list a hackathon's submissions in one call Manage Submissions, its export and the Manage Teams lock check asked ListSubmissions once per team. The backend now takes a hackathon_id (#367), so each asks once and groups the result by team. Owners hold submission:read hackathon-wide, so nothing they can see changes. --- .../lib/server/hackathon/submissions.test.ts | 28 +++++++++++ .../src/lib/server/hackathon/submissions.ts | 30 ++++++------ .../hackathon/teamAssignmentLock.test.ts | 47 ++++++++----------- .../server/hackathon/teamAssignmentLock.ts | 20 ++++---- .../[id]/submissions/manage/+page.server.ts | 6 +-- .../[id]/submissions/manage/export/+server.ts | 2 +- .../[id]/teams/manage/+page.server.ts | 6 +-- 7 files changed, 75 insertions(+), 64 deletions(-) diff --git a/components/frontend/src/lib/server/hackathon/submissions.test.ts b/components/frontend/src/lib/server/hackathon/submissions.test.ts index 878d591c..a5bde5a2 100644 --- a/components/frontend/src/lib/server/hackathon/submissions.test.ts +++ b/components/frontend/src/lib/server/hackathon/submissions.test.ts @@ -1,9 +1,11 @@ import { describe, it, expect } from "vitest" import type { Submission } from "$lib/server/grpc/generated/hackathon/entities/submission" import type { Team } from "$lib/server/grpc/generated/hackathon/entities/team" +import type { TeamServiceClient } from "$lib/server/grpc/generated/hackathon/team_service" import type { User } from "$lib/server/grpc/generated/user/entities/user" import { submissionOverview, + submissionsByTeam, submissionStanding, submissionVersions, } from "./submissions" @@ -248,3 +250,29 @@ describe("submissionOverview", () => { expect(rows[0]?.members).toEqual(["Amy", "anon", "Zed"]) }) }) + +describe("submissionsByTeam", () => { + it("asks once for the hackathon and groups by team, keeping the order", async () => { + const asked: unknown[] = [] + const client = { + listSubmissions: async (req: unknown) => { + asked.push(req) + + return { + submissions: [ + sub({ version: 1, teamId: "team-a" }), + sub({ version: 2, teamId: "team-a" }), + sub({ version: 1, teamId: "team-b" }), + ], + } + }, + } as unknown as TeamServiceClient + + const byTeam = await submissionsByTeam(client, "hackathon-1") + + expect(asked).toEqual([{ hackathonId: "hackathon-1" }]) + expect(byTeam.get("team-a")?.map((s) => s.version)).toEqual([1, 2]) + expect(byTeam.get("team-b")?.map((s) => s.version)).toEqual([1]) + expect(byTeam.has("team-c")).toBe(false) + }) +}) diff --git a/components/frontend/src/lib/server/hackathon/submissions.ts b/components/frontend/src/lib/server/hackathon/submissions.ts index 78e2fcde..d0be3f6b 100644 --- a/components/frontend/src/lib/server/hackathon/submissions.ts +++ b/components/frontend/src/lib/server/hackathon/submissions.ts @@ -120,15 +120,12 @@ export interface TeamSubmissionRow extends TeamSubmissions { } /** - * Every team's submissions, keyed by team id. + * Every submission in the hackathon, keyed by team id. A team with none has no + * entry. * - * One `ListSubmissions` per team, in parallel: `TeamService.List` returns teams - * without their submissions (`team_service.go:63-68`), and there is no RPC that - * lists a hackathon's submissions in one go. - * - * TODO(backend: submissions-hackathon-list): one call once the backend offers - * one — either `List` loading the submissions edge that `Get` already loads, or - * a hackathon-scoped `ListSubmissions`. + * One hackathon-scoped `ListSubmissions`. It comes back ordered by team, then by + * version ascending, so each team's list keeps the order a team-scoped call + * would give it. * * `ListSubmissions` rather than the submissions nested in `team.get`, for the * same reason the participant page gives: only this one is meant to be complete. @@ -137,17 +134,18 @@ export interface TeamSubmissionRow extends TeamSubmissions { */ export async function submissionsByTeam( team: TeamServiceClient, - teams: Team[], + hackathonId: string, ): Promise> { - const lists = await Promise.all( - teams.map(async (t) => { - const { submissions } = await team.listSubmissions({ teamId: t.id }) + const { submissions } = await team.listSubmissions({ hackathonId }) - return [t.id, submissions] as const - }), - ) + const byTeam = new Map() + for (const s of submissions) { + const list = byTeam.get(s.teamId) + if (list) list.push(s) + else byTeam.set(s.teamId, [s]) + } - return new Map(lists) + return byTeam } /** diff --git a/components/frontend/src/lib/server/hackathon/teamAssignmentLock.test.ts b/components/frontend/src/lib/server/hackathon/teamAssignmentLock.test.ts index facab551..f0e88132 100644 --- a/components/frontend/src/lib/server/hackathon/teamAssignmentLock.test.ts +++ b/components/frontend/src/lib/server/hackathon/teamAssignmentLock.test.ts @@ -5,20 +5,23 @@ import type { HackathonState } from "$lib/server/grpc/generated/hackathon/entiti import type { TeamServiceClient } from "$lib/server/grpc/generated/hackathon/team_service" import { ClientError, Status } from "nice-grpc-common" -/** A client whose `listSubmissions` answers per team id. Nothing else is called. */ -function client(perTeam: Record): TeamServiceClient { +/** + * A client whose `listSubmissions` answers for hackathon `h1` only. Nothing + * else is called. + */ +function client(count: number | "refuse"): TeamServiceClient { return { - listSubmissions: async ({ teamId }: { teamId: string }) => { - const answer = perTeam[teamId] - if (answer === "refuse") { + listSubmissions: async ({ hackathonId }: { hackathonId?: string }) => { + if (count === "refuse") { throw new ClientError( "/hackathon.TeamService/ListSubmissions", Status.PERMISSION_DENIED, "nope", ) } + const n = hackathonId === "h1" ? count : 0 - return { submissions: Array.from({ length: answer ?? 0 }, () => ({})) } + return { submissions: Array.from({ length: n }, () => ({})) } }, } as unknown as TeamServiceClient } @@ -33,43 +36,31 @@ function state(viewTeams: boolean): HackathonState { describe("assignmentLockReasons", () => { it("is open with no submissions and teams not published", async () => { - expect( - await assignmentLockReasons( - client({ t1: 0, t2: 0 }), - ["t1", "t2"], - state(false), - ), - ).toEqual([]) + expect(await assignmentLockReasons(client(0), "h1", state(false))).toEqual( + [], + ) }) - it("counts submissions across every team", async () => { - expect( - await assignmentLockReasons( - client({ t1: 2, t2: 1 }), - ["t1", "t2"], - state(false), - ), - ).toEqual(["3 submissions exist"]) + it("counts every submission in the hackathon", async () => { + expect(await assignmentLockReasons(client(3), "h1", state(false))).toEqual([ + "3 submissions exist", + ]) }) it("locks on published teams alone", async () => { - expect(await assignmentLockReasons(client({}), [], state(true))).toEqual([ + expect(await assignmentLockReasons(client(0), "h1", state(true))).toEqual([ "teams are published", ]) }) it("locks when submissions cannot be checked, rather than guess open", async () => { expect( - await assignmentLockReasons( - client({ t1: "refuse" }), - ["t1"], - state(false), - ), + await assignmentLockReasons(client("refuse"), "h1", state(false)), ).toEqual(["submissions could not be checked"]) }) it("reads a hackathon with no state row as not published", async () => { - expect(await assignmentLockReasons(client({}), [], undefined)).toEqual([]) + expect(await assignmentLockReasons(client(0), "h1", undefined)).toEqual([]) }) }) diff --git a/components/frontend/src/lib/server/hackathon/teamAssignmentLock.ts b/components/frontend/src/lib/server/hackathon/teamAssignmentLock.ts index 7154f8e4..bdd925f6 100644 --- a/components/frontend/src/lib/server/hackathon/teamAssignmentLock.ts +++ b/components/frontend/src/lib/server/hackathon/teamAssignmentLock.ts @@ -32,26 +32,22 @@ import { enabledCapabilities } from "./phaseForm" */ export async function assignmentLockReasons( team: TeamServiceClient, - teamIds: readonly string[], + hackathonId: string, state: HackathonState | undefined, ): Promise { const published = enabledCapabilities(state).includes( Capability.CAPABILITY_VIEW_TEAMS, ) - // `TeamService.List` does not load submissions, so each team is asked. A - // refusal or failure counts as "cannot tell", which locks: unlocking wrongly - // can delete a submission, locking wrongly costs a reload. + // Every team's submissions in one hackathon-scoped call, not only those on + // approved projects: a submission anywhere in the hackathon is one a save + // could take with it. A refusal or failure counts as "cannot tell", which + // locks: unlocking wrongly can delete a submission, locking wrongly costs a + // reload. let submissions: number | null = 0 try { - const counts = await Promise.all( - teamIds.map(async (teamId) => { - const { submissions } = await team.listSubmissions({ teamId }) - - return submissions.length - }), - ) - submissions = counts.reduce((n, c) => n + c, 0) + submissions = (await team.listSubmissions({ hackathonId })).submissions + .length } catch { submissions = null } diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/+page.server.ts b/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/+page.server.ts index 7a416460..66b5ef83 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/+page.server.ts +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/+page.server.ts @@ -61,15 +61,15 @@ export const load: PageServerLoad = async (event) => { // Every team, whichever tab is open. The tab is applied on the page, for two // reasons: a search has to see every team to say which tab its matches are // on, and a load that never reads `?standing=` is one SvelteKit does not - // re-run when only the tab changes — switching tabs would otherwise repeat a - // `ListSubmissions` per team. + // re-run when only the tab changes — switching tabs would otherwise repeat + // the `ListSubmissions`. return { hackathonId: hackathon.id, submissionsOpen, teamsReadable: true, rows: submissionOverview( teams, - await submissionsByTeam(team, teams), + await submissionsByTeam(team, hackathon.id), projectTitles, names, ), diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/export/+server.ts b/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/export/+server.ts index 8488eb1e..c5d129da 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/export/+server.ts +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/submissions/manage/export/+server.ts @@ -69,7 +69,7 @@ export const GET: RequestHandler = async (event) => { const overview = submissionOverview( teams, - await submissionsByTeam(team, teams), + await submissionsByTeam(team, h.id), new Map(h.projects.map((p) => [p.id, p.title])), namesByUserId(h.members, h.owners), ) diff --git a/components/frontend/src/routes/(app)/my/hackathon/[id]/teams/manage/+page.server.ts b/components/frontend/src/routes/(app)/my/hackathon/[id]/teams/manage/+page.server.ts index 18d1ecac..87d95bec 100644 --- a/components/frontend/src/routes/(app)/my/hackathon/[id]/teams/manage/+page.server.ts +++ b/components/frontend/src/routes/(app)/my/hackathon/[id]/teams/manage/+page.server.ts @@ -36,11 +36,9 @@ export const load: PageServerLoad = async (event) => { const { teams } = await team.list({ hackathonId: event.params.id }) - // Every team, not only those on approved projects: a submission anywhere in - // the hackathon is one a save could take with it. const lockReasons = await assignmentLockReasons( team, - teams.map((t) => t.id), + event.params.id, hackathon.state, ) @@ -224,7 +222,7 @@ export const actions: Actions = { }) const locked = await assignmentLockReasons( team, - all.map((t) => t.id), + event.params.id, latest?.state, )