Added a v3 standings page #514

Merged

johnsturgeon merged 3 commits from implement-v3-standings-page-513 into main 2026-09-11 21:00:21 +02:00

Conversation 1 Commits 3 Files changed 10 +306 -23

johnsturgeon commented 2026-09-11 19:14:57 +02:00

Owner

Copy link

Also fixed a bug in the menu where it folded under the table header row

Resolves #513

Also fixed a bug in the menu where it folded under the table header row Resolves #513

johnsturgeon added 1 commit 2026-09-11 19:14:57 +02:00

Added a v3 standings page ... 154c8eb53d

Also fixed a bug in the menu where it folded under the table header row

Resolves #513

johnsturgeon commented 2026-09-11 19:16:20 +02:00

Author

Owner

Copy link

This is a clean, well-tested PR. I inspected the route, the new PlayerAward.by_player_for_week helper, the models it leans on, and the template.

Summary

PR #514 adds a v3 standings page that renders one row per active player with season totals, this-week record, and award icons, plus a small nav z-index fix and a font-weight tweak. The data access is deliberately batched (one query for picks, one for awards, cache-warmed games/teams) and the query-count regression test is a genuinely good guard; I found no blocking issues.

Worth fixing

• app/routers/v3.py:125 — the awards legend query runs unordered (select(Award)), so the icon legend row order is nondeterministic across DB restarts/backends. If the header legend should read in a stable order (e.g. by point_value or slug), add an .order_by(...). Minor, but the picks/standings pages generally present awards consistently.

Nits

• The whole "no N+1" property rests on an implicit coupling: by_player_for_week doesn't eager-load PlayerAward.award, so player_award.award.icon/.name in app/templates/v3/standings.html:39 only stay free because the route eagerly runs select(Award) (line 125) first and populates the identity map. That's correct today and the cost test locks it in, but it's fragile — if someone later drops the legend query, the page silently regresses to a lazy load per award row. A one-line comment on by_player_for_week (or a selectinload on award) would make the intent self-documenting.

• app/templates/v3/standings.html:74 computes the Total column as player.wins + player.bonus while games-back on line 78 uses player.total_points. They're identical today (total_points is defined as wins + bonus in app/models/player.py:65), but using player.total_points in both spots removes the drift risk and reads more clearly.

• by_player_for_week returns every PlayerAward row for the week, including per-game awards (in_your_face has a non-null game_id). A player who earned the same per-game award on two games this week would render two identical icons with the same alt/title. If that's intended (each is a distinct achievement), ignore this; otherwise you may want to de-dup by award for the icon strip. The tests only cover a single week-level award, so this case is unexercised.

• winning_pct returns None for a player with no games (app/models/player.py:147); the template's | float filter coerces that to 0.0, so the page is safe — worth a quick mental note that the same value is not guarded in standings_email.py's :.3f format (pre-existing, out of scope here).

Nice touch on the cost test asserting large == small rather than a magic number — that's the right way to express "doesn't scale with the pool."

<!-- claude-code-review --> This is a clean, well-tested PR. I inspected the route, the new `PlayerAward.by_player_for_week` helper, the models it leans on, and the template. ## Summary PR #514 adds a v3 standings page that renders one row per active player with season totals, this-week record, and award icons, plus a small nav z-index fix and a font-weight tweak. The data access is deliberately batched (one query for picks, one for awards, cache-warmed games/teams) and the query-count regression test is a genuinely good guard; I found no blocking issues. ## Worth fixing - `app/routers/v3.py:125` — the awards legend query runs unordered (`select(Award)`), so the icon legend row order is nondeterministic across DB restarts/backends. If the header legend should read in a stable order (e.g. by `point_value` or `slug`), add an `.order_by(...)`. Minor, but the picks/standings pages generally present awards consistently. ## Nits - The whole "no N+1" property rests on an implicit coupling: `by_player_for_week` doesn't eager-load `PlayerAward.award`, so `player_award.award.icon`/`.name` in `app/templates/v3/standings.html:39` only stay free because the route eagerly runs `select(Award)` (line 125) first and populates the identity map. That's correct today and the cost test locks it in, but it's fragile — if someone later drops the legend query, the page silently regresses to a lazy load per award row. A one-line comment on `by_player_for_week` (or a `selectinload` on `award`) would make the intent self-documenting. - `app/templates/v3/standings.html:74` computes the Total column as `player.wins + player.bonus` while games-back on line 78 uses `player.total_points`. They're identical today (`total_points` is defined as `wins + bonus` in `app/models/player.py:65`), but using `player.total_points` in both spots removes the drift risk and reads more clearly. - `by_player_for_week` returns *every* `PlayerAward` row for the week, including per-game awards (`in_your_face` has a non-null `game_id`). A player who earned the same per-game award on two games this week would render two identical icons with the same alt/title. If that's intended (each is a distinct achievement), ignore this; otherwise you may want to de-dup by award for the icon strip. The tests only cover a single week-level award, so this case is unexercised. - `winning_pct` returns `None` for a player with no games (`app/models/player.py:147`); the template's `| float` filter coerces that to `0.0`, so the page is safe — worth a quick mental note that the same value is *not* guarded in `standings_email.py`'s `:.3f` format (pre-existing, out of scope here). Nice touch on the cost test asserting `large == small` rather than a magic number — that's the right way to express "doesn't scale with the pool."

johnsturgeon added 1 commit 2026-09-11 20:46:53 +02:00

Addressed review feedback ... 8129d4c3a4

Trying to standardize on how I fetch the active player list

johnsturgeon added 1 commit 2026-09-11 20:56:47 +02:00

Fixed up a couple more review nits 84e86ae4d7

johnsturgeon merged commit eb1bbe7425 into main 2026-09-11 21:00:21 +02:00

johnsturgeon deleted branch implement-v3-standings-page-513 2026-09-11 21:00:22 +02:00

johnsturgeon referenced this pull request from a commit 2026-09-11 21:00:22 +02:00

Merge pull request 'Added a v3 standings page' (#514) from implement-v3-standings-page-513 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Kestra

  

bug

Something isn't working

  

enhancement

New feature or request

  

someday

Someday / Maybe

  

subtask

  

☁️ api

  

🎛️ infrastructure

  

🐞 sentry

  

📆 2025 Season

  

📝 pages

allpicks

Related to the All Picks page

  

📝 pages

picks

  

📝 pages

standings

  

🚀 performance

No labels

Kestra

bug

enhancement

someday

subtask

☁️ api

🎛️ infrastructure

🐞 sentry

📆 2025 Season

📝 pages

allpicks

📝 pages

picks

📝 pages

standings

🚀 performance

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

johnsturgeon/tgfp-web!514

No description provided.