From c98bd359f76e3df3cb65d683503f7c6195956d40 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 22 Oct 2026 20:43:18 -0400 Subject: [PATCH] fix: the admin requests list names who asked, not a cut-off user id (#6265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /admin/requests showed "by user f7e1ec86" — the first eight characters of the requester's id, a placeholder from the first build. The admin list now sends each requester's username, looked up for the whole page in one query, and the row reads "by ". The id stays as the fallback for a list that comes back without names. Co-Authored-By: Claude Opus 5.5 --- internal/api/admin_requests.go | 22 +++++++++++++ internal/api/admin_requests_test.go | 3 ++ internal/api/requests.go | 7 +++-- internal/db/dbq/users.sql.go | 31 +++++++++++++++++++ internal/db/queries/users.sql | 5 +++ web/src/lib/api/types.ts | 2 ++ web/src/routes/admin/requests/+page.svelte | 9 +++--- .../routes/admin/requests/requests.test.ts | 11 +++++++ 8 files changed, 83 insertions(+), 7 deletions(-) diff --git a/internal/api/admin_requests.go b/internal/api/admin_requests.go index 0b3f87cb..ae7f5b99 100644 --- a/internal/api/admin_requests.go +++ b/internal/api/admin_requests.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "errors" "net/http" @@ -59,15 +60,36 @@ func (h *handlers) handleListAdminRequests(w http.ResponseWriter, r *http.Reques } q := dbq.New(h.pool) + names := requesterNames(r.Context(), q, rows) out := make([]requestView, 0, len(rows)) for _, row := range rows { v := requestViewFrom(row) + v.Username = names[row.UserID] fillProgress(r.Context(), q, row, &v) out = append(out, v) } writeJSON(w, http.StatusOK, out) } +// requesterNames maps each request's user id to that user's username, in one +// query for the whole page. Like fillProgress, a failed lookup is swallowed: +// the names are a reporting nicety, and the page falls back to the id. +func requesterNames(ctx context.Context, q *dbq.Queries, rows []dbq.Request) map[pgtype.UUID]string { + ids := make([]pgtype.UUID, 0, len(rows)) + for _, row := range rows { + ids = append(ids, row.UserID) + } + names := make(map[pgtype.UUID]string, len(rows)) + users, err := q.ListUsernamesByIDs(ctx, ids) + if err != nil { + return names + } + for _, u := range users { + names[u.ID] = u.Username + } + return names +} + // approveRequestBody is the optional JSON body for POST /api/admin/requests/:id/approve. // profile_id picks the release-type profile an artist request is followed // on; omitted, the default profile. diff --git a/internal/api/admin_requests_test.go b/internal/api/admin_requests_test.go index 2d6ff88a..091a19da 100644 --- a/internal/api/admin_requests_test.go +++ b/internal/api/admin_requests_test.go @@ -90,6 +90,9 @@ func TestHandleListAdminRequests_DefaultStatus_Pending(t *testing.T) { if rv.Status != "pending" { t.Errorf("result status = %q, want pending", rv.Status) } + if rv.Username != "alice" { + t.Errorf("result username = %q, want alice (the requester)", rv.Username) + } } } diff --git a/internal/api/requests.go b/internal/api/requests.go index 826a886b..58c1920f 100644 --- a/internal/api/requests.go +++ b/internal/api/requests.go @@ -16,8 +16,11 @@ import ( // requestView is the JSON shape returned by all /api/requests handlers. // Field names follow the lowercase snake_case convention for /api/* responses. type requestView struct { - ID pgtype.UUID `json:"id"` - UserID pgtype.UUID `json:"user_id"` + ID pgtype.UUID `json:"id"` + UserID pgtype.UUID `json:"user_id"` + // Username names the requester. Only the admin list fills it; a user's + // own /requests page has no need to name them back to themselves. + Username string `json:"username,omitempty"` Status string `json:"status"` Kind string `json:"kind"` ArtistMBID string `json:"artist_mbid"` diff --git a/internal/db/dbq/users.sql.go b/internal/db/dbq/users.sql.go index 2dd5632f..f021d104 100644 --- a/internal/db/dbq/users.sql.go +++ b/internal/db/dbq/users.sql.go @@ -388,6 +388,37 @@ func (q *Queries) ListActiveUsersWithTimezones(ctx context.Context) ([]ListActiv return items, nil } +const listUsernamesByIDs = `-- name: ListUsernamesByIDs :many +SELECT id, username FROM users WHERE id = ANY($1::uuid[]) +` + +type ListUsernamesByIDsRow struct { + ID pgtype.UUID + Username string +} + +// Names a list's people in one round trip, e.g. who made each request on +// /admin/requests. +func (q *Queries) ListUsernamesByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([]ListUsernamesByIDsRow, error) { + rows, err := q.db.Query(ctx, listUsernamesByIDs, dollar_1) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListUsernamesByIDsRow + for rows.Next() { + var i ListUsernamesByIDsRow + if err := rows.Scan(&i.ID, &i.Username); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listUsers = `-- name: ListUsers :many SELECT id, username, display_name, is_admin, auto_approve_requests, debug_mode_enabled, created_at diff --git a/internal/db/queries/users.sql b/internal/db/queries/users.sql index a3dd36b3..e576b6b3 100644 --- a/internal/db/queries/users.sql +++ b/internal/db/queries/users.sql @@ -43,6 +43,11 @@ UPDATE users SET subsonic_password = $2 WHERE id = $1; -- name: GetUserByID :one SELECT * FROM users WHERE id = $1; +-- name: ListUsernamesByIDs :many +-- Names a list's people in one round trip, e.g. who made each request on +-- /admin/requests. +SELECT id, username FROM users WHERE id = ANY($1::uuid[]); + -- name: SetListenBrainzToken :exec UPDATE users SET listenbrainz_token = $2, diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 48dbd69e..7e66e9a2 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -168,6 +168,8 @@ export type CatalogueSearchResult = { export type MusicRequest = { id: string; user_id: string; + // The requester's username. Only the admin list sends it. + username?: string; status: RequestStatus; kind: RequestKind; artist_mbid: string; diff --git a/web/src/routes/admin/requests/+page.svelte b/web/src/routes/admin/requests/+page.svelte index d7e3ced4..dec17d87 100644 --- a/web/src/routes/admin/requests/+page.svelte +++ b/web/src/routes/admin/requests/+page.svelte @@ -49,13 +49,12 @@ let rejectOpen = $state(null); let rejectNotes = $state(''); - // The id is a UUID — first 8 chars is plenty to disambiguate without a real - // username, and matches what the polish pass will replace with the real - // username surface. + // Names the requester. The id is the fallback for a list that came back + // without names; its first 8 characters are enough to tell users apart. function rowMeta(r: MusicRequest): string { const when = new Date(r.requested_at).toLocaleDateString(); - const userBit = `by user ${r.user_id.slice(0, 8)}`; - return `${userBit} · ${when}`; + const who = r.username || `user ${r.user_id.slice(0, 8)}`; + return `by ${who} · ${when}`; } async function invalidate() { diff --git a/web/src/routes/admin/requests/requests.test.ts b/web/src/routes/admin/requests/requests.test.ts index 251674ac..a748ce10 100644 --- a/web/src/routes/admin/requests/requests.test.ts +++ b/web/src/routes/admin/requests/requests.test.ts @@ -125,4 +125,15 @@ describe('/admin/requests', () => { ]); expect(screen.getByText(/wants the album/i)).toBeInTheDocument(); }); + + test('A row names who asked for it', () => { + setup([{ ...baseRow, username: 'alice' }]); + expect(screen.getByText(/^by alice · /)).toBeInTheDocument(); + expect(screen.queryByText(/u1234567/)).not.toBeInTheDocument(); + }); + + test('A row without a name falls back to the user id', () => { + setup(); + expect(screen.getByText(/^by user u1234567 · /)).toBeInTheDocument(); + }); });