From 7f203b92454c6cb78778b8f011a8867f0f261114 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 27 Sep 2026 13:01:12 +0800 Subject: [PATCH] =?UTF-8?q?fix(api):=20ListUsers=20=E7=9A=84=20total=20?= =?UTF-8?q?=E6=8C=89=E5=90=8C=E4=B8=80=E7=BB=84=E8=BF=87=E6=BB=A4=E6=9D=A1?= =?UTF-8?q?=E4=BB=B6=E8=AE=A1=E6=95=B0=EF=BC=8C=E9=9D=A2=E6=9D=BF=E4=B8=8D?= =?UTF-8?q?=E5=86=8D=E7=BF=BB=E5=87=BA=E7=A9=BA=E9=A1=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/api/pgrepo.go | 17 ++++++------- internal/api/repo.go | 4 ++-- internal/pgint/paging_test.go | 45 +++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 05ee4f5..df42258 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -1850,17 +1850,9 @@ func (p *PGRepo) DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID // ---- user admin (spec §7, admin-only) ---- // ListUsers returns a page of non-deleted users matching the optional filters, -// newest first. total is the unfiltered count so the admin page can render -// pagination without a second round-trip. +// newest first. total counts every user the same filters match, so the admin +// page can size its pagination without a second round-trip. func (p *PGRepo) ListUsers(ctx context.Context, opts ListUsersOpts) ([]UserView, int, error) { - var total int - { - q := `SELECT count(*) FROM users WHERE deleted_at IS NULL` - if err := p.db.QueryRowContext(ctx, q).Scan(&total); err != nil { - return nil, 0, err - } - } - limit := opts.Limit if limit <= 0 || limit > 100 { limit = 20 @@ -1893,6 +1885,11 @@ func (p *PGRepo) ListUsers(ctx context.Context, opts ListUsersOpts) ([]UserView, where += ` AND u.disabled = false` } + var total int + if err := p.db.QueryRowContext(ctx, `SELECT count(*) FROM users u`+where, args...).Scan(&total); err != nil { + return nil, 0, err + } + q := `SELECT u.id, u.username, COALESCE(u.email, ''), u.role::text, u.disabled, u.email_verified, u.created_at, u.updated_at, diff --git a/internal/api/repo.go b/internal/api/repo.go index 2c0edad..d9e6c80 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -734,8 +734,8 @@ type Repo interface { // ---- user admin (spec §7, admin-only) ---- // ListUsers returns a page of non-deleted users matching the optional filters, - // newest first. total is the unfiltered count so the admin page can render - // pagination without a second round-trip. + // newest first. total counts every user the same filters match, so the admin + // page can size its pagination without a second round-trip. ListUsers(ctx context.Context, opts ListUsersOpts) ([]UserView, int, error) // UserDetail loads one user with its linked MC accounts, or ErrNotFound. // A deleted user is returned (the row lives for audit) but flagged. diff --git a/internal/pgint/paging_test.go b/internal/pgint/paging_test.go index 07c7308..767273f 100644 --- a/internal/pgint/paging_test.go +++ b/internal/pgint/paging_test.go @@ -192,3 +192,48 @@ func TestBackupListPaging(t *testing.T) { t.Fatalf("other's s2 = %s of %d (%v), want nothing", idsOf(vs), total, err) } } + +// ListUsers reports as total every live user the same filters match, past the +// last page too, so the admin page does not size its pagination from the whole +// table and offer pages that come back empty. +func TestListUsersTotalFollowsFilters(t *testing.T) { + ctx := context.Background() + tag := suffix(t) + newUser(t, "user", "lu"+tag) + newUser(t, "admin", "lu"+tag) + bob := newUser(t, "user", "lu"+tag) + newUser(t, "user", "other") // live, but matches no query below + gone := newUser(t, "user", "lu"+tag) + if err := repo.SetUserDisabled(ctx, bob.ID, true); err != nil { + t.Fatalf("SetUserDisabled: %v", err) + } + if err := repo.DeleteUser(ctx, gone.ID, "pgint"); err != nil { + t.Fatalf("DeleteUser: %v", err) + } + var live int + if err := db.QueryRowContext(ctx, `SELECT count(*) FROM users WHERE deleted_at IS NULL`).Scan(&live); err != nil { + t.Fatalf("count: %v", err) + } + + for _, c := range []struct { + name string + opts api.ListUsersOpts + rows, all int + }{ + {"no filter", api.ListUsersOpts{Limit: 1}, 1, live}, + {"query", api.ListUsersOpts{Query: tag, Limit: 2}, 2, 3}, + {"query past the last page", api.ListUsersOpts{Query: tag, Limit: 2, Offset: 4}, 0, 3}, + {"query and role", api.ListUsersOpts{Query: tag, Role: "admin"}, 1, 1}, + {"query and disabled", api.ListUsersOpts{Query: tag, Hidden: "true"}, 1, 1}, + {"query and enabled", api.ListUsersOpts{Query: tag, Hidden: "false"}, 2, 2}, + } { + users, total, err := repo.ListUsers(ctx, c.opts) + if err != nil || len(users) != c.rows || total != c.all { + t.Errorf("%s: %d rows, total %d, err %v; want %d rows, total %d", c.name, len(users), total, err, c.rows, c.all) + } + } + users, _, err := repo.ListUsers(ctx, api.ListUsersOpts{Query: tag, Hidden: "true"}) + if err != nil || len(users) != 1 || users[0].ID != bob.ID { + t.Fatalf("disabled filter = %+v, %v; want only %s", users, err, bob.Username) + } +}