diff --git a/api/bracket/generator.go b/api/bracket/generator.go index 6bbaf3ad..611c31d2 100644 --- a/api/bracket/generator.go +++ b/api/bracket/generator.go @@ -204,6 +204,9 @@ func GenerateDoubleElimination(entries []SeedEntry) ([]BracketMatch, error) { loserMatchesInFirstRound := losersInRound / 2 var prevLoserMatches []int + // Track the index (into loserMatches) of each match, grouped by losers + // round, so we can wire the internal progression round-by-round below. + var loserRoundIdx [][]int for lRound := 1; lRound <= numLoserRounds; lRound++ { var matchesThisRound int @@ -221,6 +224,7 @@ func GenerateDoubleElimination(entries []SeedEntry) ([]BracketMatch, error) { } var currentLoserMatches []int + var currentRoundIdx []int for i := 0; i < matchesThisRound; i++ { matchNum++ lm := BracketMatch{ @@ -229,31 +233,44 @@ func GenerateDoubleElimination(entries []SeedEntry) ([]BracketMatch, error) { RoundName: fmt.Sprintf("Losers Round %d", lRound), } - // Wire losers bracket matches to next losers bracket match - if lRound < numLoserRounds { - // Will be calculated after all matches are created - } - + currentRoundIdx = append(currentRoundIdx, len(loserMatches)) currentLoserMatches = append(currentLoserMatches, matchNum) loserMatches = append(loserMatches, lm) } prevLoserMatches = currentLoserMatches + loserRoundIdx = append(loserRoundIdx, currentRoundIdx) } - // Wire losers bracket internal progression - allLoserMatches := loserMatches - for i := range allLoserMatches { - if i < len(allLoserMatches)-1 { - // Simple sequential wiring for losers bracket - nextIdx := (i / 2) + len(winnerMatches) + len(allLoserMatches[:i+1]) - if nextIdx < len(winnerMatches)+len(allLoserMatches) { - nextMatchNum := allLoserMatches[nextIdx-len(winnerMatches)].MatchNumber - allLoserMatches[i].NextMatchNumber = nextMatchNum + // Wire losers bracket internal progression round-by-round. Each match in + // losers round r feeds its winner into a match in losers round r+1; the + // last losers round's match is the LB final and is wired to grand finals + // separately below. + // + // Slot convention (chosen so an LB survivor never collides with a winners + // bracket dropout landing in the same destination): + // - When the next round halves the field (minor round: survivors meet + // each other), match i maps to match i/2 with slot from parity. + // - When the next round keeps the same match count (major round: + // survivor meets an incoming winners bracket dropout), match i maps to + // match i and the survivor always takes slot 1, reserving slot 2 for + // the dropout. + for r := 0; r < len(loserRoundIdx)-1; r++ { + curr := loserRoundIdx[r] + next := loserRoundIdx[r+1] + halving := len(next) < len(curr) + for i, idx := range curr { + if halving { + destIdx := next[i/2] + loserMatches[idx].NextMatchNumber = loserMatches[destIdx].MatchNumber if i%2 == 0 { - allLoserMatches[i].NextMatchSlot = 1 + loserMatches[idx].NextMatchSlot = 1 } else { - allLoserMatches[i].NextMatchSlot = 2 + loserMatches[idx].NextMatchSlot = 2 } + } else { + destIdx := next[i] + loserMatches[idx].NextMatchNumber = loserMatches[destIdx].MatchNumber + loserMatches[idx].NextMatchSlot = 1 } } } @@ -286,11 +303,18 @@ func GenerateDoubleElimination(entries []SeedEntry) ([]BracketMatch, error) { wbFinals.NextMatchNumber = matchNum wbFinals.NextMatchSlot = 1 - // Wire LB finals winner to grand finals slot 2 + // Wire grand finals slot 2: normally fed by the LB final winner. With only a + // single winners-bracket round (e.g. exactly 2 teams) there are no losers + // bracket matches, so the WB final's loser is the "losers bracket" and must + // feed grand finals slot 2 directly. Without this, slot 2 can never be + // filled and the event cannot complete. if len(loserMatches) > 0 { lbFinals := &loserMatches[len(loserMatches)-1] lbFinals.NextMatchNumber = matchNum lbFinals.NextMatchSlot = 2 + } else { + wbFinals.LoserNextMatchNumber = matchNum + wbFinals.LoserNextMatchSlot = 2 } // Combine all matches diff --git a/api/bracket/generator_test.go b/api/bracket/generator_test.go index 88ceb8e4..bfadc63e 100644 --- a/api/bracket/generator_test.go +++ b/api/bracket/generator_test.go @@ -1,6 +1,7 @@ package bracket import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -147,6 +148,107 @@ func TestGenerateDoubleElimination_TooFewEntries(t *testing.T) { assert.Error(t, err) } +// TestGenerateDoubleElimination_LosersBracketWiring asserts that the internal +// losers-bracket progression is structurally valid for several bracket sizes: +// every non-final LB match points its winner forward into a later, existing LB +// match in a later round, exactly one LB match feeds grand finals, and no +// destination slot is targeted by more than one source. +func TestGenerateDoubleElimination_LosersBracketWiring(t *testing.T) { + for _, n := range []int{4, 8, 16} { + t.Run(roundsLabel(n), func(t *testing.T) { + matches, err := GenerateDoubleElimination(makeEntries(n)) + require.NoError(t, err) + + byNum := make(map[int]BracketMatch, len(matches)) + for _, m := range matches { + byNum[m.MatchNumber] = m + } + + grandFinals := matches[len(matches)-1] + require.Equal(t, "Grand Finals", grandFinals.RoundName) + + // Collect losers-bracket matches in declared order. + var lbMatches []BracketMatch + for _, m := range matches { + if len(m.RoundName) >= 6 && m.RoundName[:6] == "Losers" { + lbMatches = append(lbMatches, m) + } + } + require.NotEmpty(t, lbMatches, "expected losers bracket matches for n=%d", n) + + // Track (destMatch, slot) targets to detect collisions across both + // the LB-internal winner wiring and the WB-dropout loser wiring. + type target struct{ matchNum, slot int } + seen := make(map[target]int) + record := func(dst, slot int) { + seen[target{dst, slot}]++ + } + + lbFeedingGrandFinals := 0 + for _, m := range lbMatches { + if m.NextMatchNumber == grandFinals.MatchNumber { + lbFeedingGrandFinals++ + record(m.NextMatchNumber, m.NextMatchSlot) + continue + } + // Non-final LB match: must point at a later, existing match in a + // strictly later round. + require.NotZero(t, m.NextMatchNumber, + "LB match %d (%s) has no NextMatchNumber (orphaned)", m.MatchNumber, m.RoundName) + dst, ok := byNum[m.NextMatchNumber] + require.True(t, ok, "LB match %d points at non-existent match %d", m.MatchNumber, m.NextMatchNumber) + assert.Greater(t, dst.MatchNumber, m.MatchNumber, + "LB match %d should feed a later match, got %d", m.MatchNumber, dst.MatchNumber) + assert.Greater(t, dst.Round, m.Round, + "LB match %d should feed a later round", m.MatchNumber) + record(m.NextMatchNumber, m.NextMatchSlot) + } + + assert.Equal(t, 1, lbFeedingGrandFinals, "exactly one LB match should feed grand finals") + + // Include winners-bracket dropout targets so survivor/dropout + // collisions in shared LB matches would be caught. + for _, m := range matches { + if m.LoserNextMatchNumber != 0 { + record(m.LoserNextMatchNumber, m.LoserNextMatchSlot) + } + } + + for tgt, count := range seen { + assert.LessOrEqual(t, count, 1, + "destination match %d slot %d targeted by %d sources (collision)", tgt.matchNum, tgt.slot, count) + } + }) + } +} + +// TestGenerateDoubleElimination_TwoTeams verifies the 2-team edge case: with no +// losers bracket, the winners-final loser must feed grand finals slot 2 so the +// final can be filled and the event can complete. +func TestGenerateDoubleElimination_TwoTeams(t *testing.T) { + matches, err := GenerateDoubleElimination(makeEntries(2)) + require.NoError(t, err) + + grandFinals := matches[len(matches)-1] + require.Equal(t, "Grand Finals", grandFinals.RoundName) + + // Some match must feed grand finals slot 2. + slot2Fed := false + for _, m := range matches { + if m.NextMatchNumber == grandFinals.MatchNumber && m.NextMatchSlot == 2 { + slot2Fed = true + } + if m.LoserNextMatchNumber == grandFinals.MatchNumber && m.LoserNextMatchSlot == 2 { + slot2Fed = true + } + } + assert.True(t, slot2Fed, "grand finals slot 2 must have a feeder for a 2-team double elimination") +} + +func roundsLabel(n int) string { + return fmt.Sprintf("n=%d", n) +} + func TestGenerateRoundRobin_4Teams(t *testing.T) { entries := makeEntries(4) diff --git a/api/config/config.go b/api/config/config.go index 5cf3280e..864e701a 100644 --- a/api/config/config.go +++ b/api/config/config.go @@ -44,9 +44,17 @@ func Load() (*Config, error) { cfg.Env = "development" } - origins := os.Getenv("CORS_ALLOWED_ORIGINS") - if origins != "" { - cfg.CORSAllowedOrigins = strings.Split(origins, ",") + // Normalize origins once at parse time so every consumer (the CORS + // middleware and the WebSocket origin allowlist) shares an identical, + // whitespace-free set. Trimming here makes the parity the ws.Handler doc + // comment claims actually hold even when the env var has spaces after + // commas (e.g. "https://a.com, https://b.com"). + if origins := os.Getenv("CORS_ALLOWED_ORIGINS"); origins != "" { + for _, o := range strings.Split(origins, ",") { + if o = strings.TrimSpace(o); o != "" { + cfg.CORSAllowedOrigins = append(cfg.CORSAllowedOrigins, o) + } + } } return cfg, nil diff --git a/api/db/generated/matches.sql.go b/api/db/generated/matches.sql.go index f2daaf74..ba000836 100644 --- a/api/db/generated/matches.sql.go +++ b/api/db/generated/matches.sql.go @@ -1047,7 +1047,7 @@ func (q *Queries) ListLiveMatches(ctx context.Context, arg ListLiveMatchesParams const listMatchesByCourt = `-- name: ListMatchesByCourt :many SELECT id, public_id, tournament_id, division_id, pod_id, court_id, created_by_user_id, match_type, round, round_name, match_number, team1_id, team2_id, team1_seed, team2_seed, scoring_preset_id, games_per_set, sets_to_win, points_to_win, win_by, max_points, rally_scoring, timeouts_per_game, timeout_duration_sec, freeze_at, team1_score, team2_score, current_set, current_game, serving_team, server_number, set_scores, status, started_at, completed_at, winner_team_id, loser_team_id, win_reason, next_match_id, next_match_slot, loser_next_match_id, loser_next_match_slot, referee_user_id, notes, expires_at, scheduled_at, created_at, updated_at, match_series_id, court_queue_position FROM matches WHERE court_id = $1 -ORDER BY scheduled_at NULLS LAST, created_at +ORDER BY court_queue_position NULLS LAST, scheduled_at NULLS LAST, created_at LIMIT $2 OFFSET $3 ` diff --git a/api/db/generated/registrations.sql.go b/api/db/generated/registrations.sql.go index 149dcc12..a6720e79 100644 --- a/api/db/generated/registrations.sql.go +++ b/api/db/generated/registrations.sql.go @@ -31,6 +31,18 @@ func (q *Queries) BulkUpdateNoShow(ctx context.Context, arg BulkUpdateNoShowPara return err } +const countActiveRegistrationsByDivision = `-- name: CountActiveRegistrationsByDivision :one +SELECT COUNT(*) FROM registrations +WHERE division_id = $1 AND status IN ('approved', 'checked_in') +` + +func (q *Queries) CountActiveRegistrationsByDivision(ctx context.Context, divisionID int64) (int64, error) { + row := q.db.QueryRow(ctx, countActiveRegistrationsByDivision, divisionID) + var count int64 + err := row.Scan(&count) + return count, err +} + const countRegistrationsByDivision = `-- name: CountRegistrationsByDivision :one SELECT COUNT(*) FROM registrations WHERE division_id = $1 ` diff --git a/api/db/generated/tournaments.sql.go b/api/db/generated/tournaments.sql.go index 53f0aca0..e1005722 100644 --- a/api/db/generated/tournaments.sql.go +++ b/api/db/generated/tournaments.sql.go @@ -12,6 +12,18 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +const countPublicTournaments = `-- name: CountPublicTournaments :one +SELECT COUNT(*) FROM tournaments +WHERE deleted_at IS NULL AND status = ANY($1::text[]) +` + +func (q *Queries) CountPublicTournaments(ctx context.Context, dollar_1 []string) (int64, error) { + row := q.db.QueryRow(ctx, countPublicTournaments, dollar_1) + var count int64 + err := row.Scan(&count) + return count, err +} + const countSearchTournaments = `-- name: CountSearchTournaments :one SELECT COUNT(*) FROM tournaments WHERE deleted_at IS NULL @@ -387,6 +399,73 @@ func (q *Queries) GetTournamentsByIDs(ctx context.Context, dollar_1 []int64) ([] return items, nil } +const listPublicTournaments = `-- name: ListPublicTournaments :many +SELECT id, public_id, name, slug, status, start_date, end_date, venue_id, league_id, season_id, description, logo_url, banner_url, contact_email, contact_phone, website_url, registration_open_at, registration_close_at, max_participants, rules_document_url, cancellation_reason, social_links, notes, sponsor_info, show_registrations, created_by_user_id, td_user_id, created_at, updated_at, deleted_at, sport_id FROM tournaments +WHERE deleted_at IS NULL AND status = ANY($1::text[]) +ORDER BY start_date DESC +LIMIT $2 OFFSET $3 +` + +type ListPublicTournamentsParams struct { + Column1 []string `json:"column_1"` + Limit int32 `json:"limit"` + Offset int32 `json:"offset"` +} + +// Public directory listing: filters to publicly-visible statuses in SQL so +// LIMIT/OFFSET and the matching count are computed over the same filtered set. +func (q *Queries) ListPublicTournaments(ctx context.Context, arg ListPublicTournamentsParams) ([]Tournament, error) { + rows, err := q.db.Query(ctx, listPublicTournaments, arg.Column1, arg.Limit, arg.Offset) + if err != nil { + return nil, err + } + defer rows.Close() + items := []Tournament{} + for rows.Next() { + var i Tournament + if err := rows.Scan( + &i.ID, + &i.PublicID, + &i.Name, + &i.Slug, + &i.Status, + &i.StartDate, + &i.EndDate, + &i.VenueID, + &i.LeagueID, + &i.SeasonID, + &i.Description, + &i.LogoUrl, + &i.BannerUrl, + &i.ContactEmail, + &i.ContactPhone, + &i.WebsiteUrl, + &i.RegistrationOpenAt, + &i.RegistrationCloseAt, + &i.MaxParticipants, + &i.RulesDocumentUrl, + &i.CancellationReason, + &i.SocialLinks, + &i.Notes, + &i.SponsorInfo, + &i.ShowRegistrations, + &i.CreatedByUserID, + &i.TdUserID, + &i.CreatedAt, + &i.UpdatedAt, + &i.DeletedAt, + &i.SportID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listTournaments = `-- name: ListTournaments :many SELECT id, public_id, name, slug, status, start_date, end_date, venue_id, league_id, season_id, description, logo_url, banner_url, contact_email, contact_phone, website_url, registration_open_at, registration_close_at, max_participants, rules_document_url, cancellation_reason, social_links, notes, sponsor_info, show_registrations, created_by_user_id, td_user_id, created_at, updated_at, deleted_at, sport_id FROM tournaments WHERE deleted_at IS NULL diff --git a/api/db/migrations/00044_partial_unique_soft_delete.sql b/api/db/migrations/00044_partial_unique_soft_delete.sql new file mode 100644 index 00000000..68839702 --- /dev/null +++ b/api/db/migrations/00044_partial_unique_soft_delete.sql @@ -0,0 +1,78 @@ +-- +goose Up +-- B6: Soft-delete vs. unique-constraint mismatch. +-- +-- Several tables enforce slug/email/name uniqueness with plain table-level +-- UNIQUE constraints that cover ALL rows, including soft-deleted ones +-- (deleted_at IS NOT NULL). The application's pre-insert existence checks +-- (CheckTeamSlugExists, SlugExistsDivision, SlugExistsSeason, +-- SlugExistsTournament, SlugExistsLeague, CheckVenueSlugExists, +-- GetUserByEmail, etc.) all filter `deleted_at IS NULL`, so once a row is +-- soft-deleted the app considers its slug/email/name free, but the still-live +-- UNIQUE constraint rejects the re-insert with a 23505 unique_violation. +-- +-- This migration realigns the DB constraints with the app's view by replacing +-- each blanket UNIQUE with a PARTIAL UNIQUE INDEX scoped to live rows +-- (WHERE deleted_at IS NULL), matching the pattern already used by courts +-- (00006). The auto-generated constraint names below follow Postgres' +-- default `__key` convention; IF EXISTS makes each drop safe. + +-- teams.slug (00003): TEXT NOT NULL UNIQUE -> partial unique index. +ALTER TABLE teams DROP CONSTRAINT IF EXISTS teams_slug_key; +CREATE UNIQUE INDEX teams_slug_uniq ON teams(slug) WHERE deleted_at IS NULL; + +-- venues.slug (00005): TEXT NOT NULL UNIQUE -> partial unique index. +ALTER TABLE venues DROP CONSTRAINT IF EXISTS venues_slug_key; +CREATE UNIQUE INDEX venues_slug_uniq ON venues(slug) WHERE deleted_at IS NULL; + +-- leagues.slug (00007): TEXT NOT NULL UNIQUE -> partial unique index. +ALTER TABLE leagues DROP CONSTRAINT IF EXISTS leagues_slug_key; +CREATE UNIQUE INDEX leagues_slug_uniq ON leagues(slug) WHERE deleted_at IS NULL; + +-- seasons (00008): UNIQUE (league_id, slug) -> partial unique index. +ALTER TABLE seasons DROP CONSTRAINT IF EXISTS seasons_league_id_slug_key; +CREATE UNIQUE INDEX seasons_league_slug_uniq ON seasons(league_id, slug) WHERE deleted_at IS NULL; + +-- tournaments.slug (00010): TEXT NOT NULL UNIQUE -> partial unique index. +ALTER TABLE tournaments DROP CONSTRAINT IF EXISTS tournaments_slug_key; +CREATE UNIQUE INDEX tournaments_slug_uniq ON tournaments(slug) WHERE deleted_at IS NULL; + +-- divisions (00011): UNIQUE (tournament_id, slug) -> partial unique index. +ALTER TABLE divisions DROP CONSTRAINT IF EXISTS divisions_tournament_id_slug_key; +CREATE UNIQUE INDEX divisions_tournament_slug_uniq ON divisions(tournament_id, slug) WHERE deleted_at IS NULL; + +-- pods (00012): UNIQUE (division_id, name) -> partial unique index. +ALTER TABLE pods DROP CONSTRAINT IF EXISTS pods_division_id_name_key; +CREATE UNIQUE INDEX pods_division_name_uniq ON pods(division_id, name) WHERE deleted_at IS NULL; + +-- users.email (00001): TEXT UNIQUE (email is nullable) -> partial unique +-- index scoped to live, non-null emails, matching GetUserByEmail's filter. +ALTER TABLE users DROP CONSTRAINT IF EXISTS users_email_key; +CREATE UNIQUE INDEX users_email_uniq ON users(email) WHERE email IS NOT NULL AND deleted_at IS NULL; + +-- +goose Down +-- Reverse the changes: drop the partial unique indexes and restore the +-- original blanket UNIQUE constraints. + +DROP INDEX IF EXISTS users_email_uniq; +ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email); + +DROP INDEX IF EXISTS pods_division_name_uniq; +ALTER TABLE pods ADD CONSTRAINT pods_division_id_name_key UNIQUE (division_id, name); + +DROP INDEX IF EXISTS divisions_tournament_slug_uniq; +ALTER TABLE divisions ADD CONSTRAINT divisions_tournament_id_slug_key UNIQUE (tournament_id, slug); + +DROP INDEX IF EXISTS tournaments_slug_uniq; +ALTER TABLE tournaments ADD CONSTRAINT tournaments_slug_key UNIQUE (slug); + +DROP INDEX IF EXISTS seasons_league_slug_uniq; +ALTER TABLE seasons ADD CONSTRAINT seasons_league_id_slug_key UNIQUE (league_id, slug); + +DROP INDEX IF EXISTS leagues_slug_uniq; +ALTER TABLE leagues ADD CONSTRAINT leagues_slug_key UNIQUE (slug); + +DROP INDEX IF EXISTS venues_slug_uniq; +ALTER TABLE venues ADD CONSTRAINT venues_slug_key UNIQUE (slug); + +DROP INDEX IF EXISTS teams_slug_uniq; +ALTER TABLE teams ADD CONSTRAINT teams_slug_key UNIQUE (slug); diff --git a/api/db/queries/matches.sql b/api/db/queries/matches.sql index 1ef5ef86..1c9b695c 100644 --- a/api/db/queries/matches.sql +++ b/api/db/queries/matches.sql @@ -136,7 +136,7 @@ LIMIT $2 OFFSET $3; -- name: ListMatchesByCourt :many SELECT * FROM matches WHERE court_id = $1 -ORDER BY scheduled_at NULLS LAST, created_at +ORDER BY court_queue_position NULLS LAST, scheduled_at NULLS LAST, created_at LIMIT $2 OFFSET $3; -- name: ListMatchesByCourtActive :many diff --git a/api/db/queries/registrations.sql b/api/db/queries/registrations.sql index eb31edd4..0e9929aa 100644 --- a/api/db/queries/registrations.sql +++ b/api/db/queries/registrations.sql @@ -22,6 +22,10 @@ SELECT COUNT(*) FROM registrations WHERE division_id = $1; SELECT COUNT(*) FROM registrations WHERE division_id = $1 AND status = $2; +-- name: CountActiveRegistrationsByDivision :one +SELECT COUNT(*) FROM registrations +WHERE division_id = $1 AND status IN ('approved', 'checked_in'); + -- name: ListRegistrationsByDivisionAndStatus :many SELECT * FROM registrations WHERE division_id = $1 AND status = $2 diff --git a/api/db/queries/tournaments.sql b/api/db/queries/tournaments.sql index 41c9b1d1..31bb21c9 100644 --- a/api/db/queries/tournaments.sql +++ b/api/db/queries/tournaments.sql @@ -122,6 +122,18 @@ LIMIT $2 OFFSET $3; SELECT COUNT(*) FROM tournaments WHERE status = $1 AND deleted_at IS NULL; +-- name: ListPublicTournaments :many +-- Public directory listing: filters to publicly-visible statuses in SQL so +-- LIMIT/OFFSET and the matching count are computed over the same filtered set. +SELECT * FROM tournaments +WHERE deleted_at IS NULL AND status = ANY($1::text[]) +ORDER BY start_date DESC +LIMIT $2 OFFSET $3; + +-- name: CountPublicTournaments :one +SELECT COUNT(*) FROM tournaments +WHERE deleted_at IS NULL AND status = ANY($1::text[]); + -- name: SearchTournamentsByStatus :many SELECT * FROM tournaments WHERE deleted_at IS NULL diff --git a/api/engine/config.go b/api/engine/config.go index 55272be0..45e91466 100644 --- a/api/engine/config.go +++ b/api/engine/config.go @@ -95,11 +95,11 @@ func (c ScoringConfig) IsEndChange(team1Score, team2Score, currentGameNum int32, } midpoint := (c.PointsToWin + 1) / 2 - leading := team1Score - if team2Score > leading { - leading = team2Score - } - // Trigger exactly once when the leading score first reaches the midpoint. - return leading == midpoint && (team1Score == midpoint || team2Score == midpoint) + // Fire exactly once: only when one team newly reaches the midpoint while the + // other is still strictly below it. Because scores increment by exactly one + // point at a time, the scoring team always lands precisely on the midpoint, + // so this fires at 6-5 (or 5-6) but NOT again at 6-6, 7-6, etc. + return (team1Score == midpoint && team2Score < midpoint) || + (team2Score == midpoint && team1Score < midpoint) } diff --git a/api/engine/config_test.go b/api/engine/config_test.go index 8859948d..8298ff1a 100644 --- a/api/engine/config_test.go +++ b/api/engine/config_test.go @@ -100,3 +100,20 @@ func TestIsEndChange_SingleGame(t *testing.T) { assert.True(t, cfg.IsEndChange(6, 3, 1, 0)) assert.False(t, cfg.IsEndChange(5, 3, 1, 0)) } + +func TestIsEndChange_DoesNotFireTwiceInDeuce(t *testing.T) { + cfg := ScoringConfig{PointsToWin: 11, WinBy: 2, GamesPerSet: 1, SetsToWin: 1} + // Deciding game is game 1. Midpoint = 6. + + // First team to reach the midpoint triggers the end change exactly once. + assert.True(t, cfg.IsEndChange(6, 5, 1, 0), "should fire when team1 first reaches midpoint") + assert.True(t, cfg.IsEndChange(5, 6, 1, 0), "should fire when team2 first reaches midpoint") + + // Once the trailing team also reaches the midpoint, the end change must NOT + // fire a second time (the previous implementation re-fired at 6-6). + assert.False(t, cfg.IsEndChange(6, 6, 1, 0), "must NOT fire again at 6-6") + + // Past the midpoint, no fire. + assert.False(t, cfg.IsEndChange(7, 6, 1, 0), "past the midpoint: no fire") + assert.False(t, cfg.IsEndChange(7, 5, 1, 0), "leader past midpoint: no fire") +} diff --git a/api/engine/scoring.go b/api/engine/scoring.go index 5a059791..33eb9015 100644 --- a/api/engine/scoring.go +++ b/api/engine/scoring.go @@ -122,11 +122,18 @@ func (e *ScoringEngine) Point(state MatchState, scoringTeam int32) EngineResult Winner: winner, }) - // Check match over. - team1Wins, team2Wins := countWins(state.CompletedGames) - gamesToWin := e.config.GamesToWin() + // Check match over. The match is decided by SET wins, not raw game + // wins: CompletedGames accumulates every game of the entire match, so we + // segment it into sets (each won by the first team to GamesToWin() games) + // and only finish when a team reaches SetsToWin sets. With SetsToWin==1 + // this reduces to "first team to GamesToWin() games wins the match". + team1Sets, team2Sets := e.countSetWins(state.CompletedGames) + setsToWin := e.config.SetsToWin + if setsToWin < 1 { + setsToWin = 1 + } - if team1Wins >= gamesToWin || team2Wins >= gamesToWin { + if team1Sets >= setsToWin || team2Sets >= setsToWin { result.MatchOverDetected = true } @@ -293,3 +300,36 @@ func countWins(games []GameResult) (int32, int32) { } return t1, t2 } + +// countSetWins segments the match's completed games into sets and tallies set +// wins per team. games is the full, ordered list of completed games for the +// whole match (CompletedGames is never reset per set). Within each set the +// first team to GamesToWin() game wins takes the set; the in-set tally then +// resets for the next set. This makes match completion respect SetsToWin +// rather than counting raw game wins globally. +func (e *ScoringEngine) countSetWins(games []GameResult) (int32, int32) { + gamesToWin := e.config.GamesToWin() + if gamesToWin < 1 { + gamesToWin = 1 + } + + var setWins1, setWins2 int32 + var gameWins1, gameWins2 int32 + for _, g := range games { + switch g.Winner { + case 1: + gameWins1++ + case 2: + gameWins2++ + } + + if gameWins1 >= gamesToWin { + setWins1++ + gameWins1, gameWins2 = 0, 0 + } else if gameWins2 >= gamesToWin { + setWins2++ + gameWins1, gameWins2 = 0, 0 + } + } + return setWins1, setWins2 +} diff --git a/api/engine/scoring_test.go b/api/engine/scoring_test.go index 26c7327a..eb23295d 100644 --- a/api/engine/scoring_test.go +++ b/api/engine/scoring_test.go @@ -204,6 +204,38 @@ func TestMatchNotOver_BestOf3_OneWinEach(t *testing.T) { assert.False(t, result.MatchOverDetected, "1-1 in games, need game 3") } +func TestMatchOver_SetsToWin_RequiresMultipleSets(t *testing.T) { + // Best-of-3 SETS where each set is a single game (GamesPerSet=1 -> 1 game per set). + // A team must win 2 sets (i.e. 2 games) to win the match; winning the first + // set/game must NOT end the match. + cfg, _ := ParseScoringConfig(SideOutScoring, 11, 2, 0, 1, 2, 0) + eng := NewScoringEngine(cfg) + + // Win the first set (game 1). + state := defaultState() + state.TeamOneScore = 10 + state.TeamTwoScore = 5 + + result := eng.Point(state, 1) // 11-5, team 1 wins game 1 + require.False(t, result.IsError) + assert.True(t, result.GameOverDetected) + assert.False(t, result.MatchOverDetected, "winning only the first of two required sets must not end the match") + + // Win the second set (game 2) -> 2 sets -> match over. + state2 := defaultState() + state2.CompletedGames = []GameResult{ + {GameNum: 1, TeamOneScore: 11, TeamTwoScore: 5, Winner: 1}, + } + state2.CurrentGameNum = 2 + state2.TeamOneScore = 10 + state2.TeamTwoScore = 8 + + result2 := eng.Point(state2, 1) // 11-8, team 1 wins game 2 -> 2 sets + require.False(t, result2.IsError) + assert.True(t, result2.GameOverDetected) + assert.True(t, result2.MatchOverDetected, "winning both required sets must end the match") +} + // --- End change detection --- func TestEndChangeDetected(t *testing.T) { diff --git a/api/handler/auth.go b/api/handler/auth.go index 4d189e2d..924eb3e3 100644 --- a/api/handler/auth.go +++ b/api/handler/auth.go @@ -3,10 +3,12 @@ package handler import ( "errors" + "log/slog" "net/http" "time" "github.com/court-command/court-command/auth" + "github.com/court-command/court-command/middleware" "github.com/court-command/court-command/service" "github.com/court-command/court-command/session" ) @@ -15,13 +17,24 @@ import ( type AuthHandler struct { authService *service.AuthService secureCookie bool + // orgRoles resolves a user's org roles via the Logto Management API + // when the JWT lacks the organization_roles claim. Mirrors the + // resolver JWTSession uses so MeJWT can elevate platform_admins the + // same way protected handlers do. May be nil (tests / dev without + // Logto Mgmt creds): the resolver path is then skipped. + orgRoles middleware.OrgRoleResolver } // NewAuthHandler creates a new AuthHandler. -func NewAuthHandler(authService *service.AuthService, secureCookie bool) *AuthHandler { +// +// orgRoles may be nil; when nil, MeJWT relies solely on the JWT fast +// path for role elevation (the same posture JWTSession takes with a nil +// resolver). +func NewAuthHandler(authService *service.AuthService, secureCookie bool, orgRoles middleware.OrgRoleResolver) *AuthHandler { return &AuthHandler{ authService: authService, secureCookie: secureCookie, + orgRoles: orgRoles, } } @@ -181,6 +194,40 @@ func (h *AuthHandler) MeJWT(w http.ResponseWriter, r *http.Request) { user.Role = elevated } + // Management API fallback: in production Logto does NOT populate the + // organization_roles claim on API-resource access tokens, so the JWT + // fast path above never fires and user.Role stays at the local DB + // default ('player'). This mirrors JWTSession's Path 2 + // (api/middleware/jwt_session.go): when the token carries an + // organization_id and the user isn't already platform_admin, ask the + // Logto Management API for the user's org roles and elevate. Without + // this, /auth/me reports 'player' for a genuine platform_admin and the + // SPA hides the admin UI even though the backend RequirePlatformAdmin + // gate (which runs under JWTSession's resolver) would authorize them. + if h.orgRoles != nil && + user.Role != "platform_admin" && + claims.OrganizationID != "" { + roles, lookupErr := h.orgRoles.GetUserOrganizationRoles( + r.Context(), claims.OrganizationID, claims.Subject) + if lookupErr != nil { + // Don't fail the request -- the user may legitimately be a + // non-admin, and a Logto Mgmt API hiccup shouldn't break + // /auth/me. Log so ops can see degraded elevation behavior. + slog.WarnContext(r.Context(), + "logto org-role lookup failed in /auth/me; falling back to local DB role", + "user_id", claims.Subject, + "org_id", claims.OrganizationID, + "error", lookupErr) + } else { + for _, role := range roles { + if role == "platform_admin" { + user.Role = "platform_admin" + break + } + } + } + } + resp := &MeResponse{UserResponse: user} // Impersonation signal: the act claim carries the impersonating admin's diff --git a/api/handler/court.go b/api/handler/court.go index 0bd9f769..54010c59 100644 --- a/api/handler/court.go +++ b/api/handler/court.go @@ -293,6 +293,19 @@ func (h *CourtHandler) UpdateCourt(w http.ResponseWriter, r *http.Request) { return } + // Court-ownership guard — mirrors the overlay config write handlers so a + // caller cannot modify a court outside their org/tournament. Platform + // admins, the court creator, and managers of the court's owning venue pass. + ok, err := h.venueService.CanManageCourt(r.Context(), courtID, sess.UserID, sess.Role) + if err != nil { + handleServiceError(w, err) + return + } + if !ok { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "You do not have permission to manage this court") + return + } + var body struct { Name *string `json:"name"` SurfaceType *string `json:"surface_type"` @@ -361,6 +374,19 @@ func (h *CourtHandler) DeleteCourt(w http.ResponseWriter, r *http.Request) { return } + // Court-ownership guard — mirrors the overlay config write handlers so a + // caller cannot delete a court outside their org/tournament. Platform + // admins, the court creator, and managers of the court's owning venue pass. + ok, err := h.venueService.CanManageCourt(r.Context(), courtID, sess.UserID, sess.Role) + if err != nil { + handleServiceError(w, err) + return + } + if !ok { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "You do not have permission to manage this court") + return + } + if err := h.venueService.DeleteCourt(r.Context(), courtID); err != nil { handleServiceError(w, err) return diff --git a/api/handler/overlay.go b/api/handler/overlay.go index 9bdc81f1..1e3da70e 100644 --- a/api/handler/overlay.go +++ b/api/handler/overlay.go @@ -49,6 +49,36 @@ func (h *OverlayHandler) requireSession(w http.ResponseWriter, r *http.Request) return sess } +// requireCourtManage authenticates the request and verifies the caller may +// manage the target court's overlay configuration. On success it returns the +// resolved courtID and true. On any failure it writes the appropriate error +// response (401/400/403/404) and returns false, so callers can simply +// `if courtID, ok := h.requireCourtManage(w, r); !ok { return }`. +func (h *OverlayHandler) requireCourtManage(w http.ResponseWriter, r *http.Request) (int64, bool) { + sess := h.requireSession(w, r) + if sess == nil { + return 0, false + } + + courtID, err := h.parseCourtID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + return 0, false + } + + ok, err := h.overlayService.CanManageCourt(r.Context(), courtID, sess.UserID, sess.Role) + if err != nil { + HandleServiceError(w, err) + return 0, false + } + if !ok { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "You do not have permission to manage this court's overlay") + return 0, false + } + + return courtID, true +} + // ResolveCourtSlug handles GET /api/v1/overlay/court/{courtID}/resolve // Public endpoint — resolves a court slug (or numeric ID) to its canonical // court_id and slug. Used by the frontend to map URL slugs to numeric IDs @@ -105,13 +135,8 @@ func (h *OverlayHandler) GetOverlayData(w http.ResponseWriter, r *http.Request) // GetConfig handles GET /api/v1/overlay/court/{courtID}/config func (h *OverlayHandler) GetConfig(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -126,13 +151,8 @@ func (h *OverlayHandler) GetConfig(w http.ResponseWriter, r *http.Request) { // UpdateTheme handles PUT /api/v1/overlay/court/{courtID}/config/theme func (h *OverlayHandler) UpdateTheme(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -161,13 +181,8 @@ func (h *OverlayHandler) UpdateTheme(w http.ResponseWriter, r *http.Request) { // UpdateElements handles PUT /api/v1/overlay/court/{courtID}/config/elements func (h *OverlayHandler) UpdateElements(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -190,13 +205,8 @@ func (h *OverlayHandler) UpdateElements(w http.ResponseWriter, r *http.Request) // GenerateToken handles POST /api/v1/overlay/court/{courtID}/config/token/generate func (h *OverlayHandler) GenerateToken(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -211,13 +221,8 @@ func (h *OverlayHandler) GenerateToken(w http.ResponseWriter, r *http.Request) { // RevokeToken handles DELETE /api/v1/overlay/court/{courtID}/config/token func (h *OverlayHandler) RevokeToken(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -232,13 +237,13 @@ func (h *OverlayHandler) RevokeToken(w http.ResponseWriter, r *http.Request) { // SetSourceProfile handles PUT /api/v1/overlay/court/{courtID}/config/source-profile func (h *OverlayHandler) SetSourceProfile(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { + sess := h.requireSession(w, r) + if sess == nil { return } - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -250,6 +255,23 @@ func (h *OverlayHandler) SetSourceProfile(w http.ResponseWriter, r *http.Request return } + // When linking a profile (not clearing), verify the caller owns the + // referenced source profile (or is a platform admin) — mirroring the + // ownership gate the dedicated source-profile handlers enforce. Without + // this, any court manager could wire their court to a profile they do + // not own. + if req.SourceProfileID != nil { + profile, err := h.sourceProfileService.GetByID(r.Context(), *req.SourceProfileID) + if err != nil { + WriteError(w, http.StatusNotFound, "NOT_FOUND", "Source profile not found") + return + } + if profile.CreatedByUserID != sess.UserID && sess.Role != "platform_admin" { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Access denied") + return + } + } + config, err := h.overlayService.SetSourceProfile(r.Context(), courtID, req.SourceProfileID) if err != nil { WriteError(w, http.StatusInternalServerError, "UPDATE_FAILED", err.Error()) @@ -263,13 +285,8 @@ func (h *OverlayHandler) SetSourceProfile(w http.ResponseWriter, r *http.Request // Allows Broadcast Operators to override any canonical overlay field per-court // without modifying the underlying tournament/team/match data. Authenticated. func (h *OverlayHandler) UpdateDataOverrides(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } @@ -293,13 +310,8 @@ func (h *OverlayHandler) UpdateDataOverrides(w http.ResponseWriter, r *http.Requ // ClearDataOverrides handles DELETE /api/v1/overlay/court/{courtID}/config/data-overrides // Resets all per-court data overrides to empty. Authenticated. func (h *OverlayHandler) ClearDataOverrides(w http.ResponseWriter, r *http.Request) { - if sess := h.requireSession(w, r); sess == nil { - return - } - - courtID, err := h.parseCourtID(r) - if err != nil { - WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid court ID") + courtID, ok := h.requireCourtManage(w, r) + if !ok { return } diff --git a/api/handler/public.go b/api/handler/public.go index 2ea38e57..a7e7fe6d 100644 --- a/api/handler/public.go +++ b/api/handler/public.go @@ -193,15 +193,27 @@ func parseLimitOffset(r *http.Request, defaultLimit, maxLimit int32) (int32, int return int32(limit), int32(offset) } -// publicTournamentStatuses are the only statuses visible on the public directory. -var publicTournamentStatuses = map[string]bool{ - "published": true, - "registration_open": true, - "registration_closed": true, - "in_progress": true, - "completed": true, +// publicTournamentStatusList is the canonical ordered list of statuses visible +// on the public directory. It is the single source of truth; the lookup map +// below is derived from it so the SQL filter and the validation map can never +// drift apart. +var publicTournamentStatusList = []string{ + "published", + "registration_open", + "registration_closed", + "in_progress", + "completed", } +// publicTournamentStatuses are the only statuses visible on the public directory. +var publicTournamentStatuses = func() map[string]bool { + m := make(map[string]bool, len(publicTournamentStatusList)) + for _, s := range publicTournamentStatusList { + m[s] = true + } + return m +}() + // ListTournaments handles GET /api/v1/public/tournaments // Filterable by status query param (only publicly visible statuses allowed). func (h *PublicHandler) ListTournaments(w http.ResponseWriter, r *http.Request) { @@ -215,6 +227,7 @@ func (h *PublicHandler) ListTournaments(w http.ResponseWriter, r *http.Request) } var tournaments []generated.Tournament + var total int64 var err error if status != "" { @@ -223,25 +236,21 @@ func (h *PublicHandler) ListTournaments(w http.ResponseWriter, r *http.Request) Limit: limit, Offset: offset, }) + if err == nil { + total, err = h.queries.CountTournamentsByStatus(r.Context(), status) + } } else { - // No filter: fetch all tournaments and keep only publicly visible ones. - // Use a generous limit to compensate for post-filter reduction. - all, fetchErr := h.queries.ListTournaments(r.Context(), generated.ListTournamentsParams{ - Limit: limit + 50, - Offset: offset, + // No status filter: filter to publicly-visible statuses in SQL so + // LIMIT/OFFSET and the total are computed over the same set. This + // keeps page boundaries aligned (no skipped/duplicated rows) and the + // reported total matching the visible public-only items. + tournaments, err = h.queries.ListPublicTournaments(r.Context(), generated.ListPublicTournamentsParams{ + Column1: publicTournamentStatusList, + Limit: limit, + Offset: offset, }) - err = fetchErr if err == nil { - tournaments = make([]generated.Tournament, 0, len(all)) - for _, t := range all { - if publicTournamentStatuses[t.Status] { - tournaments = append(tournaments, t) - } - } - // Respect the original limit after filtering - if int32(len(tournaments)) > limit { - tournaments = tournaments[:limit] - } + total, err = h.queries.CountPublicTournaments(r.Context(), publicTournamentStatusList) } } @@ -254,7 +263,6 @@ func (h *PublicHandler) ListTournaments(w http.ResponseWriter, r *http.Request) tournaments = []generated.Tournament{} } - total, _ := h.queries.CountTournaments(r.Context()) Paginated(w, toPublicTournaments(tournaments), total, int(limit), int(offset)) } diff --git a/api/handler/registration.go b/api/handler/registration.go index c1a05b77..1cbffe15 100644 --- a/api/handler/registration.go +++ b/api/handler/registration.go @@ -24,17 +24,15 @@ func NewRegistrationHandler(svc *service.RegistrationService) *RegistrationHandl return &RegistrationHandler{regSvc: svc} } -// Routes returns a chi.Router with all registration routes mounted. -// Expects to be mounted under /divisions/{divisionID}/registrations. +// Routes returns a chi.Router with the mutating registration routes. Expects +// to be mounted under /divisions/{divisionID}/registrations behind the +// authenticated + sport-isolation middleware chain (see router.go); each +// handler additionally enforces role checks. The public read routes +// (ListRegistrations, ListSeekingPartner, GetRegistration) are registered +// directly on the parent node in router.go so they stay unauthenticated. func (h *RegistrationHandler) Routes() chi.Router { r := chi.NewRouter() - // Public routes - r.Get("/", h.ListRegistrations) - r.Get("/seeking-partner", h.ListSeekingPartner) - r.Get("/{registrationID}", h.GetRegistration) - - // Authenticated routes r.Post("/", h.Register) r.Patch("/{registrationID}/status", h.UpdateStatus) r.Patch("/{registrationID}/seed", h.UpdateSeed) @@ -52,6 +50,35 @@ func parseDivisionID(r *http.Request) (int64, error) { return strconv.ParseInt(chi.URLParam(r, "divisionID"), 10, 64) } +// isRegistrationStaff reports whether the current request is made by a +// privileged staff member who may see staff-only registration fields +// (admin_notes). The registration read routes are public, so unauthenticated +// callers and plain players never receive admin_notes; only platform admins and +// tournament directors do. +func isRegistrationStaff(r *http.Request) bool { + sess := session.SessionData(r.Context()) + if sess == nil { + return false + } + switch sess.Role { + case "platform_admin", "tournament_director": + return true + default: + return false + } +} + +// canManageRegistrations reports whether the given role may mutate +// registrations (status, seed, placement, check-in, withdraw, admin notes). +func canManageRegistrations(role string) bool { + switch role { + case "platform_admin", "tournament_director": + return true + default: + return false + } +} + // Register creates a new registration. func (h *RegistrationHandler) Register(w http.ResponseWriter, r *http.Request) { sess := session.SessionData(r.Context()) @@ -113,7 +140,7 @@ func (h *RegistrationHandler) GetRegistration(w http.ResponseWriter, r *http.Req return } - reg, err := h.regSvc.GetByID(r.Context(), regID) + reg, err := h.regSvc.GetByID(r.Context(), regID, isRegistrationStaff(r)) if err != nil { HandleServiceError(w, err) return @@ -131,10 +158,11 @@ func (h *RegistrationHandler) ListRegistrations(w http.ResponseWriter, r *http.R } limit, offset := parsePagination(r) + includeStaff := isRegistrationStaff(r) // Check for optional status filter if status := r.URL.Query().Get("status"); status != "" { - regs, total, err := h.regSvc.ListByDivisionAndStatus(r.Context(), divisionID, status, limit, offset) + regs, total, err := h.regSvc.ListByDivisionAndStatus(r.Context(), divisionID, status, limit, offset, includeStaff) if err != nil { HandleServiceError(w, err) return @@ -143,7 +171,7 @@ func (h *RegistrationHandler) ListRegistrations(w http.ResponseWriter, r *http.R return } - regs, total, err := h.regSvc.ListByDivision(r.Context(), divisionID, limit, offset) + regs, total, err := h.regSvc.ListByDivision(r.Context(), divisionID, limit, offset, includeStaff) if err != nil { HandleServiceError(w, err) return @@ -159,6 +187,16 @@ func (h *RegistrationHandler) UpdateStatus(w http.ResponseWriter, r *http.Reques WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } + + divisionID, err := parseDivisionID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid division ID") + return + } regID, err := strconv.ParseInt(chi.URLParam(r, "registrationID"), 10, 64) if err != nil { @@ -180,7 +218,7 @@ func (h *RegistrationHandler) UpdateStatus(w http.ResponseWriter, r *http.Reques return } - reg, err := h.regSvc.UpdateStatus(r.Context(), regID, body.Status) + reg, err := h.regSvc.UpdateStatus(r.Context(), divisionID, regID, body.Status) if err != nil { HandleServiceError(w, err) return @@ -196,6 +234,16 @@ func (h *RegistrationHandler) UpdateSeed(w http.ResponseWriter, r *http.Request) WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } + + divisionID, err := parseDivisionID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid division ID") + return + } regID, err := strconv.ParseInt(chi.URLParam(r, "registrationID"), 10, 64) if err != nil { @@ -214,7 +262,7 @@ func (h *RegistrationHandler) UpdateSeed(w http.ResponseWriter, r *http.Request) seed := pgtype.Int4{Int32: body.Seed, Valid: true} - reg, err := h.regSvc.UpdateSeed(r.Context(), regID, seed) + reg, err := h.regSvc.UpdateSeed(r.Context(), divisionID, regID, seed) if err != nil { HandleServiceError(w, err) return @@ -230,6 +278,16 @@ func (h *RegistrationHandler) UpdatePlacement(w http.ResponseWriter, r *http.Req WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } + + divisionID, err := parseDivisionID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid division ID") + return + } regID, err := strconv.ParseInt(chi.URLParam(r, "registrationID"), 10, 64) if err != nil { @@ -248,7 +306,7 @@ func (h *RegistrationHandler) UpdatePlacement(w http.ResponseWriter, r *http.Req placement := pgtype.Int4{Int32: body.Placement, Valid: true} - reg, err := h.regSvc.UpdatePlacement(r.Context(), regID, placement) + reg, err := h.regSvc.UpdatePlacement(r.Context(), divisionID, regID, placement) if err != nil { HandleServiceError(w, err) return @@ -264,6 +322,16 @@ func (h *RegistrationHandler) CheckIn(w http.ResponseWriter, r *http.Request) { WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } + + divisionID, err := parseDivisionID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid division ID") + return + } regID, err := strconv.ParseInt(chi.URLParam(r, "registrationID"), 10, 64) if err != nil { @@ -271,7 +339,7 @@ func (h *RegistrationHandler) CheckIn(w http.ResponseWriter, r *http.Request) { return } - reg, err := h.regSvc.CheckIn(r.Context(), regID) + reg, err := h.regSvc.CheckIn(r.Context(), divisionID, regID) if err != nil { HandleServiceError(w, err) return @@ -287,6 +355,16 @@ func (h *RegistrationHandler) WithdrawMidTournament(w http.ResponseWriter, r *ht WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } + + divisionID, err := parseDivisionID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid division ID") + return + } regID, err := strconv.ParseInt(chi.URLParam(r, "registrationID"), 10, 64) if err != nil { @@ -294,7 +372,7 @@ func (h *RegistrationHandler) WithdrawMidTournament(w http.ResponseWriter, r *ht return } - reg, err := h.regSvc.WithdrawMidTournament(r.Context(), regID) + reg, err := h.regSvc.WithdrawMidTournament(r.Context(), divisionID, regID) if err != nil { HandleServiceError(w, err) return @@ -312,6 +390,10 @@ func (h *RegistrationHandler) BulkNoShow(w http.ResponseWriter, r *http.Request) WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } divisionID, err := parseDivisionID(r) if err != nil { @@ -359,6 +441,16 @@ func (h *RegistrationHandler) UpdateAdminNotes(w http.ResponseWriter, r *http.Re WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageRegistrations(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage registrations") + return + } + + divisionID, err := parseDivisionID(r) + if err != nil { + WriteError(w, http.StatusBadRequest, "INVALID_ID", "Invalid division ID") + return + } regID, err := strconv.ParseInt(chi.URLParam(r, "registrationID"), 10, 64) if err != nil { @@ -374,7 +466,7 @@ func (h *RegistrationHandler) UpdateAdminNotes(w http.ResponseWriter, r *http.Re return } - reg, err := h.regSvc.UpdateAdminNotes(r.Context(), regID, body.AdminNotes) + reg, err := h.regSvc.UpdateAdminNotes(r.Context(), divisionID, regID, body.AdminNotes) if err != nil { HandleServiceError(w, err) return diff --git a/api/handler/scoring_preset.go b/api/handler/scoring_preset.go index cc6252c5..1c349201 100644 --- a/api/handler/scoring_preset.go +++ b/api/handler/scoring_preset.go @@ -22,6 +22,19 @@ func NewScoringPresetHandler(service *service.ScoringPresetService) *ScoringPres return &ScoringPresetHandler{service: service} } +// canManageScoringPresets reports whether the given role may create or modify +// scoring presets. Presets are a global, cross-tenant pool, so writes are +// restricted to privileged operational roles; plain players (and any other +// authenticated role) get read-only access via List/Get. +func canManageScoringPresets(role string) bool { + switch role { + case "platform_admin", "tournament_director": + return true + default: + return false + } +} + // Routes returns a chi.Router with all scoring preset routes mounted. func (h *ScoringPresetHandler) Routes() chi.Router { r := chi.NewRouter() @@ -62,6 +75,10 @@ func (h *ScoringPresetHandler) Create(w http.ResponseWriter, r *http.Request) { WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageScoringPresets(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage scoring presets") + return + } var body struct { Name string `json:"name"` @@ -160,6 +177,10 @@ func (h *ScoringPresetHandler) Update(w http.ResponseWriter, r *http.Request) { WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageScoringPresets(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage scoring presets") + return + } id, err := strconv.ParseInt(chi.URLParam(r, "presetID"), 10, 64) if err != nil { @@ -236,6 +257,10 @@ func (h *ScoringPresetHandler) Deactivate(w http.ResponseWriter, r *http.Request WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageScoringPresets(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage scoring presets") + return + } id, err := strconv.ParseInt(chi.URLParam(r, "presetID"), 10, 64) if err != nil { @@ -258,6 +283,10 @@ func (h *ScoringPresetHandler) Activate(w http.ResponseWriter, r *http.Request) WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "Not authenticated") return } + if !canManageScoringPresets(sess.Role) { + WriteError(w, http.StatusForbidden, "FORBIDDEN", "Only platform admins or tournament directors can manage scoring presets") + return + } id, err := strconv.ParseInt(chi.URLParam(r, "presetID"), 10, 64) if err != nil { diff --git a/api/main.go b/api/main.go index bb50d59b..6e32ae4d 100644 --- a/api/main.go +++ b/api/main.go @@ -99,7 +99,9 @@ func main() { // Phase 1+2 handlers secureCookie := !cfg.IsDevelopment() - authHandler := handler.NewAuthHandler(authService, secureCookie) + // authHandler is constructed later (after orgRoleResolver) so MeJWT + // can share the Management-API role resolver JWTSession uses; see the + // NewAuthHandler call below the orgRoleResolver block. healthHandler := handler.NewHealthHandler(pool, sessionStore.Client()) playerHandler := handler.NewPlayerHandler(playerService) teamHandler := handler.NewTeamHandler(teamService) @@ -288,6 +290,12 @@ func main() { logtoClient, sessionStore.Client(), middleware.OrgRolesCacheTTLFromEnv()) } + // AuthHandler takes the resolver so MeJWT (GET /api/v1/auth/me) elevates + // platform_admins via the Logto Management API the same way JWTSession + // does for the protected route groups. Constructed here, after the + // resolver, rather than with the other Phase 1+2 handlers above. + authHandler := handler.NewAuthHandler(authService, secureCookie, orgRoleResolver) + // Auto-bootstrap the Logto tenant on every boot. This calls the // same idempotent provisioning logic as the api/cmd/logto-seed CLI: // - registers the API resource + 12 scopes diff --git a/api/middleware/optional_jwt.go b/api/middleware/optional_jwt.go index 62f68227..ce783995 100644 --- a/api/middleware/optional_jwt.go +++ b/api/middleware/optional_jwt.go @@ -47,13 +47,17 @@ import ( // valid JWT if present. Pass-through on every failure path so anonymous // requests survive. // -// Args mirror JWTSession (validator + Logto client + queries + userSync) -// because the populate path is identical to the bridge. +// Args mirror JWTSession (validator + Logto client + queries + userSync +// + orgRoles) because the populate path is identical to the bridge. +// +// orgRoles can be nil -- in that case only the JWT fast path runs (the +// same posture JWTSession takes with a nil resolver). func OptionalJWT( validator *auth.Validator, client LogtoUserFetcher, queries JWTSessionQueries, userSync UserSyncer, + orgRoles OrgRoleResolver, ) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -141,9 +145,38 @@ func OptionalJWT( // the local users.role column which defaults to 'player' for // freshly-mirrored users; we override here when claims show an // elevated org role. + // + // Path 1: JWT fast path. Free if it works; no-op otherwise. if elevated := claims.ElevatedRole(); elevated != "" && elevated != data.Role { data.Role = elevated } + + // Path 2: Management API path -- mirror JWTSession exactly. + // Logto does NOT populate organization_roles on API-resource + // access tokens, so in production Path 1 never fires and + // data.Role stays at the local default ('player'). Without + // this, mixed-auth admin-only writes (e.g. /standings recompute + // /override, /tournaments/{id}/staff) wrongly 403 a genuine + // platform_admin whose session.Data comes solely from this + // global middleware. Permissive on resolver error: pass through + // with the local role rather than failing a possibly-anonymous + // request. + if orgRoles != nil && + data.Role != "platform_admin" && + claims.OrganizationID != "" { + roles, lookupErr := orgRoles.GetUserOrganizationRoles( + r.Context(), claims.OrganizationID, sub) + if lookupErr != nil { + slog.WarnContext(r.Context(), + "optional-jwt org-role lookup failed; falling back to local DB role", + "user_id", sub, + "org_id", claims.OrganizationID, + "err", lookupErr) + } else if containsRole(roles, "platform_admin") { + data.Role = "platform_admin" + } + } + ctx := session.SetSessionData(r.Context(), data) next.ServeHTTP(w, r.WithContext(ctx)) }) diff --git a/api/middleware/optional_jwt_test.go b/api/middleware/optional_jwt_test.go index dd6ba13d..36a383a0 100644 --- a/api/middleware/optional_jwt_test.go +++ b/api/middleware/optional_jwt_test.go @@ -34,7 +34,7 @@ func optionalJWTRunner( authHeader string, ) (sess *session.Data, reached bool, status int) { t.Helper() - mw := middleware.OptionalJWT(v, fetcher, queries, syncer) + mw := middleware.OptionalJWT(v, fetcher, queries, syncer, nil) h := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { reached = true sess = session.SessionData(r.Context()) @@ -170,7 +170,7 @@ func TestOptionalJWT_PreExistingSession_PreservesIt(t *testing.T) { preData := &session.Data{UserID: 999, Role: "player", PublicID: "CC-99999"} mw := middleware.OptionalJWT(v, &fakeFetcher{}, - &fakeQueries{user: makeUser()}, &fakeSyncer{}) + &fakeQueries{user: makeUser()}, &fakeSyncer{}, nil) var observed *session.Data h := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { observed = session.SessionData(r.Context()) diff --git a/api/middleware/rate_limit.go b/api/middleware/rate_limit.go index 2351c51f..3d0d34b0 100644 --- a/api/middleware/rate_limit.go +++ b/api/middleware/rate_limit.go @@ -72,14 +72,20 @@ func (rl *RateLimiter) allow(key string) bool { return true } - // Refill tokens based on elapsed time + // Refill tokens based on whole intervals elapsed; preserve the remainder. + // Advancing lastSeen unconditionally to now() would discard the + // sub-interval fractional time, so a client polling more often than once + // per interval would never accrue a full interval and could never refill. elapsed := time.Since(v.lastSeen) - refill := int(elapsed/rl.interval) * rl.rate - v.tokens += refill - if v.tokens > rl.burst { - v.tokens = rl.burst + if intervals := int(elapsed / rl.interval); intervals > 0 { + v.tokens += intervals * rl.rate + if v.tokens > rl.burst { + v.tokens = rl.burst + } + // Advance lastSeen only by the whole intervals consumed, so the + // leftover fractional time still counts toward the next refill. + v.lastSeen = v.lastSeen.Add(time.Duration(intervals) * rl.interval) } - v.lastSeen = time.Now() if v.tokens <= 0 { return false diff --git a/api/router/router.go b/api/router/router.go index daa41dae..a31e1344 100644 --- a/api/router/router.go +++ b/api/router/router.go @@ -219,7 +219,7 @@ func New(cfg *Config) chi.Router { // this, the SPA's JWT can never reach those handlers. if cfg.JWTValidator != nil && cfg.LogtoClient != nil && cfg.UserSyncService != nil && cfg.Queries != nil { r.Use(middleware.OptionalJWT( - cfg.JWTValidator, cfg.LogtoClient, cfg.Queries, cfg.UserSyncService)) + cfg.JWTValidator, cfg.LogtoClient, cfg.Queries, cfg.UserSyncService, cfg.OrgRoles)) } // API v1 routes @@ -384,9 +384,29 @@ func New(cfg *Config) chi.Router { r.Mount("/", cfg.AnnouncementHandler.FlatAnnouncementRoutes()) }) - // Division sub-resources (registrations and pods — auth checked by handlers) + // Division sub-resources (registrations and pods). + // + // Registration reads are intentionally public; the mutating routes + // (status, seed, placement, check-in, withdraw, admin-notes, create, + // bulk-no-show) must sit behind the same auth + sport-isolation chain + // as other authed write groups so RequireSportMatchesJWT enforces + // org/sport isolation (the service-layer division scoping alone does + // not run the isolation middleware). Chi allows only one Mount per + // path, so the public reads are mounted on this node and the authed + // mutations use a Group — mirroring the /matches split above. r.Route("/divisions/{divisionID}/registrations", func(r chi.Router) { - r.Mount("/", cfg.RegistrationHandler.Routes()) + // Public reads (no auth) — registered directly like /matches so + // only the authed subtree uses Mount (chi allows one Mount per + // path). + r.Get("/", cfg.RegistrationHandler.ListRegistrations) + r.Get("/seeking-partner", cfg.RegistrationHandler.ListSeekingPartner) + r.Get("/{registrationID}", cfg.RegistrationHandler.GetRegistration) + + // Authenticated mutations (auth + sport isolation). + r.Group(func(r chi.Router) { + useAuth(r, cfg) + r.Mount("/", cfg.RegistrationHandler.Routes()) + }) }) r.Route("/divisions/{divisionID}/pods", func(r chi.Router) { r.Mount("/", cfg.PodHandler.Routes()) diff --git a/api/service/api_key.go b/api/service/api_key.go index e853f1b6..3a24b4fe 100644 --- a/api/service/api_key.go +++ b/api/service/api_key.go @@ -159,6 +159,23 @@ func (s *ApiKeyService) ValidateApiKey(ctx context.Context, rawKey string) (*gen return nil, NewValidation("API key has expired") } + // Reject keys belonging to non-active or soft-deleted users. The key's + // own is_active flag is checked in the query (GetApiKeyByHash), but a + // suspend/ban only wipes the user's sessions -- it does NOT deactivate + // their API keys -- so without this check a banned/suspended user keeps + // a working authenticated identity through any key created before the + // ban. Mirrors the status gate the password Login path enforces. + user, err := s.queries.GetUserByID(ctx, apiKey.UserID) + if err != nil { + return nil, NewNotFound("invalid API key") + } + if user.DeletedAt.Valid { + return nil, NewNotFound("invalid API key") + } + if user.Status != "active" { + return nil, NewValidationf("account is %s", user.Status) + } + // Update last used (fire and forget) _ = s.queries.UpdateApiKeyLastUsed(ctx, apiKey.ID) diff --git a/api/service/events.go b/api/service/events.go index 573ed90a..25792b06 100644 --- a/api/service/events.go +++ b/api/service/events.go @@ -88,3 +88,39 @@ var AllEventTypes = []string{ EventTypeFault, EventTypeLineCall, } + +// scoreMutatingEventTypes are the event types that change a match's +// score/serving state. These MUST be applied through the scoring engine +// (ScorePoint, SideOut, RemovePoint, etc.) so the engine remains the single +// source of truth for scoring; they are rejected on the generic RecordEvent +// annotation endpoint. +var scoreMutatingEventTypes = map[string]bool{ + EventTypePointTeam1: true, + EventTypePointTeam2: true, + EventTypePointRemoved: true, + EventTypeSideOut: true, + EventTypeUndo: true, + EventTypeConfirmGameOver: true, + EventTypeConfirmMatchOver: true, +} + +// validEventTypes is the set of every valid event_type value, derived from +// AllEventTypes for O(1) membership checks. +var validEventTypes = func() map[string]bool { + m := make(map[string]bool, len(AllEventTypes)) + for _, t := range AllEventTypes { + m[t] = true + } + return m +}() + +// IsValidEventType reports whether t is a recognized event_type. +func IsValidEventType(t string) bool { + return validEventTypes[t] +} + +// IsScoreMutatingEventType reports whether t changes scoring/serving state and +// therefore must be routed through the scoring engine rather than RecordEvent. +func IsScoreMutatingEventType(t string) bool { + return scoreMutatingEventTypes[t] +} diff --git a/api/service/match.go b/api/service/match.go index db8dff60..17c94ea7 100644 --- a/api/service/match.go +++ b/api/service/match.go @@ -1026,9 +1026,29 @@ func (s *MatchService) StartMatch(ctx context.Context, matchID int64, userID int }, nil } -// RecordEvent records a scoring event and updates the match score. -// Uses a transaction to ensure atomicity. +// RecordEvent records a non-score-mutating timeline annotation (e.g. let, fault, +// line_call, timeout). Uses a transaction to ensure atomicity. +// +// Score-mutating event types (points, side-outs, undo, game/match confirmation) +// must NOT be recorded here: they have to flow through the scoring engine +// (ScorePoint, SideOut, RemovePoint, ConfirmGameOver, ...) so the engine stays +// the single source of truth. Previously this method re-implemented scoring +// inline with logic that diverged from the engine (e.g. flipping the serving +// team on every side-out instead of only on the second server's loss, and no +// game-over detection), corrupting live match state. Such types are now +// rejected, and unknown event types are rejected as well. func (s *MatchService) RecordEvent(ctx context.Context, matchID int64, eventType string, payload json.RawMessage, userID int64) (MatchEventResponse, error) { + if !IsValidEventType(eventType) { + return MatchEventResponse{}, &ValidationError{ + Message: fmt.Sprintf("unknown event_type %q", eventType), + } + } + if IsScoreMutatingEventType(eventType) { + return MatchEventResponse{}, &ValidationError{ + Message: fmt.Sprintf("event_type %q changes the score and must be submitted through the scoring action endpoints (e.g. /point, /sideout, /remove-point, /undo, /confirm-game, /confirm-match), not the events endpoint", eventType), + } + } + tx, err := s.pool.Begin(ctx) if err != nil { return MatchEventResponse{}, fmt.Errorf("failed to begin transaction: %w", err) @@ -1060,7 +1080,8 @@ func (s *MatchService) RecordEvent(ctx context.Context, matchID int64, eventType payload = json.RawMessage("{}") } - // Snapshot BEFORE this event + // Snapshot the current state with the annotation. Score/serving state is + // left unchanged — this endpoint records annotations only. event, err := qtx.CreateMatchEvent(ctx, generated.CreateMatchEventParams{ MatchID: matchID, SequenceID: nextSeq, @@ -1079,51 +1100,6 @@ func (s *MatchService) RecordEvent(ctx context.Context, matchID int64, eventType return MatchEventResponse{}, fmt.Errorf("failed to create event: %w", err) } - // Apply the event to update match score - newT1 := match.Team1Score - newT2 := match.Team2Score - newServingTeam := match.ServingTeam - newServerNumber := match.ServerNumber - - switch eventType { - case "point_team1": - newT1++ - if match.RallyScoring { - newServingTeam = pgtype.Int4{Int32: 1, Valid: true} - } - case "point_team2": - newT2++ - if match.RallyScoring { - newServingTeam = pgtype.Int4{Int32: 2, Valid: true} - } - case "side_out": - if newServingTeam.Valid && newServingTeam.Int32 == 1 { - newServingTeam = pgtype.Int4{Int32: 2, Valid: true} - } else { - newServingTeam = pgtype.Int4{Int32: 1, Valid: true} - } - // Toggle server number for doubles - if newServerNumber.Valid && newServerNumber.Int32 == 1 { - newServerNumber = pgtype.Int4{Int32: 2, Valid: true} - } else if newServerNumber.Valid { - newServerNumber = pgtype.Int4{Int32: 1, Valid: true} - } - } - - _, err = qtx.UpdateMatchScoring(ctx, generated.UpdateMatchScoringParams{ - ID: matchID, - Team1Score: newT1, - Team2Score: newT2, - CurrentSet: match.CurrentSet, - CurrentGame: match.CurrentGame, - ServingTeam: newServingTeam, - ServerNumber: newServerNumber, - SetScores: match.SetScores, - }) - if err != nil { - return MatchEventResponse{}, fmt.Errorf("failed to update match scoring: %w", err) - } - if err := tx.Commit(ctx); err != nil { return MatchEventResponse{}, fmt.Errorf("failed to commit event: %w", err) } @@ -1491,23 +1467,35 @@ func matchToScoringConfig(m generated.Match) engine.ScoringConfig { return cfg } -// applyEngineResult writes the engine result back to the database in a transaction. -// It updates the match scoring state and records an event with the current snapshot. +// applyEngineResult computes and persists a scoring action in a single +// transaction. +// +// The engine computation is supplied as `compute` and is run AGAINST THE +// ROW-LOCKED match (GetMatchForUpdate) inside the transaction, not against an +// earlier unlocked read. This closes the lost-update race: the value written +// always derives from the row read under FOR UPDATE, so two concurrent or +// rapid-repeat scoring requests can no longer both compute N+1 from the same +// stale N and clobber each other. // -// Returns the raw generated.Match, the MatchEvent, and the enriched -// MatchResponse. Enrichment runs exactly once (per CR-4). Callers MUST use -// the returned MatchResponse instead of calling enrichedMatchResponse again. +// The event row stores the PRE-action snapshot (the locked match's state before +// `compute` is applied), matching RecordEvent's convention. This is what Undo +// relies on when it restores from the latest event. +// +// Returns the raw generated.Match, the engine instance (config-bound, useful for +// ScoreCall), the EngineResult, the MatchEvent, and the enriched MatchResponse. +// Enrichment runs exactly once (per CR-4). Callers MUST use the returned +// MatchResponse instead of calling enrichedMatchResponse again. func (s *MatchService) applyEngineResult( ctx context.Context, matchID int64, - result engine.EngineResult, + compute func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult, eventType string, payload json.RawMessage, userID int64, -) (generated.Match, generated.MatchEvent, MatchResponse, error) { +) (generated.Match, *engine.ScoringEngine, engine.EngineResult, generated.MatchEvent, MatchResponse, error) { tx, err := s.pool.Begin(ctx) if err != nil { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to begin transaction: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to begin transaction: %w", err) } defer tx.Rollback(ctx) @@ -1516,9 +1504,17 @@ func (s *MatchService) applyEngineResult( match, err := qtx.GetMatchForUpdate(ctx, matchID) if err != nil { if errors.Is(err, pgx.ErrNoRows) { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, &NotFoundError{Message: "match not found"} + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, &NotFoundError{Message: "match not found"} } - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("get match for engine result: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("get match for engine result: %w", err) + } + + // Run the engine against the row-locked match so the persisted value derives + // from the locked read (lost-update protection). + eng := engine.NewScoringEngine(matchToScoringConfig(match)) + result := compute(eng, matchToEngineState(match)) + if result.IsError { + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, &ValidationError{Message: result.ErrorMessage} } // Encode completed games as set_scores JSON. @@ -1554,7 +1550,7 @@ func (s *MatchService) applyEngineResult( SetScores: setScores, }) if err != nil { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to update match scoring: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to update match scoring: %w", err) } // If status changed, update it. @@ -1564,71 +1560,60 @@ func (s *MatchService) applyEngineResult( Status: newStatus, }) if err != nil { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to update match status: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to update match status: %w", err) } } // Record event. nextSeq, err := qtx.GetNextSequenceID(ctx, matchID) if err != nil { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to get next sequence: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to get next sequence: %w", err) } if payload == nil { payload = json.RawMessage("{}") } + // Snapshot the state BEFORE this action (the row read under lock), so the + // snapshot semantics match RecordEvent and Undo can correctly restore the + // prior state from the latest event. event, err := qtx.CreateMatchEvent(ctx, generated.CreateMatchEventParams{ MatchID: matchID, SequenceID: nextSeq, EventType: eventType, - Team1Score: result.State.TeamOneScore, - Team2Score: result.State.TeamTwoScore, + Team1Score: match.Team1Score, + Team2Score: match.Team2Score, CurrentSet: match.CurrentSet, - CurrentGame: result.State.CurrentGameNum, - ServingTeam: pgtype.Int4{Int32: result.State.ServingTeam, Valid: true}, - ServerNumber: pgtype.Int4{Int32: result.State.ServerNumber, Valid: true}, - SetScores: setScores, + CurrentGame: match.CurrentGame, + ServingTeam: match.ServingTeam, + ServerNumber: match.ServerNumber, + SetScores: match.SetScores, Payload: payload, CreatedByUserID: pgtype.Int8{Int64: userID, Valid: true}, }) if err != nil { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to create event: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to create event: %w", err) } if err := tx.Commit(ctx); err != nil { - return generated.Match{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to commit: %w", err) + return generated.Match{}, nil, engine.EngineResult{}, generated.MatchEvent{}, MatchResponse{}, fmt.Errorf("failed to commit: %w", err) } // Enrich ONCE (CR-4). broadcast + HTTP response share this value. resp := s.enrichedMatchResponse(ctx, updated) s.broadcastMatchUpdate(ctx, updated, resp) - return updated, event, resp, nil + return updated, eng, result, event, resp, nil } // ScorePoint awards a point to the given team via the scoring engine. func (s *MatchService) ScorePoint(ctx context.Context, matchID int64, team int32, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for score point: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.Point(state, team) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - payload, _ := json.Marshal(map[string]interface{}{"team": team}) eventType := fmt.Sprintf("point_team%d", team) - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, eventType, payload, userID) + _, eng, result, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.Point(state, team) + }, eventType, payload, userID) if err != nil { return ScoringActionResult{}, err } @@ -1645,24 +1630,10 @@ func (s *MatchService) ScorePoint(ctx context.Context, matchID int64, team int32 // SideOut handles a side-out (loss of serve). func (s *MatchService) SideOut(ctx context.Context, matchID int64, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for side out: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.SideOut(state) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeSideOut, nil, userID) + _, eng, result, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.SideOut(state) + }, EventTypeSideOut, nil, userID) if err != nil { return ScoringActionResult{}, err } @@ -1676,26 +1647,12 @@ func (s *MatchService) SideOut(ctx context.Context, matchID int64, userID int64) // RemovePoint removes the last point scored for a team. func (s *MatchService) RemovePoint(ctx context.Context, matchID int64, team int32, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for remove point: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - // Use current serving state as previous (simplified undo — full undo via Undo endpoint). - result := eng.RemovePoint(state, team, state.ServingTeam, state.ServerNumber) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - payload, _ := json.Marshal(map[string]interface{}{"team": team}) - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypePointRemoved, payload, userID) + _, eng, result, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + // Use current serving state as previous (simplified undo — full undo via Undo endpoint). + return eng.RemovePoint(state, team, state.ServingTeam, state.ServerNumber) + }, EventTypePointRemoved, payload, userID) if err != nil { return ScoringActionResult{}, err } @@ -1709,24 +1666,10 @@ func (s *MatchService) RemovePoint(ctx context.Context, matchID int64, team int3 // ConfirmGameOver transitions to the next game after a game win is detected. func (s *MatchService) ConfirmGameOver(ctx context.Context, matchID int64, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for confirm game over: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.ConfirmGameOver(state) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeConfirmGameOver, nil, userID) + _, eng, result, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.ConfirmGameOver(state) + }, EventTypeConfirmGameOver, nil, userID) if err != nil { return ScoringActionResult{}, err } @@ -1740,24 +1683,10 @@ func (s *MatchService) ConfirmGameOver(ctx context.Context, matchID int64, userI // ConfirmMatchOver finalizes the match as completed. func (s *MatchService) ConfirmMatchOver(ctx context.Context, matchID int64, winnerTeamID, loserTeamID int64, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for confirm match over: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.ConfirmMatchOver(state) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - - updated, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeConfirmMatchOver, nil, userID) + updated, _, _, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.ConfirmMatchOver(state) + }, EventTypeConfirmMatchOver, nil, userID) if err != nil { return ScoringActionResult{}, err } @@ -1827,25 +1756,11 @@ func (s *MatchService) ConfirmMatchOver(ctx context.Context, matchID int64, winn // CallTimeout records a timeout event. func (s *MatchService) CallTimeout(ctx context.Context, matchID int64, team int32, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for timeout: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.Timeout(state, team) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - payload, _ := json.Marshal(map[string]interface{}{"team": team}) - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeTimeout, payload, userID) + _, eng, result, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.Timeout(state, team) + }, EventTypeTimeout, payload, userID) if err != nil { return ScoringActionResult{}, err } @@ -1859,24 +1774,10 @@ func (s *MatchService) CallTimeout(ctx context.Context, matchID int64, team int3 // PauseMatch pauses the match. func (s *MatchService) PauseMatch(ctx context.Context, matchID int64, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for pause: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.Pause(state) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeMatchPaused, nil, userID) + _, _, _, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.Pause(state) + }, EventTypeMatchPaused, nil, userID) if err != nil { return ScoringActionResult{}, err } @@ -1889,24 +1790,10 @@ func (s *MatchService) PauseMatch(ctx context.Context, matchID int64, userID int // ResumeMatch resumes a paused match. func (s *MatchService) ResumeMatch(ctx context.Context, matchID int64, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for resume: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.Resume(state) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeMatchResumed, nil, userID) + _, _, _, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.Resume(state) + }, EventTypeMatchResumed, nil, userID) if err != nil { return ScoringActionResult{}, err } @@ -1921,29 +1808,15 @@ func (s *MatchService) ResumeMatch(ctx context.Context, matchID int64, userID in // optional free text that's recorded in the forfeit_declared event payload // for audit purposes; an empty string is allowed. func (s *MatchService) DeclareForfeit(ctx context.Context, matchID int64, forfeitingTeam int32, winnerTeamID, loserTeamID int64, reason string, userID int64) (ScoringActionResult, error) { - match, err := s.queries.GetMatch(ctx, matchID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return ScoringActionResult{}, &NotFoundError{Message: "match not found"} - } - return ScoringActionResult{}, fmt.Errorf("get match for forfeit: %w", err) - } - - cfg := matchToScoringConfig(match) - eng := engine.NewScoringEngine(cfg) - state := matchToEngineState(match) - - result := eng.Forfeit(state, forfeitingTeam) - if result.IsError { - return ScoringActionResult{}, &ValidationError{Message: result.ErrorMessage} - } - payloadMap := map[string]interface{}{"forfeiting_team": forfeitingTeam} if reason != "" { payloadMap["reason"] = reason } payload, _ := json.Marshal(payloadMap) - _, event, resp, err := s.applyEngineResult(ctx, matchID, result, EventTypeForfeitDeclared, payload, userID) + _, _, _, event, resp, err := s.applyEngineResult(ctx, matchID, + func(eng *engine.ScoringEngine, state engine.MatchState) engine.EngineResult { + return eng.Forfeit(state, forfeitingTeam) + }, EventTypeForfeitDeclared, payload, userID) if err != nil { return ScoringActionResult{}, err } diff --git a/api/service/match_contract_test.go b/api/service/match_contract_test.go index 8d4ffa52..a1f88a69 100644 --- a/api/service/match_contract_test.go +++ b/api/service/match_contract_test.go @@ -321,8 +321,11 @@ func keysOf(m map[string]json.RawMessage) []string { // --------------------------------------------------------------------------- // TestApplyEngineResult_ReturnsPreEnrichedResponse asserts that the private -// scoring helper returns FOUR values, the third being a MatchResponse. This -// is the mechanical guarantee that every mutation enriches exactly once. +// scoring helper returns a MatchResponse. This is the mechanical guarantee +// that every mutation enriches exactly once. The helper also returns the +// engine instance and EngineResult (it now runs the engine against the +// row-locked match to avoid lost updates), so callers can read scoring flags +// and build the score call. // // Reflection cannot see unexported methods on MatchService without an // instance, so we verify by re-parsing the AST and confirming the return @@ -356,7 +359,7 @@ func TestApplyEngineResult_ReturnsPreEnrichedResponse(t *testing.T) { names = append(names, exprString(field.Type)) } } - want := []string{"generated.Match", "generated.MatchEvent", "MatchResponse", "error"} + want := []string{"generated.Match", "*engine.ScoringEngine", "engine.EngineResult", "generated.MatchEvent", "MatchResponse", "error"} if !reflect.DeepEqual(names, want) { t.Errorf("applyEngineResult return types = %v, want %v", names, want) } diff --git a/api/service/match_series.go b/api/service/match_series.go index c3b2ed68..28ae6060 100644 --- a/api/service/match_series.go +++ b/api/service/match_series.go @@ -3,9 +3,11 @@ package service import ( "context" "encoding/json" + "errors" "fmt" "time" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" @@ -413,18 +415,56 @@ func (s *MatchSeriesService) RecordMatchResult(ctx context.Context, seriesID int return MatchSeriesResponse{}, &ValidationError{Message: "series must be in_progress to record results"} } - // Determine which team won and update wins - team1Wins := series.Team1Wins - team2Wins := series.Team2Wins + // Validate the child match: it must exist, belong to THIS series, and be + // completed. Without this, the matchID parameter was ignored entirely, + // letting a caller record a result for a foreign or never-played match. + childMatch, err := qtx.GetMatchForUpdate(ctx, matchID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return MatchSeriesResponse{}, &ValidationError{Message: "match not found"} + } + return MatchSeriesResponse{}, fmt.Errorf("get child match for series result: %w", err) + } + if !childMatch.MatchSeriesID.Valid || childMatch.MatchSeriesID.Int64 != seriesID { + return MatchSeriesResponse{}, &ValidationError{Message: "match does not belong to this series"} + } + if childMatch.Status != "completed" { + return MatchSeriesResponse{}, &ValidationError{Message: "child match must be completed to record its result"} + } - if series.Team1ID.Valid && winnerTeamID == series.Team1ID.Int64 { - team1Wins++ - } else if series.Team2ID.Valid && winnerTeamID == series.Team2ID.Int64 { - team2Wins++ - } else { + // Derive the winner from the child match's own recorded winner rather than + // trusting the client-supplied winnerTeamID. The winner must still be one of + // the series teams. + if !childMatch.WinnerTeamID.Valid { + return MatchSeriesResponse{}, &ValidationError{Message: "child match has no recorded winner"} + } + derivedWinner := childMatch.WinnerTeamID.Int64 + if (!series.Team1ID.Valid || derivedWinner != series.Team1ID.Int64) && + (!series.Team2ID.Valid || derivedWinner != series.Team2ID.Int64) { return MatchSeriesResponse{}, &ValidationError{Message: "winner must be one of the series teams"} } + // Reconcile win counts by counting completed, series-linked child matches + // per winning team. This is idempotent: replaying the same result (or a + // previously counted match) cannot inflate a team's wins, because we recount + // from the authoritative set of completed matches each time. + childMatches, err := qtx.ListMatchesBySeriesID(ctx, pgtype.Int8{Int64: seriesID, Valid: true}) + if err != nil { + return MatchSeriesResponse{}, fmt.Errorf("list series child matches: %w", err) + } + var team1Wins, team2Wins int32 + for _, cm := range childMatches { + if cm.Status != "completed" || !cm.WinnerTeamID.Valid { + continue + } + switch { + case series.Team1ID.Valid && cm.WinnerTeamID.Int64 == series.Team1ID.Int64: + team1Wins++ + case series.Team2ID.Valid && cm.WinnerTeamID.Int64 == series.Team2ID.Int64: + team2Wins++ + } + } + _, err = qtx.UpdateMatchSeriesScore(ctx, generated.UpdateMatchSeriesScoreParams{ ID: seriesID, Team1Wins: team1Wins, diff --git a/api/service/overlay.go b/api/service/overlay.go index 557fea59..5865cda3 100644 --- a/api/service/overlay.go +++ b/api/service/overlay.go @@ -304,6 +304,47 @@ func (s *OverlayService) GetCourtSlug(ctx context.Context, courtID int64) (strin return slug, nil } +// CanManageCourt reports whether the given user may manage the overlay +// configuration for a court. Returns true for platform admins, the court's +// creator, or a manager of the court's owning venue. Mirrors +// VenueService.CanManageVenue so the same ownership boundary that gates venue +// and court mutations also gates overlay control-panel writes. Returns +// NotFoundError when the court does not exist. +func (s *OverlayService) CanManageCourt(ctx context.Context, courtID, userID int64, userRole string) (bool, error) { + if userRole == "platform_admin" { + return true, nil + } + + court, err := s.queries.GetCourtByID(ctx, courtID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return false, &NotFoundError{Message: "court not found"} + } + return false, fmt.Errorf("get court for authorization: %w", err) + } + + // The court creator may always manage it. + if court.CreatedByUserID.Valid && court.CreatedByUserID.Int64 == userID { + return true, nil + } + + // Otherwise, the caller must be a manager of the court's owning venue. + if court.VenueID.Valid { + isMgr, err := s.queries.IsVenueManager(ctx, generated.IsVenueManagerParams{ + VenueID: court.VenueID.Int64, + UserID: userID, + }) + if err != nil { + return false, fmt.Errorf("check venue manager for court authorization: %w", err) + } + if isMgr { + return true, nil + } + } + + return false, nil +} + func generateSecureToken() (string, error) { bytes := make([]byte, 32) if _, err := rand.Read(bytes); err != nil { diff --git a/api/service/registration.go b/api/service/registration.go index 6c2aa888..608ac2e7 100644 --- a/api/service/registration.go +++ b/api/service/registration.go @@ -41,17 +41,24 @@ type RegistrationResponse struct { CheckedInAt *string `json:"checked_in_at,omitempty"` } -func toRegistrationResponse(r generated.Registration) RegistrationResponse { +// toRegistrationResponse maps a registration row to its API representation. +// When includeStaff is false (unauthenticated / non-staff readers), staff-only +// fields (admin_notes) are omitted so internal tournament-director commentary is +// never leaked on the public read paths. +func toRegistrationResponse(r generated.Registration, includeStaff bool) RegistrationResponse { resp := RegistrationResponse{ ID: r.ID, DivisionID: r.DivisionID, RegisteredByUserID: r.RegisteredByUserID, Status: r.Status, RegistrationNotes: r.RegistrationNotes, - AdminNotes: r.AdminNotes, RegisteredAt: r.RegisteredAt.Format(time.RFC3339), } + if includeStaff { + resp.AdminNotes = r.AdminNotes + } + if r.TeamID.Valid { resp.TeamID = &r.TeamID.Int64 } @@ -100,16 +107,17 @@ func (s *RegistrationService) Register(ctx context.Context, params generated.Cre return RegistrationResponse{}, &ValidationError{Message: "division is not open for registration"} } - // Check capacity — if max_teams set, check approved count + // Check capacity — if max_teams set, count all slot-occupying registrations. + // Both 'approved' and 'checked_in' teams occupy a real field slot (matching + // bracket generation and standings, which treat approved+checked_in as the + // active set); counting only 'approved' would let check-ins free phantom + // slots and allow registrations to exceed max_teams. if division.MaxTeams.Valid && division.MaxTeams.Int32 > 0 { - approvedCount, err := s.queries.CountRegistrationsByDivisionAndStatus(ctx, generated.CountRegistrationsByDivisionAndStatusParams{ - DivisionID: params.DivisionID, - Status: "approved", - }) + activeCount, err := s.queries.CountActiveRegistrationsByDivision(ctx, params.DivisionID) if err != nil { return RegistrationResponse{}, fmt.Errorf("failed to count registrations: %w", err) } - if approvedCount >= int64(division.MaxTeams.Int32) { + if activeCount >= int64(division.MaxTeams.Int32) { // Division is full — waitlist params.Status = "waitlisted" } @@ -129,11 +137,13 @@ func (s *RegistrationService) Register(ctx context.Context, params generated.Cre return RegistrationResponse{}, fmt.Errorf("failed to create registration: %w", err) } - return toRegistrationResponse(reg), nil + return toRegistrationResponse(reg, true), nil } -// GetByID retrieves a registration by ID. -func (s *RegistrationService) GetByID(ctx context.Context, id int64) (RegistrationResponse, error) { +// GetByID retrieves a registration by ID. includeStaff controls whether +// staff-only fields (admin_notes) are returned; pass false for unauthenticated +// or non-staff callers. +func (s *RegistrationService) GetByID(ctx context.Context, id int64, includeStaff bool) (RegistrationResponse, error) { reg, err := s.queries.GetRegistrationByID(ctx, id) if err != nil { if errors.Is(err, pgx.ErrNoRows) { @@ -141,11 +151,12 @@ func (s *RegistrationService) GetByID(ctx context.Context, id int64) (Registrati } return RegistrationResponse{}, fmt.Errorf("get registration by id: %w", err) } - return toRegistrationResponse(reg), nil + return toRegistrationResponse(reg, includeStaff), nil } -// ListByDivision returns registrations for a division. -func (s *RegistrationService) ListByDivision(ctx context.Context, divisionID int64, limit, offset int32) ([]RegistrationResponse, int64, error) { +// ListByDivision returns registrations for a division. includeStaff controls +// whether staff-only fields (admin_notes) are returned. +func (s *RegistrationService) ListByDivision(ctx context.Context, divisionID int64, limit, offset int32, includeStaff bool) ([]RegistrationResponse, int64, error) { regs, err := s.queries.ListRegistrationsByDivision(ctx, generated.ListRegistrationsByDivisionParams{ DivisionID: divisionID, Limit: limit, @@ -162,14 +173,15 @@ func (s *RegistrationService) ListByDivision(ctx context.Context, divisionID int result := make([]RegistrationResponse, len(regs)) for i, r := range regs { - result[i] = toRegistrationResponse(r) + result[i] = toRegistrationResponse(r, includeStaff) } return result, count, nil } // ListByDivisionAndStatus returns registrations filtered by division and status. -func (s *RegistrationService) ListByDivisionAndStatus(ctx context.Context, divisionID int64, status string, limit, offset int32) ([]RegistrationResponse, int64, error) { +// includeStaff controls whether staff-only fields (admin_notes) are returned. +func (s *RegistrationService) ListByDivisionAndStatus(ctx context.Context, divisionID int64, status string, limit, offset int32, includeStaff bool) ([]RegistrationResponse, int64, error) { regs, err := s.queries.ListRegistrationsByDivisionAndStatus(ctx, generated.ListRegistrationsByDivisionAndStatusParams{ DivisionID: divisionID, Status: status, @@ -190,14 +202,16 @@ func (s *RegistrationService) ListByDivisionAndStatus(ctx context.Context, divis result := make([]RegistrationResponse, len(regs)) for i, r := range regs { - result[i] = toRegistrationResponse(r) + result[i] = toRegistrationResponse(r, includeStaff) } return result, count, nil } // UpdateStatus updates a registration's status, with auto-promote from waitlist on withdrawal/rejection. -func (s *RegistrationService) UpdateStatus(ctx context.Context, id int64, newStatus string) (RegistrationResponse, error) { +// The registration must belong to divisionID; otherwise a not-found error is +// returned so callers cannot mutate registrations outside the URL's division. +func (s *RegistrationService) UpdateStatus(ctx context.Context, divisionID, id int64, newStatus string) (RegistrationResponse, error) { reg, err := s.queries.GetRegistrationByID(ctx, id) if err != nil { if errors.Is(err, pgx.ErrNoRows) { @@ -205,6 +219,9 @@ func (s *RegistrationService) UpdateStatus(ctx context.Context, id int64, newSta } return RegistrationResponse{}, fmt.Errorf("get registration for status update: %w", err) } + if reg.DivisionID != divisionID { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } updated, err := s.queries.UpdateRegistrationStatus(ctx, generated.UpdateRegistrationStatusParams{ ID: id, @@ -228,11 +245,23 @@ func (s *RegistrationService) UpdateStatus(ctx context.Context, id int64, newSta } } - return toRegistrationResponse(updated), nil + return toRegistrationResponse(updated, true), nil } -// UpdateSeed updates a registration's seed. -func (s *RegistrationService) UpdateSeed(ctx context.Context, id int64, seed pgtype.Int4) (RegistrationResponse, error) { +// UpdateSeed updates a registration's seed. The registration must belong to +// divisionID; otherwise a not-found error is returned. +func (s *RegistrationService) UpdateSeed(ctx context.Context, divisionID, id int64, seed pgtype.Int4) (RegistrationResponse, error) { + existing, err := s.queries.GetRegistrationByID(ctx, id) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } + return RegistrationResponse{}, fmt.Errorf("get registration for seed update: %w", err) + } + if existing.DivisionID != divisionID { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } + reg, err := s.queries.UpdateRegistrationSeed(ctx, generated.UpdateRegistrationSeedParams{ ID: id, Seed: seed, @@ -243,11 +272,23 @@ func (s *RegistrationService) UpdateSeed(ctx context.Context, id int64, seed pgt } return RegistrationResponse{}, fmt.Errorf("update registration seed: %w", err) } - return toRegistrationResponse(reg), nil + return toRegistrationResponse(reg, true), nil } -// UpdatePlacement updates a registration's final placement. -func (s *RegistrationService) UpdatePlacement(ctx context.Context, id int64, placement pgtype.Int4) (RegistrationResponse, error) { +// UpdatePlacement updates a registration's final placement. The registration +// must belong to divisionID; otherwise a not-found error is returned. +func (s *RegistrationService) UpdatePlacement(ctx context.Context, divisionID, id int64, placement pgtype.Int4) (RegistrationResponse, error) { + existing, err := s.queries.GetRegistrationByID(ctx, id) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } + return RegistrationResponse{}, fmt.Errorf("get registration for placement update: %w", err) + } + if existing.DivisionID != divisionID { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } + reg, err := s.queries.UpdateRegistrationPlacement(ctx, generated.UpdateRegistrationPlacementParams{ ID: id, FinalPlacement: placement, @@ -258,7 +299,7 @@ func (s *RegistrationService) UpdatePlacement(ctx context.Context, id int64, pla } return RegistrationResponse{}, fmt.Errorf("update registration placement: %w", err) } - return toRegistrationResponse(reg), nil + return toRegistrationResponse(reg, true), nil } // BulkNoShow marks the given registration IDs as no_show, scoped to the @@ -276,6 +317,7 @@ func (s *RegistrationService) BulkNoShow(ctx context.Context, divisionID int64, } // ListSeekingPartner returns registrations that are seeking a partner. +// This is a public read path, so staff-only fields are always omitted. func (s *RegistrationService) ListSeekingPartner(ctx context.Context, divisionID int64) ([]RegistrationResponse, error) { regs, err := s.queries.ListSeekingPartner(ctx, divisionID) if err != nil { @@ -284,14 +326,15 @@ func (s *RegistrationService) ListSeekingPartner(ctx context.Context, divisionID result := make([]RegistrationResponse, len(regs)) for i, r := range regs { - result[i] = toRegistrationResponse(r) + result[i] = toRegistrationResponse(r, false) } return result, nil } -// CheckIn marks a registration as checked in. -func (s *RegistrationService) CheckIn(ctx context.Context, id int64) (RegistrationResponse, error) { +// CheckIn marks a registration as checked in. The registration must belong to +// divisionID; otherwise a not-found error is returned. +func (s *RegistrationService) CheckIn(ctx context.Context, divisionID, id int64) (RegistrationResponse, error) { reg, err := s.queries.GetRegistrationByID(ctx, id) if err != nil { if errors.Is(err, pgx.ErrNoRows) { @@ -299,6 +342,9 @@ func (s *RegistrationService) CheckIn(ctx context.Context, id int64) (Registrati } return RegistrationResponse{}, fmt.Errorf("get registration for check-in: %w", err) } + if reg.DivisionID != divisionID { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } if reg.Status != "approved" { return RegistrationResponse{}, &ValidationError{Message: "only approved registrations can check in"} @@ -312,11 +358,12 @@ func (s *RegistrationService) CheckIn(ctx context.Context, id int64) (Registrati return RegistrationResponse{}, fmt.Errorf("failed to check in: %w", err) } - return toRegistrationResponse(updated), nil + return toRegistrationResponse(updated, true), nil } -// WithdrawMidTournament withdraws a registration mid-tournament. -func (s *RegistrationService) WithdrawMidTournament(ctx context.Context, id int64) (RegistrationResponse, error) { +// WithdrawMidTournament withdraws a registration mid-tournament. The +// registration must belong to divisionID; otherwise a not-found error is returned. +func (s *RegistrationService) WithdrawMidTournament(ctx context.Context, divisionID, id int64) (RegistrationResponse, error) { reg, err := s.queries.GetRegistrationByID(ctx, id) if err != nil { if errors.Is(err, pgx.ErrNoRows) { @@ -324,6 +371,9 @@ func (s *RegistrationService) WithdrawMidTournament(ctx context.Context, id int6 } return RegistrationResponse{}, fmt.Errorf("get registration for withdrawal: %w", err) } + if reg.DivisionID != divisionID { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } if reg.Status != "checked_in" && reg.Status != "approved" { return RegistrationResponse{}, &ValidationError{Message: "only approved or checked-in registrations can withdraw mid-tournament"} @@ -337,11 +387,23 @@ func (s *RegistrationService) WithdrawMidTournament(ctx context.Context, id int6 return RegistrationResponse{}, fmt.Errorf("failed to withdraw: %w", err) } - return toRegistrationResponse(updated), nil + return toRegistrationResponse(updated, true), nil } -// UpdateAdminNotes updates the admin notes on a registration. -func (s *RegistrationService) UpdateAdminNotes(ctx context.Context, id int64, notes *string) (RegistrationResponse, error) { +// UpdateAdminNotes updates the admin notes on a registration. The registration +// must belong to divisionID; otherwise a not-found error is returned. +func (s *RegistrationService) UpdateAdminNotes(ctx context.Context, divisionID, id int64, notes *string) (RegistrationResponse, error) { + existing, err := s.queries.GetRegistrationByID(ctx, id) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } + return RegistrationResponse{}, fmt.Errorf("get registration for admin notes update: %w", err) + } + if existing.DivisionID != divisionID { + return RegistrationResponse{}, &NotFoundError{Message: "registration not found"} + } + updated, err := s.queries.UpdateRegistrationAdminNotes(ctx, generated.UpdateRegistrationAdminNotesParams{ ID: id, AdminNotes: notes, @@ -353,5 +415,5 @@ func (s *RegistrationService) UpdateAdminNotes(ctx context.Context, id int64, no return RegistrationResponse{}, fmt.Errorf("update registration admin notes: %w", err) } - return toRegistrationResponse(updated), nil + return toRegistrationResponse(updated, true), nil } diff --git a/api/service/standings.go b/api/service/standings.go index 8aaee3d1..095806ba 100644 --- a/api/service/standings.go +++ b/api/service/standings.go @@ -113,6 +113,21 @@ type teamStats struct { PointsAgainst int32 MatchesPlayed int32 StandingPoints int32 + + // Admin adjustments carried over from the existing standings entry so that + // ranking honors them. OverridePoints, when set, replaces StandingPoints as + // the ranking key; IsWithdrawn teams are sorted to the bottom. + OverridePoints *int32 + IsWithdrawn bool +} + +// rankingPoints returns the points value used to order standings, honoring an +// admin override when present. +func (t teamStats) rankingPoints() int32 { + if t.OverridePoints != nil { + return *t.OverridePoints + } + return t.StandingPoints } // ---------- Recompute ---------- @@ -127,6 +142,22 @@ func (svc *StandingsService) RecomputeStandings(ctx context.Context, seasonID, d return nil, &NotFoundError{Message: "season not found"} } + // Verify the division actually belongs to this season via its tournament, + // mirroring the canonical derivation used by the public standings read path + // (division -> tournament -> tournament.season_id). Without this, a mismatched + // (season, division) pair would upsert incoherent standings_entries rows. + division, err := svc.queries.GetDivisionByID(ctx, divisionID) + if err != nil { + return nil, &NotFoundError{Message: "division not found"} + } + tournament, err := svc.queries.GetTournamentByID(ctx, division.TournamentID) + if err != nil { + return nil, &NotFoundError{Message: "tournament not found"} + } + if !tournament.SeasonID.Valid || tournament.SeasonID.Int64 != seasonID { + return nil, &ValidationError{Message: "division does not belong to this season"} + } + method := "placement_points" if season.StandingsMethod != nil && *season.StandingsMethod != "" { method = *season.StandingsMethod @@ -175,6 +206,24 @@ func (svc *StandingsService) RecomputeStandings(ctx context.Context, seasonID, d return []StandingsEntryResponse{}, nil } + // Load existing standings entries so admin adjustments (override_points, + // is_withdrawn) are honored when ranking. UpsertStandingsEntry preserves + // these columns in the DB, but ranking must read them or the recomputed + // rank would contradict the override/withdrawal. + existing, err := svc.queries.ListStandingsByDivision(ctx, generated.ListStandingsByDivisionParams{ + SeasonID: seasonID, + DivisionID: divisionID, + Limit: 1000, + Offset: 0, + }) + if err != nil { + return nil, fmt.Errorf("failed to list existing standings: %w", err) + } + existingByTeam := make(map[int64]generated.StandingsEntry, len(existing)) + for _, e := range existing { + existingByTeam[e.TeamID] = e + } + // 3. For each team, fetch completed matches and compute stats allStats := make([]teamStats, 0, len(teamSet)) @@ -190,7 +239,7 @@ func (svc *StandingsService) RecomputeStandings(ctx context.Context, seasonID, d stats := teamStats{TeamID: teamID} for _, m := range matches { - if m.Status != "completed" { + if m.Status != "completed" && m.Status != "forfeited" { continue } @@ -221,13 +270,30 @@ func (svc *StandingsService) RecomputeStandings(ctx context.Context, seasonID, d } } + // Carry over admin adjustments from the existing entry so ranking honors them. + if prev, ok := existingByTeam[teamID]; ok { + stats.IsWithdrawn = prev.IsWithdrawn + if prev.OverridePoints.Valid { + v := prev.OverridePoints.Int32 + stats.OverridePoints = &v + } + } + allStats = append(allStats, stats) } - // 4. Sort by standing_points DESC, then point_differential DESC, then wins DESC + // 4. Sort withdrawn teams to the bottom, then by effective ranking points + // (override when set, else standing_points) DESC, then point_differential + // DESC, then wins DESC. sort.Slice(allStats, func(i, j int) bool { - if allStats[i].StandingPoints != allStats[j].StandingPoints { - return allStats[i].StandingPoints > allStats[j].StandingPoints + if allStats[i].IsWithdrawn != allStats[j].IsWithdrawn { + // Active teams (not withdrawn) rank ahead of withdrawn teams. + return !allStats[i].IsWithdrawn + } + ptsI := allStats[i].rankingPoints() + ptsJ := allStats[j].rankingPoints() + if ptsI != ptsJ { + return ptsI > ptsJ } diffI := allStats[i].PointsFor - allStats[i].PointsAgainst diffJ := allStats[j].PointsFor - allStats[j].PointsAgainst diff --git a/api/service/upload.go b/api/service/upload.go index 216bf5ac..899a8a85 100644 --- a/api/service/upload.go +++ b/api/service/upload.go @@ -93,10 +93,13 @@ func (s *UploadService) SaveFile(ctx context.Context, userID int64, file io.Read if _, err := rand.Read(randBytes); err != nil { return nil, fmt.Errorf("generating filename: %w", err) } - ext := filepath.Ext(originalName) - if ext == "" { - ext = extensionFromContentType(contentType) - } + // Always derive the on-disk extension from the validated/sniffed + // contentType, never from the client-controlled original filename. + // contentType is already gated by AllowedContentTypes above, so this + // can only ever yield a safe allowlisted extension (.jpg/.png/.gif/ + // .webp/.pdf) and never .html/.svg/.xhtml. The original filename is + // retained only in the DB original_name column for display. + ext := extensionFromContentType(contentType) filename := hex.EncodeToString(randBytes) + ext // Ensure upload directory exists diff --git a/api/service/venue.go b/api/service/venue.go index 58d0d507..07b217a6 100644 --- a/api/service/venue.go +++ b/api/service/venue.go @@ -772,6 +772,44 @@ func (s *VenueService) CanManageVenue(ctx context.Context, venueID int64, userID return isMgr, nil } +// CanManageCourt reports whether the given user may manage (edit/delete) a +// court. Returns true for platform admins, the court's creator, or a manager +// of the court's owning venue — the same ownership boundary +// OverlayService.CanManageCourt enforces for overlay-config writes, so a +// caller cannot mutate a court outside their org/tournament. Returns +// NotFoundError when the court does not exist. +func (s *VenueService) CanManageCourt(ctx context.Context, courtID int64, userID int64, userRole string) (bool, error) { + if userRole == "platform_admin" { + return true, nil + } + + court, err := s.queries.GetCourtByID(ctx, courtID) + if err != nil { + return false, &NotFoundError{Message: "court not found"} + } + + // The court creator may always manage it. + if court.CreatedByUserID.Valid && court.CreatedByUserID.Int64 == userID { + return true, nil + } + + // Otherwise, the caller must be a manager of the court's owning venue. + if court.VenueID.Valid { + isMgr, err := s.queries.IsVenueManager(ctx, generated.IsVenueManagerParams{ + VenueID: court.VenueID.Int64, + UserID: userID, + }) + if err != nil { + return false, err + } + if isMgr { + return true, nil + } + } + + return false, nil +} + // CanAdminVenue checks if a user has admin-level access to a venue // (can add/remove managers). Returns true for venue admins, creator, or platform admin. func (s *VenueService) CanAdminVenue(ctx context.Context, venueID int64, userID int64, userRole string) (bool, error) { diff --git a/api/testutil/testserver.go b/api/testutil/testserver.go index 61de55aa..c64a8c1c 100644 --- a/api/testutil/testserver.go +++ b/api/testutil/testserver.go @@ -111,7 +111,10 @@ func TestServer(t *testing.T, pool *pgxpool.Pool) *httptest.Server { courtQueueService := service.NewCourtQueueService(queries, pool, nil) // Phase 1+2 handlers - authHandler := handler.NewAuthHandler(authService, false) + // nil org-role resolver: the test server doesn't wire Logto Mgmt API + // creds, so MeJWT relies on the JWT fast path only (same posture as + // JWTSession with a nil resolver). + authHandler := handler.NewAuthHandler(authService, false, nil) healthHandler := handler.NewHealthHandler(pool, store.Client()) playerHandler := handler.NewPlayerHandler(playerService) teamHandler := handler.NewTeamHandler(teamService) diff --git a/api/ws/handler.go b/api/ws/handler.go index 761b906f..6350025f 100644 --- a/api/ws/handler.go +++ b/api/ws/handler.go @@ -1,6 +1,7 @@ package ws import ( + "context" "log/slog" "net/http" "strconv" @@ -152,7 +153,14 @@ func (h *Handler) handleSubscription(w http.ResponseWriter, r *http.Request, cha h.logger.Info("websocket connected", "channel", channel, "remote", r.RemoteAddr) // Subscribe to the Redis channel. - msgChan, cancelSub, err := h.ps.Subscribe(r.Context(), channel) + // + // Use context.WithoutCancel to strip the HTTP request's deadline/cancellation + // (the global chimw.Timeout(60s) middleware applies to /ws too) while + // preserving request-scoped values. Without this, every long-lived WebSocket + // would be forcibly torn down after 60s when the request context's deadline + // fires. The subscription is still cancelled on real client disconnect via + // the deferred cancelSub() below plus the read-pump's done channel. + msgChan, cancelSub, err := h.ps.Subscribe(context.WithoutCancel(r.Context()), channel) if err != nil { h.logger.Error("failed to subscribe", "channel", channel, "error", err) conn.WriteMessage(websocket.CloseMessage, diff --git a/web/src/components/ImageUpload.tsx b/web/src/components/ImageUpload.tsx index b2166da9..3c267610 100644 --- a/web/src/components/ImageUpload.tsx +++ b/web/src/components/ImageUpload.tsx @@ -1,5 +1,6 @@ import { useState, useRef, useCallback } from 'react' import { cn } from '../lib/cn' +import { apiPostForm } from '../lib/api' import { Upload, X, Loader2 } from 'lucide-react' interface ImageUploadProps { @@ -39,17 +40,8 @@ export function ImageUpload({ try { const formData = new FormData() formData.append('file', file) - const response = await fetch('/api/v1/uploads', { - method: 'POST', - credentials: 'include', - body: formData, - }) - if (!response.ok) { - const body = await response.json().catch(() => ({})) - throw new Error(body.error?.message || 'Upload failed') - } - const body = await response.json() - onChange(body.data?.url || body.url || null) + const data = await apiPostForm<{ url: string }>('/api/v1/uploads', formData) + onChange(data?.url || null) } catch (err) { setError((err as Error).message) } finally { diff --git a/web/src/components/MapView.tsx b/web/src/components/MapView.tsx index 67573a3b..845d9697 100644 --- a/web/src/components/MapView.tsx +++ b/web/src/components/MapView.tsx @@ -94,27 +94,43 @@ export function MapView({ }) gMarker.addListener('click', () => { - const content = ` -
- ${m.label} - ${m.sublabel ? `
${m.sublabel}` : ''} - ${onMarkerClick ? `
View details →` : ''} -
- ` - infoWindowRef.current.setContent(content) - infoWindowRef.current.open(mapInstanceRef.current, gMarker) + // Build the InfoWindow content with DOM APIs and set untrusted + // values via textContent so label/sublabel can never be parsed as + // HTML (avoids stored DOM XSS via venue/org names). + const container = document.createElement('div') + container.style.fontFamily = 'system-ui, sans-serif' + container.style.padding = '4px 0' + + const strong = document.createElement('strong') + strong.style.fontSize = '14px' + strong.textContent = m.label + container.appendChild(strong) + + if (m.sublabel) { + container.appendChild(document.createElement('br')) + const span = document.createElement('span') + span.style.fontSize = '12px' + span.style.color = '#666' + span.textContent = m.sublabel + container.appendChild(span) + } if (onMarkerClick) { - setTimeout(() => { - const link = document.getElementById(`map-marker-${m.id}`) - if (link) { - link.addEventListener('click', (e) => { - e.preventDefault() - onMarkerClick(m) - }) - } - }, 50) + container.appendChild(document.createElement('br')) + const link = document.createElement('a') + link.href = '#' + link.style.fontSize = '12px' + link.style.color = '#0ea5e9' + link.textContent = 'View details →' + link.addEventListener('click', (e) => { + e.preventDefault() + onMarkerClick(m) + }) + container.appendChild(link) } + + infoWindowRef.current.setContent(container) + infoWindowRef.current.open(mapInstanceRef.current, gMarker) }) markersRef.current.push(gMarker) diff --git a/web/src/components/VenuePicker.tsx b/web/src/components/VenuePicker.tsx index 84c81301..cdc36fe2 100644 --- a/web/src/components/VenuePicker.tsx +++ b/web/src/components/VenuePicker.tsx @@ -1,6 +1,6 @@ import { useState, useEffect, useRef } from 'react' import { useQuery } from '@tanstack/react-query' -import { apiGetPaginated, type PaginatedData } from '../lib/api' +import { apiGet, apiGetPaginated, type PaginatedData } from '../lib/api' import { buildQueryString } from '../lib/formatters' import { cn } from '../lib/cn' import { useDebounce } from '../hooks/useDebounce' @@ -36,14 +36,7 @@ export function VenuePicker({ value, onChange, className }: VenuePickerProps) { const { data: selectedVenue } = useQuery({ queryKey: ['venues', value], - queryFn: () => { - return apiGetPaginated( - `/api/v1/venues${buildQueryString({ limit: 1, offset: 0 })}`, - ).then((r) => { - const found = r.items.find((v) => v.id === value) - return found || { id: value!, name: 'Unknown Venue', city: null, state_province: null } - }) - }, + queryFn: () => apiGet(`/api/v1/venues/${value}`), enabled: !!value && !open, }) diff --git a/web/src/features/admin/UserDetail.tsx b/web/src/features/admin/UserDetail.tsx index 3c2b5b20..bc27e758 100644 --- a/web/src/features/admin/UserDetail.tsx +++ b/web/src/features/admin/UserDetail.tsx @@ -92,8 +92,11 @@ export function UserDetail({ userId }: UserDetailProps) { if (!user) return startImpersonation.mutate(user.id, { onSuccess: () => { - // Full reload to reset all cached state and enter impersonation mode - window.location.href = '/dashboard' + // Full reload to reset all cached state and enter impersonation mode. + // Navigate to the sport-scoped dashboard; if no sport is resolved, + // fall back to '/' so the SPA re-resolves sport context (matching + // ImpersonationBanner.handleStop). + window.location.href = sportSlug ? `/${sportSlug}/dashboard` : '/' }, onError: (err) => { toast('error', err.message || 'Failed to start impersonation.') diff --git a/web/src/features/dashboard/MyTeams.tsx b/web/src/features/dashboard/MyTeams.tsx index 8a11b4da..c0f1b529 100644 --- a/web/src/features/dashboard/MyTeams.tsx +++ b/web/src/features/dashboard/MyTeams.tsx @@ -3,6 +3,7 @@ import { Users } from 'lucide-react' import { Card } from '../../components/Card' import { Badge } from '../../components/Badge' import { EmptyState } from '../../components/EmptyState' +import { useSport } from '../../auth/SportContext' import type { DashboardTeam } from './hooks' interface Props { @@ -35,6 +36,9 @@ function TeamAvatar({ team }: { team: DashboardTeam }) { } export function MyTeams({ data }: Props) { + const { sport } = useSport() + const sportSlug = sport?.slug ?? '' + if (data.length === 0) { return ( ( diff --git a/web/src/features/dashboard/RecentResults.tsx b/web/src/features/dashboard/RecentResults.tsx index c64c361d..8e23a499 100644 --- a/web/src/features/dashboard/RecentResults.tsx +++ b/web/src/features/dashboard/RecentResults.tsx @@ -3,6 +3,7 @@ import { Trophy } from 'lucide-react' import { Card } from '../../components/Card' import { EmptyState } from '../../components/EmptyState' import { formatDate } from '../../lib/formatters' +import { useSport } from '../../auth/SportContext' import type { DashboardMatch } from './hooks' interface Props { @@ -10,6 +11,9 @@ interface Props { } export function RecentResults({ data }: Props) { + const { sport } = useSport() + const sportSlug = sport?.slug ?? '' + if (data.length === 0) { return ( ( diff --git a/web/src/features/dashboard/UpcomingMatches.tsx b/web/src/features/dashboard/UpcomingMatches.tsx index 454fba15..46cf0a17 100644 --- a/web/src/features/dashboard/UpcomingMatches.tsx +++ b/web/src/features/dashboard/UpcomingMatches.tsx @@ -4,6 +4,7 @@ import { Card } from '../../components/Card' import { EmptyState } from '../../components/EmptyState' import { StatusBadge } from '../../components/StatusBadge' import { formatDateTime } from '../../lib/formatters' +import { useSport } from '../../auth/SportContext' import type { DashboardMatch } from './hooks' interface Props { @@ -11,6 +12,9 @@ interface Props { } export function UpcomingMatches({ data }: Props) { + const { sport } = useSport() + const sportSlug = sport?.slug ?? '' + if (data.length === 0) { return ( ( diff --git a/web/src/features/overlay/useOverlayWebSocket.ts b/web/src/features/overlay/useOverlayWebSocket.ts index 1df318d2..74440544 100644 --- a/web/src/features/overlay/useOverlayWebSocket.ts +++ b/web/src/features/overlay/useOverlayWebSocket.ts @@ -83,7 +83,6 @@ export function useOverlayWebSocket( const socketsRef = useRef>({}) const attemptsRef = useRef>({}) const timersRef = useRef>({}) - const closedByUnmountRef = useRef(false) const onMessageRef = useRef(onMessage) onMessageRef.current = onMessage @@ -92,7 +91,18 @@ export function useOverlayWebSocket( setState('disconnected') return } - closedByUnmountRef.current = false + // Per-run cancellation. A single shared ref would be reset to false by + // the next effect run before the previous run's sockets fire their + // (asynchronous) onclose, letting a torn-down run reschedule connect() + // for the stale court/match and pollute the new run's state. Capturing + // the flag in the effect closure ties it to this run only. + let cancelled = false + // Fresh per-run maps so a late onclose from a previous run can't mutate + // the new run's live socket/state/timer entries (same label keys). + stateRef.current = {} + socketsRef.current = {} + attemptsRef.current = {} + timersRef.current = {} const specs: OverlaySocketSpec[] = [ { label: 'overlay', url: `${wsBaseUrl()}/ws/overlay/${courtID}` }, @@ -153,8 +163,13 @@ export function useOverlayWebSocket( } ws.onclose = () => { + // Ignore late closes from a torn-down effect run. `cancelled` is + // captured per-run so a previous run's socket can't act here. + if (cancelled) return + // Guard against a stale onclose deleting the new run's live socket: + // only clear/reschedule if this ws is still the registered one. + if (socketsRef.current[spec.label] !== ws) return delete socketsRef.current[spec.label] - if (closedByUnmountRef.current) return stateRef.current[spec.label] = 'disconnected' computeAggregate() const current = attemptsRef.current[spec.label] ?? 0 @@ -171,7 +186,7 @@ export function useOverlayWebSocket( specs.forEach(connect) return () => { - closedByUnmountRef.current = true + cancelled = true Object.values(timersRef.current).forEach((t) => window.clearTimeout(t)) timersRef.current = {} Object.values(socketsRef.current).forEach((ws) => ws.close()) @@ -194,12 +209,17 @@ function applyMessage( ): void { switch (msg.type) { case 'overlay_data': { - qc.setQueryData( - ['overlay', 'data', courtID, null, false], - msg.data as OverlayData, + // useOverlayData keys by (courtID, token ?? null, !!demo), so a + // hardcoded null/false key misses token-gated and demo overlays. + // Write by prefix predicate so the merge lands on whatever variant + // the renderer is actually observing — no HTTP round-trip. The raw + // backend shape is stored; normalizeOverlayData still runs via the + // query's `select` on read, preserving current behavior. Only touch + // entries something is observing (prev !== undefined). + qc.setQueriesData( + { queryKey: ['overlay', 'data', courtID] }, + (prev) => (prev === undefined ? prev : (msg.data as OverlayData)), ) - // Also mirror into any token-scoped caches. - qc.invalidateQueries({ queryKey: ['overlay', 'data', courtID] }) break } case 'config_update': { diff --git a/web/src/features/public/PublicDivisionDetail.tsx b/web/src/features/public/PublicDivisionDetail.tsx index 20e6f3a0..99d8f5c9 100644 --- a/web/src/features/public/PublicDivisionDetail.tsx +++ b/web/src/features/public/PublicDivisionDetail.tsx @@ -25,6 +25,7 @@ import { TabLayout } from '../../components/TabLayout' import { EmptyState } from '../../components/EmptyState' import { Badge } from '../../components/Badge' import { usePageTitle } from '../../hooks/usePageTitle' +import { useSport } from '../../auth/SportContext' import { cn } from '../../lib/cn' interface PublicDivisionDetailProps { @@ -384,6 +385,8 @@ function BracketMatchCard({ match: LiveMatch isLastRound: boolean }) { + const { sport } = useSport() + const sportSlug = sport?.slug ?? 'pickleball' const isComplete = match.status === 'completed' const team1Won = isComplete && match.team_1_score > match.team_2_score const team2Won = isComplete && match.team_2_score > match.team_1_score @@ -452,8 +455,8 @@ function BracketMatchCard({ if (match.public_id) { return ( } + to="/$sport/matches/$publicId" + params={{ sport: sportSlug, publicId: match.public_id }} > {inner} @@ -681,6 +684,8 @@ function MatchesTab({ } function MatchRow({ match }: { match: LiveMatch }) { + const { sport } = useSport() + const sportSlug = sport?.slug ?? 'pickleball' const isLive = match.status === 'in_progress' const isDone = ['completed', 'forfeited', 'cancelled'].includes(match.status) @@ -747,8 +752,8 @@ function MatchRow({ match }: { match: LiveMatch }) { if (match.public_id) { return ( } + to="/$sport/matches/$publicId" + params={{ sport: sportSlug, publicId: match.public_id }} className="block" > {card} diff --git a/web/src/features/public/TournamentDirectory.tsx b/web/src/features/public/TournamentDirectory.tsx index 5e91dbf2..204a0f3f 100644 --- a/web/src/features/public/TournamentDirectory.tsx +++ b/web/src/features/public/TournamentDirectory.tsx @@ -62,7 +62,10 @@ export function TournamentDirectory() { onQueryChange={setQuery} statusOptions={TOURNAMENT_STATUS_OPTIONS} selectedStatus={status} - onStatusChange={setStatus} + onStatusChange={(v) => { + setStatus(v) + pagination.setPage(1) + }} /> {isLoading ? ( diff --git a/web/src/features/referee/RefMatchConsole.tsx b/web/src/features/referee/RefMatchConsole.tsx index 0b18cc63..90ae1390 100644 --- a/web/src/features/referee/RefMatchConsole.tsx +++ b/web/src/features/referee/RefMatchConsole.tsx @@ -207,7 +207,11 @@ export function RefMatchConsole({ publicId }: RefMatchConsoleProps) { setMenuOpen(false) }, }, - prefs.keyboard && match?.status === 'in_progress' && !disabled, + prefs.keyboard && + match?.status === 'in_progress' && + !disabled && + !gameOverPrompt && + !matchOverPrompt, ) if (matchQuery.isLoading) { diff --git a/web/src/features/scorekeeper/ScorekeeperMatchConsole.tsx b/web/src/features/scorekeeper/ScorekeeperMatchConsole.tsx index 891e1b45..4b3689e7 100644 --- a/web/src/features/scorekeeper/ScorekeeperMatchConsole.tsx +++ b/web/src/features/scorekeeper/ScorekeeperMatchConsole.tsx @@ -152,7 +152,14 @@ export function ScorekeeperMatchConsole({ }, onUndo: handleUndo, }, - prefs.keyboard && match?.status === 'in_progress' && !disabled, + prefs.keyboard && + match?.status === 'in_progress' && + !disabled && + !scorePoint.isPending && + !sideOut.isPending && + !undo.isPending && + !confirmGame.isPending && + !confirmMatch.isPending, ) if (matchQuery.isLoading) { @@ -210,7 +217,11 @@ export function ScorekeeperMatchConsole({ mode="scorekeeper" disabled={disabled} pending={ - scorePoint.isPending || sideOut.isPending || undo.isPending + scorePoint.isPending || + sideOut.isPending || + undo.isPending || + confirmGame.isPending || + confirmMatch.isPending } onPoint={handlePoint} onSideOut={handleSideOut} diff --git a/web/src/features/scoring/useMatchWebSocket.ts b/web/src/features/scoring/useMatchWebSocket.ts index b0b5fc6b..cdcbf369 100644 --- a/web/src/features/scoring/useMatchWebSocket.ts +++ b/web/src/features/scoring/useMatchWebSocket.ts @@ -36,11 +36,20 @@ export function useMatchWebSocket(publicId: string | undefined): { const attemptsRef = useRef(0) const wsRef = useRef(null) const reconnectTimerRef = useRef(null) - const closedByUnmountRef = useRef(false) useEffect(() => { if (!publicId) return - closedByUnmountRef.current = false + // Per-run cancellation. A single shared ref would be reset to false by + // the next effect run before the previous run's socket fires its + // (asynchronous) onclose, letting a torn-down run reschedule connect() + // for the stale publicId and pollute the new run's state. Capturing the + // flag in the effect closure ties it to this run only. + let cancelled = false + // Fresh per-run state so a late onclose from a previous run can't mutate + // the new run's live socket/timer/attempt entries. + attemptsRef.current = 0 + wsRef.current = null + reconnectTimerRef.current = null const connect = () => { const url = buildWsUrl(publicId) @@ -76,8 +85,13 @@ export function useMatchWebSocket(publicId: string | undefined): { } ws.onclose = () => { + // Ignore late closes from a torn-down effect run. `cancelled` is + // captured per-run so a previous run's socket can't act here. + if (cancelled) return + // Guard against a stale onclose clobbering the new run's live socket: + // only clear/reschedule if this ws is still the registered one. + if (wsRef.current !== ws) return wsRef.current = null - if (closedByUnmountRef.current) return setState('disconnected') const idx = Math.min(attemptsRef.current, BACKOFF_STEPS_MS.length - 1) const delay = BACKOFF_STEPS_MS[idx] @@ -90,7 +104,7 @@ export function useMatchWebSocket(publicId: string | undefined): { connect() return () => { - closedByUnmountRef.current = true + cancelled = true if (reconnectTimerRef.current) { window.clearTimeout(reconnectTimerRef.current) reconnectTimerRef.current = null diff --git a/web/src/features/search/SearchModal.tsx b/web/src/features/search/SearchModal.tsx index aed8803c..b5d98b85 100644 --- a/web/src/features/search/SearchModal.tsx +++ b/web/src/features/search/SearchModal.tsx @@ -50,7 +50,7 @@ function mapTeams(items: SearchTeamResult[], sportSlug: string): SearchResultIte id: t.id, label: t.name, subtitle: t.short_name !== t.name ? t.short_name : undefined, - link: `/${sportSlug}/teams/${t.slug}`, + link: `/${sportSlug}/teams/${t.id}`, })) } @@ -60,7 +60,7 @@ function mapOrganizations(items: SearchOrganizationResult[], sportSlug: string): id: o.id, label: o.name, subtitle: [o.city, o.state_province].filter(Boolean).join(', ') || undefined, - link: `/${sportSlug}/organizations/${o.slug}`, + link: `/${sportSlug}/organizations/${o.id}`, })) } diff --git a/web/src/features/tournaments/DivisionRegistrations.tsx b/web/src/features/tournaments/DivisionRegistrations.tsx index 5021b0fe..ebdd4b8b 100644 --- a/web/src/features/tournaments/DivisionRegistrations.tsx +++ b/web/src/features/tournaments/DivisionRegistrations.tsx @@ -3,6 +3,7 @@ import { useListRegistrations, useBulkNoShow, useCreateRegistration, + fetchAllRegistrations, type Registration, type Division, } from './hooks' @@ -44,6 +45,7 @@ export function DivisionRegistrations({ const debouncedSearch = useDebounce(search) const pagination = usePagination(20) const [bulkNoShowOpen, setBulkNoShowOpen] = useState(false) + const [bulkNoShowLoading, setBulkNoShowLoading] = useState(false) const [showAddForm, setShowAddForm] = useState(false) const { data, isLoading } = useListRegistrations( @@ -67,15 +69,27 @@ export function DivisionRegistrations({ : registrations async function handleBulkNoShow() { + setBulkNoShowLoading(true) try { - const ids = registrations + // The dialog promises to mark *all* non-checked-in registrations, so we + // must operate on the full division set, not just the current page. + // Fetch every registration (unfiltered) before computing the id list. + const allRegistrations = await fetchAllRegistrations(divisionId) + const ids = allRegistrations .filter((r) => r.status !== 'checked_in' && r.checked_in_at == null) .map((r) => r.id) + if (ids.length === 0) { + toast('success', 'No unchecked registrations to mark') + setBulkNoShowOpen(false) + return + } await bulkNoShowMutation.mutateAsync({ registration_ids: ids }) toast('success', 'Marked unchecked registrations as no-show') setBulkNoShowOpen(false) } catch (err) { toast('error', (err as Error).message) + } finally { + setBulkNoShowLoading(false) } } @@ -223,7 +237,7 @@ export function DivisionRegistrations({ message="Mark all registrations that haven't checked in as no-show? This can be reversed per registration afterwards." confirmText="Mark No-Show" variant="danger" - loading={bulkNoShowMutation.isPending} + loading={bulkNoShowLoading || bulkNoShowMutation.isPending} /> ) diff --git a/web/src/features/tournaments/RegistrationTable.tsx b/web/src/features/tournaments/RegistrationTable.tsx index 7edaefeb..1818bfed 100644 --- a/web/src/features/tournaments/RegistrationTable.tsx +++ b/web/src/features/tournaments/RegistrationTable.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo } from 'react' +import { useState, useMemo, useEffect } from 'react' import { useListRegistrations, type Division, type Registration } from './hooks' import { useDebounce } from '../../hooks/useDebounce' import { Table } from '../../components/Table' @@ -29,7 +29,7 @@ function RegistrationsForDivision({ const { data } = useListRegistrations(String(division.id), undefined, 200) // Push data up to parent when it changes - useMemo(() => { + useEffect(() => { if (data?.items) { onData(division.id, data.items) } diff --git a/web/src/features/tournaments/hooks.ts b/web/src/features/tournaments/hooks.ts index 627aa581..d412e6e3 100644 --- a/web/src/features/tournaments/hooks.ts +++ b/web/src/features/tournaments/hooks.ts @@ -370,6 +370,40 @@ export function useListRegistrations( }) } +/** + * Fetch every registration for a division across all pages. + * + * The list endpoint is paginated (server caps the page size at 100), so a + * single client query only ever holds one page. Bulk operations that must + * cover the whole division need the complete set, which this helper assembles + * by walking the pages until `total` is reached. + */ +export async function fetchAllRegistrations( + divisionId: string, + status?: string, +): Promise { + const pageSize = 100 + const all: Registration[] = [] + let offset = 0 + + // Loop guarded by `total`; pageSize caps each request at the server limit. + // eslint-disable-next-line no-constant-condition + while (true) { + const page = await apiGetPaginated( + `/api/v1/divisions/${divisionId}/registrations${buildQueryString({ + status, + limit: pageSize, + offset, + })}`, + ) + all.push(...page.items) + offset += page.items.length + if (page.items.length === 0 || offset >= page.total) break + } + + return all +} + export function useCreateRegistration(divisionId: string) { const queryClient = useQueryClient() return useMutation({ @@ -589,6 +623,7 @@ export function useUpdateAnnouncement(announcementId: string) { apiPatch(`/api/v1/announcements/${announcementId}`, data), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ['tournaments'] }) + queryClient.invalidateQueries({ queryKey: ['announcements'] }) }, }) } @@ -600,6 +635,7 @@ export function useDeleteAnnouncement(announcementId: string) { apiDelete(`/api/v1/announcements/${announcementId}`), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ['tournaments'] }) + queryClient.invalidateQueries({ queryKey: ['announcements'] }) }, }) } diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 1920980c..9b7c4410 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -90,6 +90,15 @@ export async function apiPost(path: string, body?: unknown): Promise { return handleResponse(response) } +export async function apiPostForm(path: string, form: FormData): Promise { + // Do NOT set Content-Type so the browser adds the multipart boundary. + const headers = await buildHeaders() + const response = await fetch(`${API_BASE}${path}`, { + method: 'POST', headers, body: form, + }) + return handleResponse(response) +} + export async function apiPatch(path: string, body: unknown): Promise { const headers = await buildHeaders({ 'Content-Type': 'application/json' }) const response = await fetch(`${API_BASE}${path}`, { diff --git a/web/src/lib/formatters.ts b/web/src/lib/formatters.ts index 760523eb..f3940b99 100644 --- a/web/src/lib/formatters.ts +++ b/web/src/lib/formatters.ts @@ -1,7 +1,15 @@ export function formatDate(dateStr: string | null | undefined): string { if (!dateStr) return '\u2014' try { - return new Date(dateStr).toLocaleDateString('en-US', { + // Date-only strings ('2026-06-30') are parsed as UTC midnight per the + // ECMAScript spec; formatting in the viewer's local (negative-UTC) zone + // would render the prior day. Parse such values as local so the displayed + // calendar day matches the stored DATE regardless of timezone. + const m = dateStr.match(/^(\d{4})-(\d{2})-(\d{2})$/) + const date = m + ? new Date(Number(m[1]), Number(m[2]) - 1, Number(m[3])) + : new Date(dateStr) + return date.toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric',