feat: implement `<AddUsersMenu />` generic component by jakehwll · Pull Request #23557 · coder/coder

jakehwll · 2026-03-25T09:21:53Z

Share a reusable multi-select “Add users” popover across Organization + Group member pages, batching adds to keep the UI stable (single toast.promise, invalidate once). Also align member search to the backend q filtering and update stories/props for the new addMembers flow.

Add site/src/components/AddUsersMenu/AddUsersMenu.tsx
Batch mutations with Promise.all and single success/error toast
Invalidates only the relevant group-member query on success

Old	New

blinkagent · 2026-03-27T18:51:26Z

Heads up: this PR introduces several baseUrl-style imports (e.g. import { getErrorMessage } from "api/errors", import { users } from "api/queries/users", import type { User } from "api/typesGenerated", import { useDebouncedValue } from "hooks/debounce", import { prepareQuery } from "utils/filters", import type { ... } from "api/typesGenerated" in organizations.ts, etc.). The baseUrl resolution in tsconfig.json is deprecated and will be removed soon — these imports will stop working.

Please pull a recent version of main into your branch and make sure all new TypeScript imports use the new #/ import style (e.g. import { getErrorMessage } from "#/api/errors").

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ec30aa20d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-01T07:26:45Z

+							await addAllPromise;
+						}}
+						onSuccess={async () => {
+							// Only invalidate the group-members list we are updating.
+							await queryClient.invalidateQueries({


Refresh group member data when batched adds partially fail

The cache refresh is gated behind full Promise.all success, so if one mutateAsync fails but earlier adds succeeded, onSuccess never runs and no member-list invalidation happens. Because addMember() no longer has an onSuccess invalidation in site/src/api/queries/groups.ts, the UI can remain stale after partial success and users can immediately retry against already-added members.

Useful? React with 👍 / 👎.

A partial add is still a failure to add if only some of the data got added. NBD imo.

I think we should handle it personally, it leaves the page in a broken state that only a reload can fix which seems like a bad experience for the user.

Addressed in 7056295 — switched from Promise.all to Promise.allSettled. Now when at least one add succeeds, the member list is always invalidated/refetched. Partial failures show a descriptive toast and keep the popover open for retry.

🤖 Generated by Coder Agents

jakehwll · 2026-04-01T09:09:30Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e69ccf75d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-01T09:16:07Z

+	const useFilterResult = useFilter({
+		searchParams,
+		onSearchParamsChange: setSearchParams,
+		onUpdate: membersQuery.goToFirstPage,


Use one searchParams source for filter and pagination

OrganizationMembersPage now reads search params from two different useSearchParams() calls, then wires usePaginatedQuery to one object and useFilter to the other. useFilter.update() writes its params and then calls membersQuery.goToFirstPage, which writes again using the other snapshot; this can overwrite the just-applied filter with stale values (or drop page/filter changes). Reuse a single [searchParams, setSearchParams] pair for both hooks so URL updates are composed consistently.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-01T09:16:07Z

+							const addAllPromise = Promise.all(addPromises);
+


Invalidate members after partial add failures

The new multi-add flow uses Promise.all(...) for per-user mutations; if any request fails, await addAllPromise throws and AddUsersPopover will skip onSuccess, so the members query is never invalidated. Because some requests may already have succeeded before the rejection, the server state can change while the table stays stale, and retries can hit duplicate-member errors. Handle partial success (for example with Promise.allSettled) and always invalidate when at least one add succeeds.

Useful? React with 👍 / 👎.

this is a good catch. it also speaks to @code-asher's comment here– there's probably a better way to handle partial failures

Addressed in 7056295 — both GroupMembersPage and OrganizationMembersPage now use Promise.allSettled with proper partial failure handling.

🤖 Generated by Coder Agents

code-asher · 2026-04-07T22:57:57Z

Will go over this tomorrow!

code-asher · 2026-04-08T22:00:39Z

+							autoFocus
+						/>
+						<div
+							className="max-h-64 overflow-y-auto rounded-md border border-solid"


Does the box inside the box look kinda weird or is it just me? What if removed the inner box(es) and we just had a divider? Something like:

no, totes agree. good call pinpointing the exact line too

Suggested change

className="max-h-64 overflow-y-auto rounded-md border border-solid"

className="max-h-64 overflow-y-auto"

Addressed in 7056295 — removed rounded-md border border-solid from the inner div, keeping only max-h-64 overflow-y-auto.

🤖 Generated by Coder Agents

Applied in 7056295 — matches your suggestion exactly.

🤖 Generated by Coder Agents

code-asher · 2026-04-08T22:02:09Z

 };

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


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).

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

code-asher · 2026-04-08T22:38:10Z

+										const checkboxId = `add-member-${user.id}`;
+										return (
+											<li key={user.id} className="pl-0">
+												<label


This could just be me, sometimes I drag slightly on accident when I click and it keeps highlighting the whole page, and more strangely it inconsistently will not select the user when this happening. For whatever reason the old dialog does not do this to me.

Buut maybe this is just me clicking weirdly?? It keeps happening to me though so thought I would mention it.

2026-04-08_14-35-24.mp4

code-asher · 2026-04-08T22:47:20Z

 import { EmptyState } from "#/components/EmptyState/EmptyState";
 import { UsersFilter } from "#/components/Filter/UsersFilter";
 import { LastSeen } from "#/components/LastSeen/LastSeen";
-import { MultiMemberSelect } from "#/components/MultiUserSelect/MultiUserSelect";


I think we can delete this component (and MultiUserSelect) since it is not used anywhere else.

code-asher · 2026-04-08T22:56:43Z

-								userIds: users.map((u) => u.user_id),
+					<AddUsersPopover
+						isLoading={addMemberMutation.isPending}
+						existingUserIds={new Set(members.map((m) => m.id))}


This will not work because the members are paginated, so this only excludes the members on the current page. Exclusion would have to be implemented in the db query.

Added a comment in 7056295 acknowledging this limitation. The exclusion is best-effort since it only covers the current page. The server rejects duplicates anyway, so this serves as a UX optimization rather than a guarantee.

A proper fix would require a server-side exclusion parameter on the search query. Leaving as-is for now with the documented caveat.

🤖 Generated by Coder Agents

code-asher · 2026-04-08T23:12:54Z

 });

-const membersQuery = (data: unknown) => ({
-	key: organizationMembersKey(MockDefaultOrganization.id, {


Is this change from org members to users query correct? Group should only be querying org members I thought. For example when I check the "No Members" story, I do not see the expected error, instead it fails with 404. ("No Members" and "Filters By Members" are also broken.)

Old:

New:

code-asher · 2026-04-08T23:19:36Z

 	const { entitlements } = useDashboard();
 	const searchParamsResult = useSearchParams();
 	const showAISeatColumn = shouldShowAISeatColumn(entitlements);
+	const [searchParams, setSearchParams] = useSearchParams();


Duplicate of useSearchParams above, but we should delete the other one and use this deconstructed one!

code-asher · 2026-04-08T23:20:49Z

+						<AddUsersPopover
+							isLoading={isAddingMember}
+							onSubmit={addMembers}
+							existingUserIds={new Set(members?.map((m) => m.user_id) ?? [])}


Same problem here with members being paginated.

Same comment added in 7056295 — documented as best-effort UX optimization.

🤖 Generated by Coder Agents

code-asher · 2026-04-08T23:21:29Z

+	const addableUsersQuery = useQuery({
+		...users({
+			q: prepareQuery(debouncedSearch),
+			limit: 50,


just tagging the 50 here as well, idk if it really matters if we use 50 or 25 but probably makes sense to use the same number everywhere

code-asher · 2026-04-08T23:22:39Z

 		paginatedOrganizationMembers(organizationName, searchParamsResult[0]),
 	);

+	const useFilterResult = useFilter({


I have not looked into it yet but for some reason when I filter org members nothing changes 🤔

Oh wait might be due to the duplicate useSearchParams

- Fix duplicate useSearchParams in OrganizationMembersPage causing broken member filtering - Normalize add-user search limit from 50 to 25 to match existing components (UserAutocomplete, MultiMemberSelect) - Fix GroupPage stories to use organizationMembersKey instead of usersKey, and correct mock data shape - Remove dead MultiUserSelect component and stories (no imports)

jeremyruppel · 2026-04-09T13:25:07Z

 ) => {
-	const base = [...getRootGroupQueryKey(organization, groupName), "members"];
+	const base = groupMembersByOrganizationQueryKey(organization, groupName);
 	return req ? [...base, req] : base;


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

jeremyruppel · 2026-04-09T13:28:04Z

 };

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


could we specify the UseMutationOptions return type here?

jeremyruppel · 2026-04-09T13:30:56Z

+export type AddableUser = Pick<
+	User,
+	"id" | "username" | "name" | "email" | "avatar_url"
+>;


the only difference between this and MinimalUser is the email field. thoughts on using that instead?

jeremyruppel · 2026-04-09T13:50:35Z

+						<Button
+							type="button"
+							variant="outline"
+							onClick={() => {
+								handleOpenChange(false);
+							}}
+						>
+							Cancel
+						</Button>
+						<Button
+							type="button"
+							disabled={selectedCount === 0 || isLoading}
+							onClick={() => {
+								void handleAddUsers();
+							}}


my preference would be to remove void here but at the very least we should add it to the other onClick and be consistent

- Use Promise.allSettled instead of Promise.all for batch member adds so partial failures don't leave the page in a broken state. When at least one add succeeds, always invalidate/refetch the member list. - Remove inner border on AddUsersPopover user list (box-in-box visual) - Add comments documenting pagination limitation for existingUserIds 🤖 Generated by Coder Agents

jakehwll · 2026-04-16T12:47:15Z

Superseded by #24287 🙂

feat: implement <AddUsersMenu /> generic component

c21e000

github-actions Bot added the community Pull Requests and issues created by the community. label Mar 25, 2026

github-actions Bot assigned jakehwll Mar 25, 2026

jakehwll added 2 commits March 25, 2026 09:41

fix: remove magic invalidation

2f72731

fix: move to using /add users/ regex in tests

cca5680

matifali removed the community Pull Requests and issues created by the community. label Mar 26, 2026

jakehwll and others added 8 commits March 31, 2026 00:22

Merge branch 'main' into jakehwll/DEVEX-112-add-users-popover

48336a1

fix: resolve build conflicts

b2be80b

fix: prepend "organization" to toast

7300d26

Merge branch 'main' into jakehwll/DEVEX-112-add-users-popover

da9c3cc

Merge branch 'main' into jakehwll/DEVEX-112-add-users-popover

a8c9677

fix: resolve stories

c20dcaf

fix: remove unused import

3208234

feat: move AddUsersMenu to modules/user/ and rename

6ec30aa

jakehwll marked this pull request as ready for review April 1, 2026 07:22

jakehwll requested a review from jeremyruppel April 1, 2026 07:22

chatgpt-codex-connector Bot reviewed Apr 1, 2026

View reviewed changes

jakehwll added 3 commits April 1, 2026 08:48

fix: resolve query to consumers

8ecc4de

fix: unused keepPreviousData

8b2d5cf

fix: resolve type

0e69ccf

chatgpt-codex-connector Bot reviewed Apr 1, 2026

View reviewed changes

jakehwll requested review from code-asher and jaaydenh April 7, 2026 15:11

code-asher reviewed Apr 8, 2026

View reviewed changes

jeremyruppel reviewed Apr 9, 2026

View reviewed changes

code-asher mentioned this pull request Apr 13, 2026

chore: update design of add users dialog #24287

Merged

jakehwll closed this Apr 16, 2026

github-actions Bot locked and limited conversation to collaborators Apr 16, 2026

	className="max-h-64 overflow-y-auto rounded-md border border-solid"
	className="max-h-64 overflow-y-auto"

Conversation

jakehwll commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blinkagent Bot commented Mar 27, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakehwll commented Apr 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher commented Apr 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremyruppel Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakehwll commented Mar 25, 2026 •

edited

Loading

code-asher Apr 8, 2026 •

edited

Loading

jeremyruppel Apr 9, 2026 •

edited

Loading