mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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" /> |
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user