fix: admin roster returns one row per profile, not per performance#620
Merged
Conversation
The all-bands admin query LEFT JOINed to performances with no GROUP BY, ordered newest-event-first, and applied LIMIT 200 — so any band whose only performances were on older events sorted past the limit and silently vanished from the roster (Adelleda, whose sole set was lwbc07 2024, was uneditable). GROUP BY bp.id bounds the result by roster size; last-played context comes from the band's most recent performance via SQLite's bare-column-with-MAX semantics (validated against production D1). Default limit 200 -> 500 (prod already has 218 active profiles). Roster deletes now always send profile_N ids, which hit the existing has-performances 409 guard instead of deleting an arbitrary performance row. Closes #618 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BreakableHoodie
enabled auto-merge (squash)
July 17, 2026 13:52
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The admin roster silently truncated: the all-bands query returned one row per performance (LEFT JOIN, no GROUP BY), ordered newest-event-first, and applied LIMIT 200 — so bands whose only performances were on older events fell past the cutoff before RosterTab's client-side dedupe ever saw them. Reported by Dre: Adelleda (sole set on lwbc07, Feb 2024) was visible on the public site but uneditable in admin. The branch now does
GROUP BY bp.id— one row per active profile — so the result is bounded by roster size, not performance count.Closes #618
What changed
functions/api/admin/bands.jsevent_idbranch:GROUP BY bp.id; "last played" context (event/venue name, date) sourced from the band's most recent performance via SQLite's bare-column-with-MAX()semantics — validated against production D1 directly (Burnaby, on Vols 1+2, resolves to Vol 2 with matching event name; Suplex with 3 sets resolves to Vol 17); synthetic id is now alwaysprofile_N(verified no consumer reads it as a performance id — RosterTab and ArtistPicker key offband_profile_id); default limit 200 → 500 (prod already holds 218 active profiles, over the old default); the false "no pagination-boundary risk" comment rewritten to state the real guaranteefunctions/api/admin/bands/__tests__/bands.test.jsDeliberate behaviour change: RosterTab's Delete now always sends
profile_N, which hits the existing "has performances → 409" guard instead of deleting one arbitrary performance row — strictly safer, and consistent with the guardbulk.jsalready enforces.Out of scope: the
event_idbranch (untouched);is_active = 1filter stays (#619 covers surfacing inactive profiles).Security / correctness notes
Admin-only endpoint behind
checkPermission("viewer"), unchanged. Correctness:follower_countcorrelated subquery verified under GROUP BY via the existing dedicated test.Verification
bands.test.js81/81 (78 + 3 new); follower-count test greenvalidate:openapi: n/a — response shape field-compatible (same columns, per-profile rows)Built by Sonny · Reviewed by Theo · 🤖 Claude Code