Skip to content

Commit 88f3691

Browse files
f0sself0ssel
authored
feat: add count to get users endpoint (coder#5016)
1 parent 49b340e commit 88f3691

File tree

25 files changed

+420
-478
lines changed

25 files changed

+420
-478
lines changed

‎cli/list.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,12 @@ func list() *cobra.Command {
102102
_, _ = fmt.Fprintln(cmd.ErrOrStderr())
103103
return nil
104104
}
105-
users, err := client.Users(cmd.Context(), codersdk.UsersRequest{})
105+
userRes, err := client.Users(cmd.Context(), codersdk.UsersRequest{})
106106
if err != nil {
107107
return err
108108
}
109109
usersByID := map[uuid.UUID]codersdk.User{}
110-
for _, user := range users {
110+
for _, user := range userRes.Users {
111111
usersByID[user.ID] = user
112112
}
113113

‎cli/userlist.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,20 @@ func userList() *cobra.Command {
3030
if err != nil {
3131
return err
3232
}
33-
users, err := client.Users(cmd.Context(), codersdk.UsersRequest{})
33+
res, err := client.Users(cmd.Context(), codersdk.UsersRequest{})
3434
if err != nil {
3535
return err
3636
}
3737

3838
out := ""
3939
switch outputFormat {
4040
case "table", "":
41-
out, err = cliui.DisplayTable(users, "Username", columns)
41+
out, err = cliui.DisplayTable(res.Users, "Username", columns)
4242
if err != nil {
4343
return xerrors.Errorf("render table: %w", err)
4444
}
4545
case "json":
46-
outBytes, err := json.Marshal(users)
46+
outBytes, err := json.Marshal(res.Users)
4747
if err != nil {
4848
return xerrors.Errorf("marshal users to JSON: %w", err)
4949
}

‎coderd/coderd.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,6 @@ func New(options *Options) *API {
431431
)
432432
r.Post("/", api.postUser)
433433
r.Get("/", api.users)
434-
r.Get("/count", api.userCount)
435434
r.Post("/logout", api.postLogout)
436435
// These routes query information about site wide roles.
437436
r.Route("/roles", func(r chi.Router) {

‎coderd/coderdtest/authorize.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
245245

246246
// Endpoints that use the SQLQuery filter.
247247
"GET:/api/v2/workspaces/": {StatusCode: http.StatusOK, NoAuthorize: true},
248-
"GET:/api/v2/users/count": {StatusCode: http.StatusOK, NoAuthorize: true},
249248
}
250249

251250
// Routes like proxy routes support all HTTP methods. A helper func to expand

‎coderd/database/databasefake/databasefake.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ func (q *fakeQuerier) UpdateUserDeletedByID(_ context.Context, params database.U
538538
return sql.ErrNoRows
539539
}
540540

541-
func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.User, error) {
541+
func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.GetUsersRow, error) {
542542
q.mutex.RLock()
543543
defer q.mutex.RUnlock()
544544

@@ -579,7 +579,7 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
579579

580580
// If no users after the time, then we return an empty list.
581581
if !found {
582-
return nil, sql.ErrNoRows
582+
return []database.GetUsersRow{}, nil
583583
}
584584
}
585585

@@ -617,9 +617,11 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
617617
users = usersFilteredByRole
618618
}
619619

620+
beforePageCount := len(users)
621+
620622
if params.OffsetOpt > 0 {
621623
if int(params.OffsetOpt) > len(users)-1 {
622-
return nil, sql.ErrNoRows
624+
return []database.GetUsersRow{}, nil
623625
}
624626
users = users[params.OffsetOpt:]
625627
}
@@ -631,7 +633,30 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
631633
users = users[:params.LimitOpt]
632634
}
633635

634-
return users, nil
636+
return convertUsers(users, int64(beforePageCount)), nil
637+
}
638+
639+
func convertUsers(users []database.User, count int64) []database.GetUsersRow {
640+
rows := make([]database.GetUsersRow, len(users))
641+
for i, u := range users {
642+
rows[i] = database.GetUsersRow{
643+
ID: u.ID,
644+
Email: u.Email,
645+
Username: u.Username,
646+
HashedPassword: u.HashedPassword,
647+
CreatedAt: u.CreatedAt,
648+
UpdatedAt: u.UpdatedAt,
649+
Status: u.Status,
650+
RBACRoles: u.RBACRoles,
651+
LoginType: u.LoginType,
652+
AvatarURL: u.AvatarURL,
653+
Deleted: u.Deleted,
654+
LastSeenAt: u.LastSeenAt,
655+
Count: count,
656+
}
657+
}
658+
659+
return rows
635660
}
636661

637662
func (q *fakeQuerier) GetUsersByIDs(_ context.Context, ids []uuid.UUID) ([]database.User, error) {

‎coderd/database/modelmethods.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,25 @@ func (User) RBACObject() rbac.Object {
7171
func (License) RBACObject() rbac.Object {
7272
return rbac.ResourceLicense
7373
}
74+
75+
func ConvertUserRows(rows []GetUsersRow) []User {
76+
users := make([]User, len(rows))
77+
for i, r := range rows {
78+
users[i] = User{
79+
ID: r.ID,
80+
Email: r.Email,
81+
Username: r.Username,
82+
HashedPassword: r.HashedPassword,
83+
CreatedAt: r.CreatedAt,
84+
UpdatedAt: r.UpdatedAt,
85+
Status: r.Status,
86+
RBACRoles: r.RBACRoles,
87+
LoginType: r.LoginType,
88+
AvatarURL: r.AvatarURL,
89+
Deleted: r.Deleted,
90+
LastSeenAt: r.LastSeenAt,
91+
}
92+
}
93+
94+
return users
95+
}

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 21 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/users.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ WHERE
128128

129129
-- name: GetUsers :many
130130
SELECT
131-
*
131+
*, COUNT(*) OVER() AS count
132132
FROM
133133
users
134134
WHERE

‎coderd/telemetry/telemetry.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,11 @@ func (r *remoteReporter) createSnapshot() (*Snapshot, error) {
350350
return nil
351351
})
352352
eg.Go(func() error {
353-
users, err := r.options.Database.GetUsers(ctx, database.GetUsersParams{})
353+
userRows, err := r.options.Database.GetUsers(ctx, database.GetUsersParams{})
354354
if err != nil {
355355
return xerrors.Errorf("get users: %w", err)
356356
}
357+
users := database.ConvertUserRows(userRows)
357358
var firstUser database.User
358359
for _, dbUser := range users {
359360
if dbUser.Status != database.UserStatusActive {

‎coderd/users.go

Lines changed: 14 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -198,27 +198,32 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
198198
return
199199
}
200200

201-
users, err := api.Database.GetUsers(ctx, database.GetUsersParams{
201+
userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{
202202
AfterID: paginationParams.AfterID,
203203
OffsetOpt: int32(paginationParams.Offset),
204204
LimitOpt: int32(paginationParams.Limit),
205205
Search: params.Search,
206206
Status: params.Status,
207207
RbacRole: params.RbacRole,
208208
})
209-
if errors.Is(err, sql.ErrNoRows) {
210-
httpapi.Write(ctx, rw, http.StatusOK, []codersdk.User{})
211-
return
212-
}
213209
if err != nil {
214210
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
215211
Message: "Internal error fetching users.",
216212
Detail: err.Error(),
217213
})
218214
return
219215
}
216+
// GetUsers does not return ErrNoRows because it uses a window function to get the count.
217+
// So we need to check if the userRows is empty and return an empty array if so.
218+
if len(userRows) == 0 {
219+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.GetUsersResponse{
220+
Users: []codersdk.User{},
221+
Count: 0,
222+
})
223+
return
224+
}
220225

221-
users, err = AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, users)
226+
users, err := AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, database.ConvertUserRows(userRows))
222227
if err != nil {
223228
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
224229
Message: "Internal error fetching users.",
@@ -248,42 +253,9 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
248253
}
249254

250255
render.Status(r, http.StatusOK)
251-
render.JSON(rw, r, convertUsers(users, organizationIDsByUserID))
252-
}
253-
254-
func (api *API) userCount(rw http.ResponseWriter, r *http.Request) {
255-
ctx := r.Context()
256-
query := r.URL.Query().Get("q")
257-
params, errs := userSearchQuery(query)
258-
if len(errs) > 0 {
259-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
260-
Message: "Invalid user search query.",
261-
Validations: errs,
262-
})
263-
return
264-
}
265-
266-
sqlFilter, err := api.HTTPAuth.AuthorizeSQLFilter(r, rbac.ActionRead, rbac.ResourceUser.Type)
267-
if err != nil {
268-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
269-
Message: "Internal error preparing sql filter.",
270-
Detail: err.Error(),
271-
})
272-
return
273-
}
274-
275-
count, err := api.Database.GetAuthorizedUserCount(ctx, database.GetFilteredUserCountParams{
276-
Search: params.Search,
277-
Status: params.Status,
278-
RbacRole: params.RbacRole,
279-
}, sqlFilter)
280-
if err != nil {
281-
httpapi.InternalServerError(rw, err)
282-
return
283-
}
284-
285-
httpapi.Write(ctx, rw, http.StatusOK, codersdk.UserCountResponse{
286-
Count: count,
256+
render.JSON(rw, r, codersdk.GetUsersResponse{
257+
Users: convertUsers(users, organizationIDsByUserID),
258+
Count: int(userRows[0].Count),
287259
})
288260
}
289261

0 commit comments

Comments
 (0)