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(); + }); });