Skip to content

feat: implement <AddUsersMenu /> generic component #23557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jakehwll wants to merge 16 commits into from
Closed

feat: implement <AddUsersMenu /> generic component #23557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c21e000
feat: implement `<AddUsersMenu />` generic component
jakehwll Mar 25, 2026
2f72731
fix: remove magic invalidation
jakehwll Mar 25, 2026
cca5680
fix: move to using `/add users/` regex in tests
jakehwll Mar 25, 2026
48336a1
Merge branch 'main' into jakehwll/DEVEX-112-add-users-popover
jakehwll Mar 30, 2026
b2be80b
fix: resolve build conflicts
jakehwll Mar 30, 2026
7300d26
fix: prepend "organization" to toast
jakehwll Mar 30, 2026
da9c3cc
Merge branch 'main' into jakehwll/DEVEX-112-add-users-popover
jakehwll Mar 31, 2026
a8c9677
Merge branch 'main' into jakehwll/DEVEX-112-add-users-popover
jakehwll Apr 1, 2026
c20dcaf
fix: resolve stories
jakehwll Apr 1, 2026
3208234
fix: remove unused import
jakehwll Apr 1, 2026
6ec30aa
feat: move `AddUsersMenu` to `modules/user/` and rename
jakehwll Apr 1, 2026
8ecc4de
fix: resolve query to consumers
jakehwll Apr 1, 2026
8b2d5cf
fix: unused `keepPreviousData`
jakehwll Apr 1, 2026
0e69ccf
fix: resolve type
jakehwll Apr 1, 2026
aa58a01
fix(site): address review feedback for AddUsersPopover PR
jakehwll Apr 9, 2026
7056295
fix(site): handle partial failures and address remaining review feedback
jakehwll Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions site/src/api/queries/groups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,17 @@ export const group = (
};
};

export const groupMembersByOrganizationQueryKey = (
organization: string,
groupName: string,
) => ["organization", organization, "group", groupName, "members"];

export const getGroupMembersQueryKey = (
organization: string,
groupName: string,
req?: UsersRequest,
) => {
const base = [...getRootGroupQueryKey(organization, groupName), "members"];
const base = groupMembersByOrganizationQueryKey(organization, groupName);
return req ? [...base, req] : base;

@jeremyruppel jeremyruppel Apr 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ok that there's a query key collision here?

};

Expand Down Expand Up @@ -200,17 +205,10 @@ export const deleteGroup = (queryClient: QueryClient, organization: string) => {
};
};

export const addMembers = (queryClient: QueryClient, organization: string) => {
export const addMember = () => {

@code-asher code-asher Apr 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the switch from the bulk add endpoint to the single add endpoint?

idk if this is related, but when I try to add two members, one gets added and the other gets an error:

execute transaction: get group by ID: fetch object: sql: no rows in result set

But sometimes it does work. 🤔 Organization members always seems to work, I only get this on the group for some reason.

Maybe a backend race with trying to add individual members in parallel?

idk if it would be worth trying to fix that though when we could just add them all at once.

Another tricky thing about the single add endpoint is that if some succeed and some do not, we should probably update the list with the ones that did succeed. Otherwise it looks like nothing got added, and also if they try again it will error saying some have already been added. At that point I think the only way the user can fix the page is by reloading it.

Also does each user add query invalidate the group/org? When adding two members to an org for example I see three parallel requests for the members list which seems unfortunate. Ideally we could just do one update after all members were added (or were failed to be added). But maybe it is easier to use the bulk endpoint (idk if org members has one though).

@jakehwll jakehwll Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The race condition with parallel single-add mutations is now mitigated by switching to Promise.allSettled (7056295) — partial successes are reflected in the UI, and the member list always refreshes when at least one add succeeds.

Switching back to the bulk endpoint would avoid the parallel mutation issue entirely — leaving that decision to @jakehwll.

🤖 Generated by Coder Agents

@jeremyruppel jeremyruppel Apr 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we specify the UseMutationOptions return type here?

return {
mutationFn: ({
groupId,
userIds,
}: {
groupId: string;
userIds: string[];
}) => API.addMembers(groupId, userIds),
onSuccess: async (updatedGroup: Group) =>
invalidateGroup(queryClient, organization, updatedGroup.name),
mutationFn: ({ groupId, userId }: { groupId: string; userId: string }) =>
API.addMember(groupId, userId),
};
};

Expand Down
8 changes: 3 additions & 5 deletions site/src/api/queries/organizations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {
CreateOrganizationRequest,
GroupSyncSettings,
Organization,
PaginatedMembersRequest,
PaginatedMembersResponse,
RoleSyncSettings,
UpdateOrganizationRequest,
Expand All @@ -27,6 +26,7 @@ import {
type WorkspacePermissions,
workspacePermissionChecks,
} from "#/modules/permissions/workspaces";
import { prepareQuery } from "#/utils/filters";
import { meKey } from "./users";
import { cachedQuery } from "./util";

Expand Down Expand Up @@ -95,16 +95,14 @@ export const organizationMembers = (id: string, req: UsersRequest) => {
export const paginatedOrganizationMembers = (
id: string,
searchParams: URLSearchParams,
): UsePaginatedQueryOptions<
PaginatedMembersResponse,
PaginatedMembersRequest
> => {
): UsePaginatedQueryOptions<PaginatedMembersResponse, UsersRequest> => {
return {
searchParams,
queryPayload: ({ limit, offset }) => {
return {
limit: limit,
offset: offset,
q: prepareQuery(searchParams.get("filter") ?? ""),
};
},
queryKey: ({ payload }) => organizationMembersKey(id, payload),
Expand Down
32 changes: 0 additions & 32 deletions site/src/components/MultiUserSelect/MultiMemberSelect.stories.tsx
View file
Open in desktop

This file was deleted.

129 changes: 0 additions & 129 deletions site/src/components/MultiUserSelect/MultiUserSelect.stories.tsx
View file
Open in desktop

This file was deleted.

Loading
Loading