From 052bd114a408bfbe3d15cab632d57beb6dde1e69 Mon Sep 17 00:00:00 2001 From: Jake Howell Date: Tue, 3 Feb 2026 00:13:41 +1100 Subject: [PATCH] fix: resolve missing users in `` (#21822) Closes #21044 This pull-request addresses an issue we were seeing where we would attempt to filter the `` by the users username or email not their username (which the rendered options would show). To highlight this I created three different users. Each with a username that did not contain their `email` or `name` and attempted to filter. Attempting to search for `John` wouldn't actually show the user as his username was `x`, and infact whereas a subset of users might be returned from the backend for having `john` in the `email` it would've been filtered by the frontend for not being in the `name` field. | Name | Username | | --- | --- | | `Jake` | `z` | | `Jeff` | `y` | | `John` | `x` | | Previously | Now | | --- | --- | | OLD_USER_COMBOBOX | NEW_USER_COMBOBOX | --- coderd/database/modelqueries.go | 1 + coderd/database/queries.sql.go | 48 ++++++---- coderd/database/queries/users.sql | 8 +- coderd/searchquery/search.go | 1 + coderd/searchquery/search_test.go | 43 +++++++++ coderd/users.go | 1 + coderd/users_test.go | 102 +++++++++++++++++++++ codersdk/users.go | 4 + site/src/pages/TasksPage/UsersCombobox.tsx | 8 +- 9 files changed, 193 insertions(+), 23 deletions(-) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 501fb1cec6..fe33341cb7 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -440,6 +440,7 @@ func (q *sqlQuerier) GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, rows, err := q.db.QueryContext(ctx, query, arg.AfterID, arg.Search, + arg.Name, pq.Array(arg.Status), pq.Array(arg.RbacRole), arg.LastSeenBefore, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 98069f42d6..a818bc2a28 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -16398,7 +16398,7 @@ WHERE ELSE true END -- Start filters - -- Filter by name, email or username + -- Filter by email or username AND CASE WHEN $2 :: text != '' THEN ( email ILIKE concat('%', $2, '%') @@ -16406,58 +16406,64 @@ WHERE ) ELSE true END + -- Filter by name (display name) + AND CASE + WHEN $3 :: text != '' THEN + name ILIKE concat('%', $3, '%') + ELSE true + 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 cardinality($3 :: user_status[]) > 0 THEN - status = ANY($3 :: user_status[]) + WHEN cardinality($4 :: user_status[]) > 0 THEN + status = ANY($4 :: user_status[]) ELSE true END -- Filter by rbac_roles AND CASE -- @rbac_role allows filtering by rbac roles. If 'member' is included, show everyone, as -- everyone is a member. - WHEN cardinality($4 :: text[]) > 0 AND 'member' != ANY($4 :: text[]) THEN - rbac_roles && $4 :: text[] + WHEN cardinality($5 :: text[]) > 0 AND 'member' != ANY($5 :: text[]) THEN + rbac_roles && $5 :: text[] ELSE true END -- Filter by last_seen AND CASE - WHEN $5 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - last_seen_at <= $5 + WHEN $6 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at <= $6 ELSE true END AND CASE - WHEN $6 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - last_seen_at >= $6 + WHEN $7 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at >= $7 ELSE true END -- Filter by created_at AND CASE - WHEN $7 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - created_at <= $7 + WHEN $8 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + created_at <= $8 ELSE true END AND CASE - WHEN $8 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - created_at >= $8 + WHEN $9 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + created_at >= $9 ELSE true END AND CASE - WHEN $9::bool THEN TRUE + WHEN $10::bool THEN TRUE ELSE is_system = false END AND CASE - WHEN $10 :: bigint != 0 THEN - github_com_user_id = $10 + WHEN $11 :: bigint != 0 THEN + github_com_user_id = $11 ELSE true END -- Filter by login_type AND CASE - WHEN cardinality($11 :: login_type[]) > 0 THEN - login_type = ANY($11 :: login_type[]) + WHEN cardinality($12 :: login_type[]) > 0 THEN + login_type = ANY($12 :: login_type[]) ELSE true END -- End of filters @@ -16466,15 +16472,16 @@ WHERE -- @authorize_filter ORDER BY -- Deterministic and consistent ordering of all users. This is to ensure consistent pagination. - LOWER(username) ASC OFFSET $12 + LOWER(username) ASC OFFSET $13 LIMIT -- A null limit means "no limit", so 0 means return all - NULLIF($13 :: int, 0) + NULLIF($14 :: int, 0) ` type GetUsersParams struct { AfterID uuid.UUID `db:"after_id" json:"after_id"` Search string `db:"search" json:"search"` + Name string `db:"name" json:"name"` Status []UserStatus `db:"status" json:"status"` RbacRole []string `db:"rbac_role" json:"rbac_role"` LastSeenBefore time.Time `db:"last_seen_before" json:"last_seen_before"` @@ -16515,6 +16522,7 @@ func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUse rows, err := q.db.QueryContext(ctx, getUsers, arg.AfterID, arg.Search, + arg.Name, pq.Array(arg.Status), pq.Array(arg.RbacRole), arg.LastSeenBefore, diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 1107eaa29a..2e8649507a 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -247,7 +247,7 @@ WHERE ELSE true END -- Start filters - -- Filter by name, email or username + -- Filter by email or username AND CASE WHEN @search :: text != '' THEN ( email ILIKE concat('%', @search, '%') @@ -255,6 +255,12 @@ WHERE ) ELSE true END + -- Filter by name (display name) + AND CASE + WHEN @name :: text != '' THEN + name ILIKE concat('%', @name, '%') + ELSE true + END -- Filter by status AND CASE -- @status needs to be a text because it can be empty, If it was diff --git a/coderd/searchquery/search.go b/coderd/searchquery/search.go index d378fb7b7d..f42d7cfd84 100644 --- a/coderd/searchquery/search.go +++ b/coderd/searchquery/search.go @@ -156,6 +156,7 @@ func Users(query string) (database.GetUsersParams, []codersdk.ValidationError) { parser := httpapi.NewQueryParamParser() filter := database.GetUsersParams{ Search: parser.String(values, "", "search"), + Name: parser.String(values, "", "name"), Status: httpapi.ParseCustomList(parser, values, []database.UserStatus{}, "status", httpapi.ParseEnum[database.UserStatus]), RbacRole: parser.Strings(values, []string{}, "role"), LastSeenAfter: parser.Time3339Nano(values, time.Time{}, "last_seen_after"), diff --git a/coderd/searchquery/search_test.go b/coderd/searchquery/search_test.go index 44ae9d1021..8dba7ce593 100644 --- a/coderd/searchquery/search_test.go +++ b/coderd/searchquery/search_test.go @@ -754,6 +754,49 @@ func TestSearchUsers(t *testing.T) { }, }, + // Name filter tests + { + Name: "NameFilter", + Query: "name:John", + Expected: database.GetUsersParams{ + Name: "john", + Status: []database.UserStatus{}, + RbacRole: []string{}, + LoginType: []database.LoginType{}, + }, + }, + { + Name: "NameFilterQuoted", + Query: `name:"John Doe"`, + Expected: database.GetUsersParams{ + Name: "john doe", + Status: []database.UserStatus{}, + RbacRole: []string{}, + LoginType: []database.LoginType{}, + }, + }, + { + Name: "NameFilterWithSearch", + Query: "name:John search:johnd", + Expected: database.GetUsersParams{ + Search: "johnd", + Name: "john", + Status: []database.UserStatus{}, + RbacRole: []string{}, + LoginType: []database.LoginType{}, + }, + }, + { + Name: "NameFilterWithOtherParams", + Query: "name:John status:active role:owner", + Expected: database.GetUsersParams{ + Name: "john", + Status: []database.UserStatus{database.UserStatusActive}, + RbacRole: []string{codersdk.RoleOwner}, + LoginType: []database.LoginType{}, + }, + }, + // Failures { Name: "ExtraColon", diff --git a/coderd/users.go b/coderd/users.go index 0758642429..8de2697e56 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -297,6 +297,7 @@ func (api *API) GetUsers(rw http.ResponseWriter, r *http.Request) ([]database.Us userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{ AfterID: paginationParams.AfterID, Search: params.Search, + Name: params.Name, Status: params.Status, RbacRole: params.RbacRole, LastSeenBefore: params.LastSeenBefore, diff --git a/coderd/users_test.go b/coderd/users_test.go index dd4cb9d8ad..b00aead7d6 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -2110,6 +2110,108 @@ func TestGetUsers(t *testing.T) { require.Equal(t, res.Users[0].Username, "bob") require.Equal(t, res.Users[0].LoginType, codersdk.LoginTypeOIDC) }) + + t.Run("NameFilter", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + first := coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + // Create users with different display names + _, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{ + Email: "alice@email.com", + Username: "alice", + Name: "Alice Smith", + OrganizationIDs: []uuid.UUID{first.OrganizationID}, + UserLoginType: codersdk.LoginTypeNone, + }) + require.NoError(t, err) + + _, err = client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{ + Email: "bob@email.com", + Username: "bob", + Name: "Bob Johnson", + OrganizationIDs: []uuid.UUID{first.OrganizationID}, + UserLoginType: codersdk.LoginTypeNone, + }) + require.NoError(t, err) + + _, err = client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{ + Email: "charlie@email.com", + Username: "charlie", + Name: "Charlie Smith", + OrganizationIDs: []uuid.UUID{first.OrganizationID}, + UserLoginType: codersdk.LoginTypeNone, + }) + require.NoError(t, err) + + // Filter by name "Smith" should return Alice and Charlie + res, err := client.Users(ctx, codersdk.UsersRequest{ + Name: "Smith", + }) + require.NoError(t, err) + require.Len(t, res.Users, 2) + usernames := []string{res.Users[0].Username, res.Users[1].Username} + require.ElementsMatch(t, []string{"alice", "charlie"}, usernames) + + // Filter by name "Alice" should return only Alice + res, err = client.Users(ctx, codersdk.UsersRequest{ + Name: "Alice", + }) + require.NoError(t, err) + require.Len(t, res.Users, 1) + require.Equal(t, "alice", res.Users[0].Username) + + // Filter by name "Johnson" should return only Bob + res, err = client.Users(ctx, codersdk.UsersRequest{ + Name: "Johnson", + }) + require.NoError(t, err) + require.Len(t, res.Users, 1) + require.Equal(t, "bob", res.Users[0].Username) + + // Filter by name that doesn't exist should return no users + res, err = client.Users(ctx, codersdk.UsersRequest{ + Name: "Nonexistent", + }) + require.NoError(t, err) + require.Len(t, res.Users, 0) + }) + + t.Run("NameFilterWithSearchFilter", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + first := coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + // Create users with different display names and usernames + _, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{ + Email: "alice@email.com", + Username: "alice", + Name: "Alice Developer", + OrganizationIDs: []uuid.UUID{first.OrganizationID}, + UserLoginType: codersdk.LoginTypeNone, + }) + require.NoError(t, err) + + _, err = client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{ + Email: "bob@email.com", + Username: "bobdev", + Name: "Bob Developer", + OrganizationIDs: []uuid.UUID{first.OrganizationID}, + UserLoginType: codersdk.LoginTypeNone, + }) + require.NoError(t, err) + + // Filter by name "Developer" and search "alice" should return only Alice + // because name matches both but search matches only alice's username + res, err := client.Users(ctx, codersdk.UsersRequest{ + SearchQuery: "name:Developer search:alice", + }) + require.NoError(t, err) + require.Len(t, res.Users, 1) + require.Equal(t, "alice", res.Users[0].Username) + }) } func TestGetUsersPagination(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index 1bf09370d9..1014625314 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -26,6 +26,7 @@ const ( type UsersRequest struct { Search string `json:"search,omitempty" typescript:"-"` + Name string `json:"name,omitempty" typescript:"-"` // Filter users by status. Status UserStatus `json:"status,omitempty" typescript:"-"` // Filter users that have the given role. @@ -847,6 +848,9 @@ func (c *Client) Users(ctx context.Context, req UsersRequest) (GetUsersResponse, if req.Search != "" { params = append(params, req.Search) } + if req.Name != "" { + params = append(params, "name:"+req.Name) + } if req.Status != "" { params = append(params, "status:"+string(req.Status)) } diff --git a/site/src/pages/TasksPage/UsersCombobox.tsx b/site/src/pages/TasksPage/UsersCombobox.tsx index e3e443754a..9ed0a3bd42 100644 --- a/site/src/pages/TasksPage/UsersCombobox.tsx +++ b/site/src/pages/TasksPage/UsersCombobox.tsx @@ -46,7 +46,7 @@ export const UsersCombobox: FC = ({ const debouncedSearch = useDebouncedValue(search, 250); const { user } = useAuthenticated(); const { data: options } = useQuery({ - ...users({ q: debouncedSearch }), + ...users({ q: debouncedSearch ? `name:"${debouncedSearch}"` : "" }), select: (res) => mapUsersToOptions(res.users, user, value), placeholderData: keepPreviousData, }); @@ -75,7 +75,11 @@ export const UsersCombobox: FC = ({ - + {/* + * `shouldFilter` is false because we don't want to filter on the `value` + * because we're using the `name` field to filter on the backend. + */} +