Skip to content

Commit 052bd11

Browse files
authored
fix: resolve missing users in <UserCombobox /> (#21822)
Closes #21044 This pull-request addresses an issue we were seeing where we would attempt to filter the `<UserCombobox />` 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 | | --- | --- | | <img width="560" height="547" alt="OLD_USER_COMBOBOX" src="https://github.com/user-attachments/assets/a0567264-0034-42ac-aba0-95b05c4f92dd" /> | <img width="580" height="548" alt="NEW_USER_COMBOBOX" src="https://github.com/user-attachments/assets/1aa0c942-d340-4b1c-8dde-b97879525bfb" /> |
1 parent 3e369c0 commit 052bd11

9 files changed

Lines changed: 193 additions & 23 deletions

File tree

‎coderd/database/modelqueries.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ func (q *sqlQuerier) GetAuthorizedUsers(ctx context.Context, arg GetUsersParams,
440440
rows, err := q.db.QueryContext(ctx, query,
441441
arg.AfterID,
442442
arg.Search,
443+
arg.Name,
443444
pq.Array(arg.Status),
444445
pq.Array(arg.RbacRole),
445446
arg.LastSeenBefore,

‎coderd/database/queries.sql.go‎

Lines changed: 28 additions & 20 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: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,14 +247,20 @@ WHERE
247247
ELSE true
248248
END
249249
-- Start filters
250-
-- Filter by name, email or username
250+
-- Filter by email or username
251251
AND CASE
252252
WHEN @search :: text != '' THEN (
253253
email ILIKE concat('%', @search, '%')
254254
OR username ILIKE concat('%', @search, '%')
255255
)
256256
ELSE true
257257
END
258+
-- Filter by name (display name)
259+
AND CASE
260+
WHEN @name :: text != '' THEN
261+
name ILIKE concat('%', @name, '%')
262+
ELSE true
263+
END
258264
-- Filter by status
259265
AND CASE
260266
-- @status needs to be a text because it can be empty, If it was

‎coderd/searchquery/search.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ func Users(query string) (database.GetUsersParams, []codersdk.ValidationError) {
156156
parser := httpapi.NewQueryParamParser()
157157
filter := database.GetUsersParams{
158158
Search: parser.String(values, "", "search"),
159+
Name: parser.String(values, "", "name"),
159160
Status: httpapi.ParseCustomList(parser, values, []database.UserStatus{}, "status", httpapi.ParseEnum[database.UserStatus]),
160161
RbacRole: parser.Strings(values, []string{}, "role"),
161162
LastSeenAfter: parser.Time3339Nano(values, time.Time{}, "last_seen_after"),

‎coderd/searchquery/search_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,49 @@ func TestSearchUsers(t *testing.T) {
754754
},
755755
},
756756

757+
// Name filter tests
758+
{
759+
Name: "NameFilter",
760+
Query: "name:John",
761+
Expected: database.GetUsersParams{
762+
Name: "john",
763+
Status: []database.UserStatus{},
764+
RbacRole: []string{},
765+
LoginType: []database.LoginType{},
766+
},
767+
},
768+
{
769+
Name: "NameFilterQuoted",
770+
Query: `name:"John Doe"`,
771+
Expected: database.GetUsersParams{
772+
Name: "john doe",
773+
Status: []database.UserStatus{},
774+
RbacRole: []string{},
775+
LoginType: []database.LoginType{},
776+
},
777+
},
778+
{
779+
Name: "NameFilterWithSearch",
780+
Query: "name:John search:johnd",
781+
Expected: database.GetUsersParams{
782+
Search: "johnd",
783+
Name: "john",
784+
Status: []database.UserStatus{},
785+
RbacRole: []string{},
786+
LoginType: []database.LoginType{},
787+
},
788+
},
789+
{
790+
Name: "NameFilterWithOtherParams",
791+
Query: "name:John status:active role:owner",
792+
Expected: database.GetUsersParams{
793+
Name: "john",
794+
Status: []database.UserStatus{database.UserStatusActive},
795+
RbacRole: []string{codersdk.RoleOwner},
796+
LoginType: []database.LoginType{},
797+
},
798+
},
799+
757800
// Failures
758801
{
759802
Name: "ExtraColon",

‎coderd/users.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ func (api *API) GetUsers(rw http.ResponseWriter, r *http.Request) ([]database.Us
297297
userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{
298298
AfterID: paginationParams.AfterID,
299299
Search: params.Search,
300+
Name: params.Name,
300301
Status: params.Status,
301302
RbacRole: params.RbacRole,
302303
LastSeenBefore: params.LastSeenBefore,

‎coderd/users_test.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,6 +2110,108 @@ func TestGetUsers(t *testing.T) {
21102110
require.Equal(t, res.Users[0].Username, "bob")
21112111
require.Equal(t, res.Users[0].LoginType, codersdk.LoginTypeOIDC)
21122112
})
2113+
2114+
t.Run("NameFilter", func(t *testing.T) {
2115+
t.Parallel()
2116+
client := coderdtest.New(t, nil)
2117+
first := coderdtest.CreateFirstUser(t, client)
2118+
ctx := testutil.Context(t, testutil.WaitLong)
2119+
2120+
// Create users with different display names
2121+
_, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
2122+
2123+
Username: "alice",
2124+
Name: "Alice Smith",
2125+
OrganizationIDs: []uuid.UUID{first.OrganizationID},
2126+
UserLoginType: codersdk.LoginTypeNone,
2127+
})
2128+
require.NoError(t, err)
2129+
2130+
_, err = client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
2131+
2132+
Username: "bob",
2133+
Name: "Bob Johnson",
2134+
OrganizationIDs: []uuid.UUID{first.OrganizationID},
2135+
UserLoginType: codersdk.LoginTypeNone,
2136+
})
2137+
require.NoError(t, err)
2138+
2139+
_, err = client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
2140+
2141+
Username: "charlie",
2142+
Name: "Charlie Smith",
2143+
OrganizationIDs: []uuid.UUID{first.OrganizationID},
2144+
UserLoginType: codersdk.LoginTypeNone,
2145+
})
2146+
require.NoError(t, err)
2147+
2148+
// Filter by name "Smith" should return Alice and Charlie
2149+
res, err := client.Users(ctx, codersdk.UsersRequest{
2150+
Name: "Smith",
2151+
})
2152+
require.NoError(t, err)
2153+
require.Len(t, res.Users, 2)
2154+
usernames := []string{res.Users[0].Username, res.Users[1].Username}
2155+
require.ElementsMatch(t, []string{"alice", "charlie"}, usernames)
2156+
2157+
// Filter by name "Alice" should return only Alice
2158+
res, err = client.Users(ctx, codersdk.UsersRequest{
2159+
Name: "Alice",
2160+
})
2161+
require.NoError(t, err)
2162+
require.Len(t, res.Users, 1)
2163+
require.Equal(t, "alice", res.Users[0].Username)
2164+
2165+
// Filter by name "Johnson" should return only Bob
2166+
res, err = client.Users(ctx, codersdk.UsersRequest{
2167+
Name: "Johnson",
2168+
})
2169+
require.NoError(t, err)
2170+
require.Len(t, res.Users, 1)
2171+
require.Equal(t, "bob", res.Users[0].Username)
2172+
2173+
// Filter by name that doesn't exist should return no users
2174+
res, err = client.Users(ctx, codersdk.UsersRequest{
2175+
Name: "Nonexistent",
2176+
})
2177+
require.NoError(t, err)
2178+
require.Len(t, res.Users, 0)
2179+
})
2180+
2181+
t.Run("NameFilterWithSearchFilter", func(t *testing.T) {
2182+
t.Parallel()
2183+
client := coderdtest.New(t, nil)
2184+
first := coderdtest.CreateFirstUser(t, client)
2185+
ctx := testutil.Context(t, testutil.WaitLong)
2186+
2187+
// Create users with different display names and usernames
2188+
_, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
2189+
2190+
Username: "alice",
2191+
Name: "Alice Developer",
2192+
OrganizationIDs: []uuid.UUID{first.OrganizationID},
2193+
UserLoginType: codersdk.LoginTypeNone,
2194+
})
2195+
require.NoError(t, err)
2196+
2197+
_, err = client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{
2198+
2199+
Username: "bobdev",
2200+
Name: "Bob Developer",
2201+
OrganizationIDs: []uuid.UUID{first.OrganizationID},
2202+
UserLoginType: codersdk.LoginTypeNone,
2203+
})
2204+
require.NoError(t, err)
2205+
2206+
// Filter by name "Developer" and search "alice" should return only Alice
2207+
// because name matches both but search matches only alice's username
2208+
res, err := client.Users(ctx, codersdk.UsersRequest{
2209+
SearchQuery: "name:Developer search:alice",
2210+
})
2211+
require.NoError(t, err)
2212+
require.Len(t, res.Users, 1)
2213+
require.Equal(t, "alice", res.Users[0].Username)
2214+
})
21132215
}
21142216

