fix: rework user group management with an accessible selector

[igboyes]

Summary

• Replace the toggle-checkbox group list and the separate primary-group dropdown with one accessible control: a combobox that adds groups to the membership list, and a radio group where each member row selects the primary group and offers a button to revoke membership. Fixes VIR-2539 (conflicting option/checkbox roles, double tab stops, dead decorative checkbox) and scales better as group counts grow.
• Distinguish the previously-identical empty states: show "This user is a member of every group" when there is nothing left to add, and a "No groups have been created yet" message linking to group management when the instance has no groups.
• Generalize the orphaned base ComboBox with a placeholder prop and match its height to the standard Input; remove UserGroup and PrimaryGroup.

[linear-code]

VIR-2539

[sourcery-ai]

Hey - I've found 3 issues, and left some high level feedback:

• Consider memoizing memberIds and availableGroups (e.g. with useMemo) so they aren’t recomputed and reallocated on every render, especially since this component is intended to scale to large group counts.
• In the useListGroups error case you currently render <QueryError /> only from renderAdd, while renderMembership still runs; you may want to short-circuit the whole component on a fatal groups error to avoid partially-rendered or confusing UI states.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider memoizing `memberIds` and `availableGroups` (e.g. with `useMemo`) so they aren’t recomputed and reallocated on every render, especially since this component is intended to scale to large group counts.
- In the `useListGroups` error case you currently render `<QueryError />` only from `renderAdd`, while `renderMembership` still runs; you may want to short-circuit the whole component on a fatal groups error to avoid partially-rendered or confusing UI states.

## Individual Comments

### Comment 1
<location path="apps/web/src/users/components/UserGroups.tsx" line_range="184-185" />
<code_context>
+	}
+
+	return (
+		<div className="mb-4">
+			<InputLabel>Groups</InputLabel>
+			{renderAdd()}
+			{renderMembership()}
</code_context>
<issue_to_address>
**issue (bug_risk):** The label is no longer programmatically associated with the interactive controls, which is an accessibility regression.

Previously this used `useId` and `aria-labelledby` on `BoxGroup` to tie the label to the listbox. Now `InputLabel` has no `id`/`htmlFor`, and neither the combobox nor radio group reference it. Please either restore an `id` on `InputLabel` and point the combo/radio group to it via `aria-labelledby`, or wire `InputLabel`’s `htmlFor` to the appropriate control.
</issue_to_address>

### Comment 2
<location path="apps/web/src/users/components/__tests__/UserGroups.test.tsx" line_range="46-23" />
<code_context>
+		).not.toBeChecked();
+	});
+
+	it("shows an empty message when the user has no groups", async () => {
+		mockApiListGroups(allGroups);
+
+		renderWithProviders(
+			<UserGroups userId={userId} memberGroups={[]} primaryGroup={null} />,
+		);
+
+		expect(
+			await screen.findByText("This user is not a member of any groups."),
+		).toBeInTheDocument();
+	});
+
+	it("hides the combobox when the user is in every group", async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for the case where a user has groups but no primary group.

The remaining branch is when `memberGroups` is non-empty but `primaryGroup` is `null`, where the "No primary group" radio should be selected by default. Please add a focused test for this combination to cover that default-selection behavior.
</issue_to_address>

### Comment 3
<location path="apps/web/src/users/components/__tests__/UserGroups.test.tsx" line_range="121-23" />
<code_context>
+		);
+	});
+
+	it("sets the primary group when a radio is selected", async () => {
+		mockApiListGroups(allGroups);
+		mockApiEditUser(userId, 200, {});
+
+		renderWithProviders(
+			<UserGroups
+				userId={userId}
+				memberGroups={[member, other]}
+				primaryGroup={member}
+			/>,
+		);
+
+		await userEvent.click(await screen.findByRole("radio", { name: "bar" }));
+
+		await waitFor(() =>
+			expect(userServerFnMocks.updateUser).toHaveBeenCalledWith({
+				data: { userId, primary_group: 2 },
+			}),
+		);
+	});
+
+	it("removes a group and clears the primary when the primary is removed", async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Also test selecting the "No primary group" radio to clear `primary_group`.

To fully cover the new behavior, please add a test that clicks the "No primary group" radio and asserts `updateUser` is called with `primary_group: null`, so the transition from having a primary group to none is also verified.

Suggested implementation:

```typescript
	it("sets the primary group when a radio is selected", async () => {
		mockApiListGroups(allGroups);
		mockApiEditUser(userId, 200, {});

		renderWithProviders(
			<UserGroups
				userId={userId}
				memberGroups={[member, other]}
				primaryGroup={member}
			/>,
		);

		await userEvent.click(await screen.findByRole("radio", { name: "bar" }));

		await waitFor(() =>
			expect(userServerFnMocks.updateUser).toHaveBeenCalledWith({
				data: { userId, primary_group: 2 },
			}),
		);
	});

	it("clears the primary group when selecting the 'No primary group' option", async () => {
		mockApiListGroups(allGroups);
		mockApiEditUser(userId, 200, {});

		renderWithProviders(
			<UserGroups
				userId={userId}
				memberGroups={[member, other]}
				primaryGroup={member}
			/>,
		);

		await userEvent.click(
			await screen.findByRole("radio", {
				name: /no primary group/i,
			}),
		);

		await waitFor(() =>
			expect(userServerFnMocks.updateUser).toHaveBeenCalledWith({
				data: { userId, primary_group: null },
			}),
		);
	});

	it("removes a group and clears the primary when the primary is removed", async () => {

```

This assumes the "No primary group" radio is rendered with an accessible name matching `/no primary group/i`. If the actual label differs, adjust the `name` matcher in `findByRole` accordingly to match the existing UI text.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[igboyes]

Re the two review-body suggestions — both intentionally skipped:

• Memoize memberIds/availableGroups: this app runs the React Compiler, which auto-memoizes; a manual useMemo is redundant. Group counts are also small (~30).
• Short-circuit the whole component on a groups fetch error: intentional. The membership list is primary data from the user record (props); the groups list is secondary and only feeds the add combobox. Scoping QueryError to the add slot keeps the existing memberships viewable/removable on a failed groups fetch, per the repo's two-tier error policy.