From 08fa3a7c530ab7267d32fbc25d83053d40203a08 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 13:17:27 +0000 Subject: [PATCH 01/10] feat: add filter by status on GET /users --- coderd/database/databasefake/databasefake.go | 11 +++++ coderd/database/queries.sql.go | 17 ++++--- coderd/database/queries/users.sql | 3 ++ coderd/users.go | 10 ++-- coderd/users_test.go | 48 +++++++++++++++----- codersdk/users.go | 17 ++++--- 6 files changed, 78 insertions(+), 28 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index b48ede661296c..5f8fadb88a90c 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -225,6 +225,17 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams } users = users[:params.LimitOpt] } + + if params.Status != "" { + usersFilteredByStatus := make([]database.User, 0, len(users)) + for i, user := range users { + if user.Status == params.Status { + usersFilteredByStatus = append(usersFilteredByStatus, users[i]) + } + } + users = usersFilteredByStatus + } + tmp := make([]database.User, len(users)) copy(tmp, users) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5f0574812b79c..32b4a54abb4bf 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1884,29 +1884,34 @@ WHERE WHEN $2 :: text != '' THEN ( email LIKE concat('%', $2, '%') OR username LIKE concat('%', $2, '%') + ) + WHEN $3 :: user_status != '' THEN ( + status = $3 ) ELSE true END ORDER BY -- Deterministic and consistent ordering of all users, even if they share -- a timestamp. This is to ensure consistent pagination. - (created_at, id) ASC OFFSET $3 + (created_at, id) ASC OFFSET $4 LIMIT -- A null limit means "no limit", so -1 means return all - NULLIF($4 :: int, -1) + NULLIF($5 :: int, -1) ` type GetUsersParams struct { - AfterUser uuid.UUID `db:"after_user" json:"after_user"` - Search string `db:"search" json:"search"` - OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` - LimitOpt int32 `db:"limit_opt" json:"limit_opt"` + AfterUser uuid.UUID `db:"after_user" json:"after_user"` + Search string `db:"search" json:"search"` + Status UserStatus `db:"status" json:"status"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]User, error) { rows, err := q.db.QueryContext(ctx, getUsers, arg.AfterUser, arg.Search, + arg.Status, arg.OffsetOpt, arg.LimitOpt, ) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index bf99df6a3f785..cb81cef8cb19f 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -80,6 +80,9 @@ WHERE WHEN @search :: text != '' THEN ( email LIKE concat('%', @search, '%') OR username LIKE concat('%', @search, '%') + ) + WHEN @status :: user_status != '' THEN ( + status = @status ) ELSE true END diff --git a/coderd/users.go b/coderd/users.go index a38af8ba12f63..e17a96bfa0c21 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -90,10 +90,11 @@ func (api *api) postFirstUser(rw http.ResponseWriter, r *http.Request) { func (api *api) users(rw http.ResponseWriter, r *http.Request) { var ( - afterArg = r.URL.Query().Get("after_user") - limitArg = r.URL.Query().Get("limit") - offsetArg = r.URL.Query().Get("offset") - searchName = r.URL.Query().Get("search") + afterArg = r.URL.Query().Get("after_user") + limitArg = r.URL.Query().Get("limit") + offsetArg = r.URL.Query().Get("offset") + searchName = r.URL.Query().Get("search") + statusFilter = r.URL.Query().Get("status") ) // createdAfter is a user uuid. @@ -136,6 +137,7 @@ func (api *api) users(rw http.ResponseWriter, r *http.Request) { OffsetOpt: int32(offset), LimitOpt: int32(pageLimit), Search: searchName, + Status: database.UserStatus(statusFilter), }) if err != nil { diff --git a/coderd/users_test.go b/coderd/users_test.go index 8ee4db1c6f80e..f4def83e2db52 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -328,18 +328,44 @@ func TestUserByName(t *testing.T) { func TestGetUsers(t *testing.T) { t.Parallel() - client := coderdtest.New(t, nil) - user := coderdtest.CreateFirstUser(t, client) - client.CreateUser(context.Background(), codersdk.CreateUserRequest{ - Email: "alice@email.com", - Username: "alice", - Password: "password", - OrganizationID: user.OrganizationID, + t.Run("AllUsers", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + client.CreateUser(context.Background(), codersdk.CreateUserRequest{ + Email: "alice@email.com", + Username: "alice", + Password: "password", + OrganizationID: user.OrganizationID, + }) + // No params is all users + users, err := client.Users(context.Background(), codersdk.UsersRequest{}) + require.NoError(t, err) + require.Len(t, users, 2) + }) + t.Run("ActiveUsers", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + client.CreateUser(context.Background(), codersdk.CreateUserRequest{ + Email: "alice@email.com", + Username: "alice", + Password: "password", + OrganizationID: user.OrganizationID, + }) + suspendedUser, _ := client.CreateUser(context.Background(), codersdk.CreateUserRequest{ + Email: "bruno@email.com", + Username: "bruno", + Password: "password", + OrganizationID: user.OrganizationID, + }) + client.SuspendUser(context.Background(), suspendedUser.ID) + users, err := client.Users(context.Background(), codersdk.UsersRequest{ + Status: string(codersdk.UserStatusSuspended), + }) + require.NoError(t, err) + require.Len(t, users, 2) }) - // No params is all users - users, err := client.Users(context.Background(), codersdk.UsersRequest{}) - require.NoError(t, err) - require.Len(t, users, 2) } func TestOrganizationsByUser(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index 38506f03cd192..925b269b7118c 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -14,6 +14,13 @@ import ( // Me is used as a replacement for your own ID. var Me = uuid.Nil +type UserStatus string + +const ( + UserStatusActive UserStatus = "active" + UserStatusSuspended UserStatus = "suspended" +) + type UsersRequest struct { AfterUser uuid.UUID `json:"after_user"` Search string `json:"search"` @@ -26,15 +33,10 @@ type UsersRequest struct { // To get the next page, use offset=*. // Offset is 0 indexed, so the first record sits at offset 0. Offset int `json:"offset"` + // Filter users by status + Status string `json:"status"` } -type UserStatus string - -const ( - UserStatusActive UserStatus = "active" - UserStatusSuspended UserStatus = "suspended" -) - // User represents a user in Coder. type User struct { ID uuid.UUID `json:"id" validate:"required"` @@ -242,6 +244,7 @@ func (c *Client) Users(ctx context.Context, req UsersRequest) ([]User, error) { } q.Set("offset", strconv.Itoa(req.Offset)) q.Set("search", req.Search) + q.Set("status", req.Status) r.URL.RawQuery = q.Encode() }) if err != nil { From 39b1dfbf24fd9da1488522710cd6f28cf1898de5 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 13:22:52 +0000 Subject: [PATCH 02/10] fix: generate TS types --- site/src/api/typesGenerated.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 268d1f837c347..f46dd3662021c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -76,21 +76,22 @@ export interface TemplateVersion { readonly job: ProvisionerJob } -// From codersdk/users.go:17:6. +// From codersdk/users.go:24:6. export interface UsersRequest { readonly search: string readonly limit: number readonly offset: number + readonly status: string } -// From codersdk/users.go:39:6. +// From codersdk/users.go:41:6. export interface User { readonly email: string readonly username: string readonly status: UserStatus } -// From codersdk/users.go:47:6. +// From codersdk/users.go:49:6. export interface CreateFirstUserRequest { readonly email: string readonly username: string @@ -98,41 +99,41 @@ export interface CreateFirstUserRequest { readonly organization: string } -// From codersdk/users.go:60:6. +// From codersdk/users.go:62:6. export interface CreateUserRequest { readonly email: string readonly username: string readonly password: string } -// From codersdk/users.go:67:6. +// From codersdk/users.go:69:6. export interface UpdateUserProfileRequest { readonly email: string readonly username: string } -// From codersdk/users.go:73:6. +// From codersdk/users.go:75:6. export interface LoginWithPasswordRequest { readonly email: string readonly password: string } -// From codersdk/users.go:79:6. +// From codersdk/users.go:81:6. export interface LoginWithPasswordResponse { readonly session_token: string } -// From codersdk/users.go:84:6. +// From codersdk/users.go:86:6. export interface GenerateAPIKeyResponse { readonly key: string } -// From codersdk/users.go:88:6. +// From codersdk/users.go:90:6. export interface CreateOrganizationRequest { readonly name: string } -// From codersdk/users.go:93:6. +// From codersdk/users.go:95:6. export interface AuthMethods { readonly password: boolean readonly github: boolean @@ -235,7 +236,7 @@ export type ParameterScope = "organization" | "template" | "user" | "workspace" // From codersdk/provisionerdaemons.go:26:6. export type ProvisionerJobStatus = "pending" | "running" | "succeeded" | "canceling" | "canceled" | "failed" -// From codersdk/users.go:31:6. +// From codersdk/users.go:17:6. export type UserStatus = "active" | "suspended" // From codersdk/workspaceresources.go:15:6. From f4465d03fe0c75560c6297283a0e6684e0dd6c93 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 13:35:35 +0000 Subject: [PATCH 03/10] fix: fix tests --- coderd/users_test.go | 6 +++--- codersdk/users.go | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index f4def83e2db52..38e6fb20cc8e4 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -353,15 +353,15 @@ func TestGetUsers(t *testing.T) { Password: "password", OrganizationID: user.OrganizationID, }) - suspendedUser, _ := client.CreateUser(context.Background(), codersdk.CreateUserRequest{ + client.CreateUser(context.Background(), codersdk.CreateUserRequest{ Email: "bruno@email.com", Username: "bruno", Password: "password", OrganizationID: user.OrganizationID, }) - client.SuspendUser(context.Background(), suspendedUser.ID) + client.SuspendUser(context.Background(), user.UserID) users, err := client.Users(context.Background(), codersdk.UsersRequest{ - Status: string(codersdk.UserStatusSuspended), + Status: string(codersdk.UserStatusActive), }) require.NoError(t, err) require.Len(t, users, 2) diff --git a/codersdk/users.go b/codersdk/users.go index 925b269b7118c..221189aed4ca6 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -166,6 +166,7 @@ func (c *Client) SuspendUser(ctx context.Context, userID uuid.UUID) (User, error if res.StatusCode != http.StatusOK { return User{}, readBodyAsError(res) } + var user User return user, json.NewDecoder(res.Body).Decode(&user) } From 9a5a74bc7944a0af6536cbf6e36fba0e22b95044 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 13:36:55 +0000 Subject: [PATCH 04/10] fix: status filter o users sql --- coderd/database/queries/users.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index cb81cef8cb19f..94f15288e7a8a 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -81,6 +81,8 @@ WHERE email LIKE concat('%', @search, '%') OR username LIKE concat('%', @search, '%') ) + ELSE true + AND CASE WHEN @status :: user_status != '' THEN ( status = @status ) From 83f29c772cafafd0b14bdd01caae04b5875026ee Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 13:42:48 +0000 Subject: [PATCH 05/10] fix: fix query and add comments --- coderd/database/queries.sql.go | 7 +++++++ coderd/database/queries/users.sql | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 32b4a54abb4bf..a45929b401ecb 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1880,16 +1880,23 @@ WHERE ) ELSE true END + -- Start filters + -- Filter by name, email or username AND CASE WHEN $2 :: text != '' THEN ( email LIKE concat('%', $2, '%') OR username LIKE concat('%', $2, '%') ) + ELSE true + END + -- Filter by status + AND CASE WHEN $3 :: user_status != '' THEN ( status = $3 ) ELSE true END + -- End of filters ORDER BY -- Deterministic and consistent ordering of all users, even if they share -- a timestamp. This is to ensure consistent pagination. diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 94f15288e7a8a..eb365b04035f2 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -76,18 +76,23 @@ WHERE ) ELSE true END + -- Start filters + -- Filter by name, email or username AND CASE WHEN @search :: text != '' THEN ( email LIKE concat('%', @search, '%') OR username LIKE concat('%', @search, '%') ) ELSE true + END + -- Filter by status AND CASE WHEN @status :: user_status != '' THEN ( status = @status ) ELSE true END + -- End of filters ORDER BY -- Deterministic and consistent ordering of all users, even if they share -- a timestamp. This is to ensure consistent pagination. From 3f25967cda26e0a51e246957c7287f38f0c8fde0 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 14:27:43 +0000 Subject: [PATCH 06/10] fix: make status type on filter query --- coderd/database/databasefake/databasefake.go | 2 +- coderd/database/queries.sql.go | 12 ++++++------ coderd/database/queries/users.sql | 2 +- coderd/users.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 5f8fadb88a90c..6fb03dc96962a 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -229,7 +229,7 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams if params.Status != "" { usersFilteredByStatus := make([]database.User, 0, len(users)) for i, user := range users { - if user.Status == params.Status { + if params.Status == string(user.Status) { usersFilteredByStatus = append(usersFilteredByStatus, users[i]) } } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a45929b401ecb..0e78692b85aee 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1891,7 +1891,7 @@ WHERE END -- Filter by status AND CASE - WHEN $3 :: user_status != '' THEN ( + WHEN $3 :: text != '' THEN ( status = $3 ) ELSE true @@ -1907,11 +1907,11 @@ LIMIT ` type GetUsersParams struct { - AfterUser uuid.UUID `db:"after_user" json:"after_user"` - Search string `db:"search" json:"search"` - Status UserStatus `db:"status" json:"status"` - OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` - LimitOpt int32 `db:"limit_opt" json:"limit_opt"` + AfterUser uuid.UUID `db:"after_user" json:"after_user"` + Search string `db:"search" json:"search"` + Status string `db:"status" json:"status"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]User, error) { diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index eb365b04035f2..2514112733cf6 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -87,7 +87,7 @@ WHERE END -- Filter by status AND CASE - WHEN @status :: user_status != '' THEN ( + WHEN @status :: text != '' THEN ( status = @status ) ELSE true diff --git a/coderd/users.go b/coderd/users.go index e17a96bfa0c21..81176584b88c1 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -137,7 +137,7 @@ func (api *api) users(rw http.ResponseWriter, r *http.Request) { OffsetOpt: int32(offset), LimitOpt: int32(pageLimit), Search: searchName, - Status: database.UserStatus(statusFilter), + Status: statusFilter, }) if err != nil { From 677cb6f9e958aa441cb0c7ff230b15b6690d03b0 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 14:38:31 +0000 Subject: [PATCH 07/10] fix: add comment about @status being a text --- coderd/database/queries/users.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 2514112733cf6..03fdea6c97b7b 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -87,6 +87,8 @@ WHERE END -- Filter by status AND CASE + -- @status needs to be a text because it can be empty, If it was + -- user_status enum, it would not. WHEN @status :: text != '' THEN ( status = @status ) From 18297eace2b7b74c6d9d9f72b5864ae52427ddac Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 28 Apr 2022 13:31:37 -0500 Subject: [PATCH 08/10] chore: Update coderd/users_test.go Co-authored-by: Steven Masley --- coderd/users_test.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 788e61a158d3c..c7f84f15588e2 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -349,25 +349,34 @@ func TestGetUsers(t *testing.T) { t.Run("ActiveUsers", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) - user := coderdtest.CreateFirstUser(t, client) - client.CreateUser(context.Background(), codersdk.CreateUserRequest{ + first := coderdtest.CreateFirstUser(t, client) + active := make([]codersdk.User, 0) + alice, err := client.CreateUser(context.Background(), codersdk.CreateUserRequest{ Email: "alice@email.com", Username: "alice", Password: "password", - OrganizationID: user.OrganizationID, + OrganizationID: first.OrganizationID, }) - client.CreateUser(context.Background(), codersdk.CreateUserRequest{ + require.NoError(t, err) + active = append(active, alice) + + bruno, err := client.CreateUser(context.Background(), codersdk.CreateUserRequest{ Email: "bruno@email.com", Username: "bruno", Password: "password", - OrganizationID: user.OrganizationID, + OrganizationID: first.OrganizationID, }) - client.SuspendUser(context.Background(), user.UserID) + require.NoError(t, err) + active = append(active, bruno) + + _, err = client.SuspendUser(context.Background(), first.UserID) + require.NoError(t, err) + users, err := client.Users(context.Background(), codersdk.UsersRequest{ Status: string(codersdk.UserStatusActive), }) require.NoError(t, err) - require.Len(t, users, 2) + require.ElementsMatch(t, active, users) }) } From e9dd4272f36921aac3fe51e31949556b9fc477ec Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 18:47:59 +0000 Subject: [PATCH 09/10] fix: cast status as user_status inside of the when --- coderd/database/queries.sql.go | 4 +++- coderd/database/queries/users.sql | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index f836079abc7c4..b378e70cf8fbe 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1930,8 +1930,10 @@ WHERE END -- Filter by status AND CASE + -- @status needs to be a text because it can be empty, If it was + -- user_status enum, it would not. WHEN $3 :: text != '' THEN ( - status = $3 + status = $3 :: user_status ) ELSE true END diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 03fdea6c97b7b..c97858bcb6ebe 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -90,7 +90,7 @@ WHERE -- @status needs to be a text because it can be empty, If it was -- user_status enum, it would not. WHEN @status :: text != '' THEN ( - status = @status + status = @status :: user_status ) ELSE true END From 4a7ffd61951fe2a020abe68e6750047a2e8bc565 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 28 Apr 2022 20:49:18 +0000 Subject: [PATCH 10/10] refactor: move status filter --- coderd/database/databasefake/databasefake.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 0e6e2b1fc2d8d..44dd7f043467b 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -212,6 +212,16 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams users = tmp } + if params.Status != "" { + usersFilteredByStatus := make([]database.User, 0, len(users)) + for i, user := range users { + if params.Status == string(user.Status) { + usersFilteredByStatus = append(usersFilteredByStatus, users[i]) + } + } + users = usersFilteredByStatus + } + if params.OffsetOpt > 0 { if int(params.OffsetOpt) > len(users)-1 { return []database.User{}, nil @@ -226,16 +236,6 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams users = users[:params.LimitOpt] } - if params.Status != "" { - usersFilteredByStatus := make([]database.User, 0, len(users)) - for i, user := range users { - if params.Status == string(user.Status) { - usersFilteredByStatus = append(usersFilteredByStatus, users[i]) - } - } - users = usersFilteredByStatus - } - tmp := make([]database.User, len(users)) copy(tmp, users)