fix: the admin requests list names who asked, not a cut-off user id (#6265)
release / govulncheck (push) Successful in 37s
release / web (push) Successful in 1m39s
release / go (push) Successful in 1m51s
release / integration (push) Failing after 5m37s
release / android (push) Successful in 5m54s
release / Build signed APK (releases and dev) (push) Successful in 5m57s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 37s
release / web (push) Successful in 1m39s
release / go (push) Successful in 1m51s
release / integration (push) Failing after 5m37s
release / android (push) Successful in 5m54s
release / Build signed APK (releases and dev) (push) Successful in 5m57s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
/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 <username>". The id stays as the fallback for a list that comes back without names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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"`
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -49,13 +49,12 @@
|
||||
let rejectOpen = $state<string | null>(null);
|
||||
let rejectNotes = $state<string>('');
|
||||
|
||||
// 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() {
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user