🐛 fix: Group Scope Picker and Name Resolution Past 200 Groups - #115
🐛 fix: Group Scope Picker and Name Resolution Past 200 Groups#115dustinhealy wants to merge 7 commits into
Conversation
The scope picker's create view fetched a single list capped at the backend's 200 per request maximum and filtered it client side via cmdk, so in deployments with more groups (issue #96 reports 1500+ from Entra SSO sync) most groups could never be found or selected. The groups section now searches server side with a debounced query and offset pagination through a new useGroupSearch hook built on the existing getGroupsFn search/limit/offset support, mirroring the GroupsTab pattern. getAvailableScopesFn built its group name map from that same first page of 200 groups, so existing group scopes outside the window displayed their raw Mongo ObjectId instead of the group name. It now resolves names for exactly the group principalIds that have config overrides via GET /api/admin/groups/:id, fetched in small parallel batches, and falls back to the principalId when an individual group cannot be fetched.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95aa1ac545
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95aa1ac545
ℹ️ 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".
The create view filtered each backend page against existing configurations after pagination, while totalPages still described every group. A page whose groups were all configured rendered the global "all groups already have configurations" empty state even when later pages held eligible groups, and searches could report no matches for the same reason. Configured groups are now rendered as disabled entries with an "Already configured" badge instead of being filtered out, so the page contents always match the server totals and the empty states only describe truly empty result sets. Covered by ScopeSelector.test.tsx, which renders a fully configured first page and asserts disabled entries plus a reachable second page; both tests fail against the previous filtering. Also moves the GroupSearch hook contract from useGroupSearch.ts into src/types/hooks.ts per the repository convention that locally defined interfaces live in src/types and are referenced through the @/types namespace import.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 80c996a. Configure here.
The back control in the scope create view only flipped showCreate, so useGroupSearch kept its page and filter and reopening the view rendered the previous search and page. The dialog-level resetState cleared the input through the debounced onSearchChange, so the debounced query value and page reset lagged the emptied input by the debounce delay and a reopen inside that window could still query with the stale search. useDebouncedFilter gains a synchronous reset that cancels any pending commit and restores both values at once, useGroupSearch exposes it (clearing the page in the same call, mirrored in the GroupSearch contract in src/types/hooks.ts), and ScopeSelector routes both the back control and resetState through a shared closeCreate that resets the search state without debounce lag. The back control also gets an accessible name. Regression tests: reopening create after a search starts from page 1 unfiltered without issuing a stale search request (ScopeSelector.test.tsx), and reset clears search and page synchronously with no request for a pending debounced value (useGroupSearch.test.tsx).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afdb4de2ee
ℹ️ 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".
…Input With keepPreviousData, rows from the previous page or search stayed clickable while a new group page was in flight, so a click during a slow fetch could create a configuration for a group outside the currently requested results. Group rows are now disabled while groupSearch.isFetching, alongside the existing creating and already-configured states, and the dimmed wrapper also blocks pointer events. Covered by a gated-fetch test that holds the list request open and asserts stale rows are disabled until it resolves. The 200 character search clamp previously applied only inside getGroupsFn, so the visible input and the react-query key kept the full text while the request sent a truncated one, letting the displayed query diverge from the queried one and letting distinct long searches with a shared 200 character prefix duplicate equivalent requests under different cache keys. useGroupSearch now clamps in onSearchChange so value, debounced value, query key, and request all agree, with the server-fn clamp kept as a backstop; MAX_SEARCH_LENGTH is exported from the groups module as the single source of the limit. The truncation test now asserts the input value is clamped as well.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43a79ef643
ℹ️ 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".
Typing a new search updates the visible query immediately while the request waits out the 300ms debounce, so isFetching stayed false and rows from the previous query remained clickable against a search they no longer match. The hook now exposes isSearchPending (raw versus debounced mismatch) and the create view gates rows on it alongside isFetching.
|
@codex review |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af1dc214e1
ℹ️ 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".
… GroupsTab Search While a debounced search was fetching, keepPreviousData kept the old multi-page total visible and the paginator stayed clickable even though the rows were disabled, so clicking an old later page started the searched query at that offset and a single-page result stranded the view on an empty out-of-range page with the paginator gone. The create view now dims the paginator and ignores page changes while a fetch or debounce is pending, and useGroupSearch additionally clamps the page back to the last valid page whenever a settled total drops below it, so no code path can leave the view stranded. useGroupSearch also discarded query errors, rendering a failed groups request as an empty list. The hook now exposes isError and refetch through the GroupSearch contract and the create view renders the existing com_error_load_groups message with a retry button in place of the list, hiding the paginator while errored. The Groups management tab calls groupsQueryOptions directly and never goes through useGroupSearch, so its controlled input and query key kept a search beyond 200 characters while the server fn truncated it in transport. Its search handler now clamps with the shared MAX_SEARCH_LENGTH before state and debounce, keeping displayed and queried text identical, with the server-fn clamp still in place as a backstop. Regression coverage: pagination clicks are ignored mid-search and the view settles on the searched first page, an out-of-range page clamps back to the last valid page, a failed fetch renders the retryable error state and retry recovers, and the GroupsTab input value matches the queried search at 200 characters. All four fail before this change.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Deployments with more groups than the backend's 200-per-request cap (e.g. 1500+ Entra/AD groups synced via SSO) could not manage group-based configuration: the scope picker only ever loaded the first 200 groups, and existing group scopes outside that window displayed raw Mongo ObjectIds instead of group names.
The picker's create view now searches groups server side through a new
useGroupSearchhook (debounced search plus offset pagination built on the existinggetGroupsFnsearch/limit/offset support, mirroring theGroupsTabpattern), so every group is reachable by search or paging regardless of total count. Search strings are clamped to the backend's 200-character limit, which otherwise rejects the request with a 400. Groups that already have a configuration are no longer filtered out after pagination; they render as disabled entries with an "Already configured" badge, so page contents always match the server-derived page count and eligible groups on later pages stay reachable.getAvailableScopesFnno longer derives its name map from the first page of the groups list. It resolves names for exactly the group principalIds that have config overrides viaGET /api/admin/groups/:id, deduplicated and fetched in small parallel batches, falling back to the principalId for groups that cannot be fetched (e.g. deleted groups), so existing scopes always show their real names.Fixes #96
Change Type
Testing
New unit tests:
scopes.names.test.tsreproduces the ObjectId fallback against a simulated 1500-group backend (fails on main, passes here),useGroupSearch.test.tsxexercises the debounced search, page-offset, search-clamping, and enabled-gating behavior with only the HTTP layer mocked, andScopeSelector.test.tsxcovers the truthful-pagination regression (a fully configured first page renders disabled entries with working pagination to an eligible page two, failing against the previous filtering). Local gates: fullvitestsuite passes (812 tests),eslint --max-warnings 0clean,tsc --noEmitclean.Checklist