21152217
func TestGetUsersPagination(t *testing.T) {

‎codersdk/users.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const (
2626

2727
type UsersRequest struct {
2828
Search string `json:"search,omitempty" typescript:"-"`
29+
Name string `json:"name,omitempty" typescript:"-"`
2930
// Filter users by status.
3031
Status UserStatus `json:"status,omitempty" typescript:"-"`
3132
// Filter users that have the given role.
@@ -847,6 +848,9 @@ func (c *Client) Users(ctx context.Context, req UsersRequest) (GetUsersResponse,
847848
if req.Search != "" {
848849
params = append(params, req.Search)
849850
}
851+
if req.Name != "" {
852+
params = append(params, "name:"+req.Name)
853+
}
850854
if req.Status != "" {
851855
params = append(params, "status:"+string(req.Status))
852856
}

‎site/src/pages/TasksPage/UsersCombobox.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const UsersCombobox: FC<UsersComboboxProps> = ({
4646
const debouncedSearch = useDebouncedValue(search, 250);
4747
const { user } = useAuthenticated();
4848
const { data: options } = useQuery({
49-
...users({ q: debouncedSearch }),
49+
...users({ q: debouncedSearch ? `name:"${debouncedSearch}"` : "" }),
5050
select: (res) => mapUsersToOptions(res.users, user, value),
5151
placeholderData: keepPreviousData,
5252
});
@@ -75,7 +75,11 @@ export const UsersCombobox: FC<UsersComboboxProps> = ({
7575
</Button>
7676
</PopoverTrigger>
7777
<PopoverContent className="w-[280px] p-0">
78-
<Command>
78+
{/*
79+
* `shouldFilter` is false because we don't want to filter on the `value`
80+
* because we're using the `name` field to filter on the backend.
81+
*/}
82+
<Command shouldFilter={false}>
7983
<CommandInput
8084
placeholder="Search user..."
8185
value={search}

0 commit comments

Comments
 (0)