From ef0a4bb479a085c4accfa402463733c563c2aaca Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Sun, 9 Aug 2026 17:28:54 -0700 Subject: [PATCH 1/7] =?UTF-8?q?=F0=9F=90=9B=20fix:=20Group=20Scope=20Picke?= =?UTF-8?q?r=20and=20Name=20Resolution=20Past=20200=20Groups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../configuration/ScopeSelector.tsx | 120 ++++++++----- src/hooks/index.ts | 1 + src/hooks/useGroupSearch.test.tsx | 131 ++++++++++++++ src/hooks/useGroupSearch.ts | 45 +++++ src/locales/en/translation.json | 1 + src/server/groups.ts | 27 +++ src/server/scopes.names.test.ts | 168 ++++++++++++++++++ src/server/scopes.ts | 29 ++- 8 files changed, 463 insertions(+), 59 deletions(-) create mode 100644 src/hooks/useGroupSearch.test.tsx create mode 100644 src/hooks/useGroupSearch.ts create mode 100644 src/server/scopes.names.test.ts diff --git a/src/components/configuration/ScopeSelector.tsx b/src/components/configuration/ScopeSelector.tsx index 9af2d913..aa69ccf3 100644 --- a/src/components/configuration/ScopeSelector.tsx +++ b/src/components/configuration/ScopeSelector.tsx @@ -10,12 +10,12 @@ import type * as t from '@/types'; import { availableScopesOptions, allRolesQueryOptions, - allGroupsQueryOptions, createScopeFn, deleteScopeFn, } from '@/server'; +import { SearchInput, Pagination } from '@/components/shared'; +import { useGroupSearch, useLocalize } from '@/hooks'; import { getScopeTypeConfig } from '@/constants'; -import { useLocalize } from '@/hooks'; import { cn } from '@/utils'; // ── Main selector ─────────────────────────────────────────────────── @@ -47,10 +47,8 @@ export function ScopeSelector({ enabled: open && showCreate, }); - const { data: allGroups = [] } = useQuery({ - ...allGroupsQueryOptions, - enabled: open && showCreate, - }); + const groupSearch = useGroupSearch(open && showCreate); + const { onSearchChange: onGroupSearchChange } = groupSearch; const handleSearchChange = useCallback((value: string) => { setSearch(value); @@ -62,7 +60,8 @@ export function ScopeSelector({ setCreating(false); setDeleteTarget(null); setDeleting(false); - }, []); + onGroupSearchChange(''); + }, [onGroupSearchChange]); const close = useCallback(() => { onOpenChange(false); @@ -104,8 +103,9 @@ export function ScopeSelector({ ); const availableGroups = useMemo( - () => allGroups.filter((g) => !existingScopeKeys.has(`${PrincipalType.GROUP}:${g.id}`)), - [allGroups, existingScopeKeys], + () => + groupSearch.groups.filter((g) => !existingScopeKeys.has(`${PrincipalType.GROUP}:${g.id}`)), + [groupSearch.groups, existingScopeKeys], ); const handleCreateForRole = useCallback( @@ -260,6 +260,56 @@ export function ScopeSelector({ if (showCreate) { const noRoles = availableRoles.length === 0; const noGroups = availableGroups.length === 0; + const groupsEmptyKey = groupSearch.search + ? 'com_scope_no_matching_groups' + : 'com_scope_no_available_groups'; + + const renderGroupsContent = () => { + if (groupSearch.isLoading) { + return ( +
+ +
+ ); + } + if (noGroups) { + return ( +

+ {localize(groupsEmptyKey)} +

+ ); + } + return ( +
+ {availableGroups.map((group) => ( + + ))} +
+ ); + }; return (
{localize('com_scope_groups')}
- {noGroups ? ( -

- {localize('com_scope_no_available_groups')} -

- ) : ( - availableGroups.map((group) => ( - - )) +
+ +
+ {renderGroupsContent()} + {groupSearch.totalPages > 1 && ( +
+ +
)} diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 8991119a..b4b32416 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -3,6 +3,7 @@ export * from './useAnnouncement'; export * from './useCapabilities'; export * from './useCommandMenu'; export * from './useDebouncedFilter'; +export * from './useGroupSearch'; export * from './useHighlightRef'; export * from './useLocalize'; export * from './useProfileMutations'; diff --git a/src/hooks/useGroupSearch.test.tsx b/src/hooks/useGroupSearch.test.tsx new file mode 100644 index 00000000..3f0e255a --- /dev/null +++ b/src/hooks/useGroupSearch.test.tsx @@ -0,0 +1,131 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, waitFor, act } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { ReactNode } from 'react'; + +const TOTAL_GROUPS = 120; + +const allGroups = [ + ...Array.from({ length: TOTAL_GROUPS }, (_, i) => ({ + _id: `group-${i + 1}`, + name: `Group ${i + 1}`, + memberIds: [], + source: 'entra', + })), + { _id: 'group-needle', name: 'Needle', memberIds: [], source: 'entra' }, +]; + +vi.mock('@tanstack/react-start', () => ({ + createServerFn: () => ({ + inputValidator: () => ({ + handler: (fn: (...args: unknown[]) => unknown) => fn, + }), + handler: (fn: (...args: unknown[]) => unknown) => fn, + }), + createServerOnlyFn: (fn: (...args: unknown[]) => unknown) => fn, +})); + +vi.mock('@/server/utils/api', () => ({ + apiFetch: vi.fn(async (url: string) => { + const params = new URLSearchParams(url.split('?')[1] ?? ''); + const search = params.get('search')?.toLowerCase() ?? ''; + const limit = Number(params.get('limit') ?? 200); + const offset = Number(params.get('offset') ?? 0); + const matches = search + ? allGroups.filter((group) => group.name.toLowerCase().includes(search)) + : allGroups; + return { + ok: true, + status: 200, + json: async () => ({ + groups: matches.slice(offset, offset + limit), + total: matches.length, + }), + }; + }), + extractApiError: vi.fn(async (_res: unknown, msg: string) => { + throw new Error(msg); + }), +})); + +import { useGroupSearch } from './useGroupSearch'; +import { GROUPS_PAGE_SIZE } from '@/server'; +import { apiFetch } from '@/server/utils/api'; + +const mockedApiFetch = vi.mocked(apiFetch); + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: 0 } }, + }); + return ({ children }: { children: ReactNode }) => ( + {children} + ); +} + +beforeEach(() => { + mockedApiFetch.mockClear(); +}); + +describe('useGroupSearch', () => { + it('fetches the first page with server-side pagination totals', async () => { + const { result } = renderHook(() => useGroupSearch(), { wrapper: createWrapper() }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.groups).toHaveLength(GROUPS_PAGE_SIZE); + expect(result.current.groups[0]?.name).toBe('Group 1'); + expect(result.current.total).toBe(TOTAL_GROUPS + 1); + expect(result.current.totalPages).toBe(3); + }); + + it('fetches groups beyond the first page when the page changes', async () => { + const { result } = renderHook(() => useGroupSearch(), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + act(() => result.current.setPage(3)); + + await waitFor(() => expect(result.current.groups[0]?.name).toBe('Group 101')); + expect(result.current.page).toBe(3); + expect(mockedApiFetch).toHaveBeenLastCalledWith( + `/api/admin/groups?limit=${GROUPS_PAGE_SIZE}&offset=${2 * GROUPS_PAGE_SIZE}`, + ); + }); + + it('debounces search on the server and resets to the first page', async () => { + const { result } = renderHook(() => useGroupSearch(), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + act(() => result.current.setPage(3)); + await waitFor(() => expect(result.current.groups[0]?.name).toBe('Group 101')); + + act(() => result.current.onSearchChange('Need')); + act(() => result.current.onSearchChange('Needle')); + + await waitFor(() => + expect(result.current.groups).toEqual([expect.objectContaining({ name: 'Needle' })]), + ); + expect(result.current.page).toBe(1); + expect(result.current.total).toBe(1); + + const searchUrls = mockedApiFetch.mock.calls + .map(([url]) => url) + .filter((url) => url.includes('search=')); + expect(searchUrls).toEqual([ + `/api/admin/groups?search=Needle&limit=${GROUPS_PAGE_SIZE}&offset=0`, + ]); + }); + + it('does not fetch until enabled', async () => { + const { result, rerender } = renderHook(({ enabled }) => useGroupSearch(enabled), { + wrapper: createWrapper(), + initialProps: { enabled: false }, + }); + + expect(mockedApiFetch).not.toHaveBeenCalled(); + expect(result.current.groups).toEqual([]); + + rerender({ enabled: true }); + + await waitFor(() => expect(result.current.groups).toHaveLength(GROUPS_PAGE_SIZE)); + }); +}); diff --git a/src/hooks/useGroupSearch.ts b/src/hooks/useGroupSearch.ts new file mode 100644 index 00000000..aa3ac6a1 --- /dev/null +++ b/src/hooks/useGroupSearch.ts @@ -0,0 +1,45 @@ +import { useCallback, useState } from 'react'; +import { keepPreviousData, useQuery } from '@tanstack/react-query'; +import type { AdminGroup } from '@librechat/data-schemas'; +import { groupsQueryOptions, GROUPS_PAGE_SIZE } from '@/server'; +import { useDebouncedFilter } from './useDebouncedFilter'; + +interface GroupSearch { + readonly search: string; + readonly onSearchChange: (next: string) => void; + readonly groups: AdminGroup[]; + readonly total: number; + readonly totalPages: number; + readonly page: number; + readonly setPage: (page: number) => void; + readonly isLoading: boolean; + readonly isFetching: boolean; +} + +/** Debounced server-side group search with offset pagination, so consumers + * can browse every group instead of a single capped page. Searching resets + * to the first page; previous results are kept while a new page loads. */ +export function useGroupSearch(enabled = true): GroupSearch { + const [page, setPage] = useState(1); + const resetPage = useCallback(() => setPage(1), []); + const { value, debouncedValue, onChange } = useDebouncedFilter('', resetPage); + + const { data, isLoading, isFetching } = useQuery({ + ...groupsQueryOptions(page, debouncedValue), + placeholderData: keepPreviousData, + enabled, + }); + + const total = data?.total ?? 0; + return { + search: value, + onSearchChange: onChange, + groups: data?.groups ?? [], + total, + totalPages: Math.ceil(total / GROUPS_PAGE_SIZE), + page, + setPage, + isLoading, + isFetching, + }; +} diff --git a/src/locales/en/translation.json b/src/locales/en/translation.json index a00cd996..b13b38ec 100644 --- a/src/locales/en/translation.json +++ b/src/locales/en/translation.json @@ -856,6 +856,7 @@ "com_scope_create_error": "Failed to create configuration", "com_scope_no_available_roles": "All roles already have configurations", "com_scope_no_available_groups": "All groups already have configurations", + "com_scope_no_matching_groups": "No groups match your search", "com_scope_preview": "Preview configuration", "com_scope_delete": "Delete configuration", "com_scope_delete_confirm": "Delete configuration for \"{{name}}\"? This removes all profile overrides.", diff --git a/src/server/groups.ts b/src/server/groups.ts index e25149d3..c2bc4076 100644 --- a/src/server/groups.ts +++ b/src/server/groups.ts @@ -45,6 +45,8 @@ export const MEMBERS_PAGE_SIZE = 50; /** Backend max per request is 200. Consumers needing all groups use this cap. */ const ALL_GROUPS_LIMIT = 200; +const NAME_LOOKUP_BATCH_SIZE = 25; + const GROUP_SOURCE_LOCAL = 'local' as const; // ── Server functions ───────────────────────────────────────────────── @@ -97,6 +99,31 @@ export const allGroupsQueryOptions = queryOptions({ staleTime: 30_000, }); +async function fetchGroupNameEntry(id: string): Promise<[string, string] | null> { + const response = await apiFetch(`/api/admin/groups/${encodeURIComponent(id)}`); + if (!response.ok) return null; + const { group } = (await response.json()) as { group: RawGroup }; + return [group._id, group.name]; +} + +/** + * Resolve group names for specific IDs via GET /api/admin/groups/:id, so + * resolution works regardless of how many groups exist. Fetches in small + * parallel batches; IDs that fail to resolve are simply omitted. + */ +export async function fetchGroupNamesByIds(ids: string[]): Promise> { + const unique = [...new Set(ids)]; + const nameMap = new Map(); + for (let i = 0; i < unique.length; i += NAME_LOOKUP_BATCH_SIZE) { + const batch = unique.slice(i, i + NAME_LOOKUP_BATCH_SIZE); + const entries = await Promise.all(batch.map((id) => fetchGroupNameEntry(id).catch(() => null))); + for (const entry of entries) { + if (entry) nameMap.set(entry[0], entry[1]); + } + } + return nameMap; +} + export const getGroupAssignmentsFn = createServerFn({ method: 'GET' }).handler( async (): Promise<{ assignments: Record }> => ({ assignments: {} }), ); diff --git a/src/server/scopes.names.test.ts b/src/server/scopes.names.test.ts new file mode 100644 index 00000000..850aba6d --- /dev/null +++ b/src/server/scopes.names.test.ts @@ -0,0 +1,168 @@ +import { PrincipalType } from 'librechat-data-provider'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { BASE_CONFIG_PRINCIPAL_ID } from '@librechat/data-schemas/capabilities'; + +const IN_WINDOW_GROUP = { _id: 'aaaaaaaaaaaaaaaaaaaaaaaa', name: 'Group In Window' }; +const BEYOND_WINDOW_GROUP = { _id: 'bbbbbbbbbbbbbbbbbbbbbbbb', name: 'Group Beyond Window' }; +const DELETED_GROUP_ID = 'cccccccccccccccccccccccc'; + +/** First page of the capped list endpoint: 200 groups, none of them beyond the window. */ +const firstPageGroups = [ + IN_WINDOW_GROUP, + ...Array.from({ length: 199 }, (_, i) => ({ _id: `filler-${i}`, name: `Filler ${i}` })), +]; + +const configs = [ + { + _id: 'cfg-base', + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + priority: 0, + isActive: true, + overrides: {}, + }, + { + _id: 'cfg-role', + principalType: PrincipalType.ROLE, + principalId: 'ADMIN', + priority: 10, + isActive: true, + overrides: {}, + }, + { + _id: 'cfg-group-in-window', + principalType: PrincipalType.GROUP, + principalId: IN_WINDOW_GROUP._id, + priority: 20, + isActive: true, + overrides: {}, + }, + { + _id: 'cfg-group-beyond-window', + principalType: PrincipalType.GROUP, + principalId: BEYOND_WINDOW_GROUP._id, + priority: 20, + isActive: true, + overrides: {}, + }, + { + _id: 'cfg-group-deleted', + principalType: PrincipalType.GROUP, + principalId: DELETED_GROUP_ID, + priority: 20, + isActive: false, + overrides: {}, + }, +]; + +const groupsById = new Map( + [IN_WINDOW_GROUP, BEYOND_WINDOW_GROUP].map((group) => [group._id, group]), +); + +function jsonResponse(body: object) { + return { ok: true, status: 200, json: async () => body }; +} + +vi.mock('./utils/api', () => ({ + apiFetch: vi.fn(async (url: string) => { + if (url === '/api/admin/config') return jsonResponse({ configs }); + if (url.startsWith('/api/admin/groups?')) { + return jsonResponse({ groups: firstPageGroups, total: 1500 }); + } + const byIdMatch = url.match(/^\/api\/admin\/groups\/([^/?]+)$/); + const group = byIdMatch ? groupsById.get(byIdMatch[1]) : undefined; + if (group) return jsonResponse({ group }); + return { ok: false, status: 404, json: async () => ({ error: 'Group not found' }) }; + }), + extractApiError: vi.fn(async (_res: unknown, msg: string) => { + throw new Error(msg); + }), +})); + +vi.mock('@tanstack/react-start', () => ({ + createServerFn: () => ({ + inputValidator: () => ({ + handler: (fn: (...args: unknown[]) => unknown) => fn, + }), + handler: (fn: (...args: unknown[]) => unknown) => fn, + }), +})); + +vi.mock('@tanstack/react-query', () => ({ + queryOptions: (opts: unknown) => opts, +})); + +import { getAvailableScopesFn } from './scopes'; +import { fetchGroupNamesByIds } from './groups'; +import { apiFetch } from './utils/api'; +import type * as t from '@/types'; + +const mockedApiFetch = vi.mocked(apiFetch); + +async function getScopesByPrincipalId(): Promise> { + const { scopes } = await getAvailableScopesFn(); + return new Map(scopes.map((scope) => [scope.principalId, scope])); +} + +beforeEach(() => { + mockedApiFetch.mockClear(); +}); + +describe('getAvailableScopesFn group name resolution', () => { + it('resolves group scope names beyond the first page of the groups list', async () => { + const scopes = await getScopesByPrincipalId(); + expect(scopes.get(BEYOND_WINDOW_GROUP._id)?.name).toBe(BEYOND_WINDOW_GROUP.name); + expect(scopes.get(IN_WINDOW_GROUP._id)?.name).toBe(IN_WINDOW_GROUP.name); + }); + + it('falls back to the principalId when a group cannot be fetched', async () => { + const scopes = await getScopesByPrincipalId(); + expect(scopes.get(DELETED_GROUP_ID)?.name).toBe(DELETED_GROUP_ID); + }); + + it('excludes the base config and keeps role principalIds as names', async () => { + const scopes = await getScopesByPrincipalId(); + expect(scopes.has(BASE_CONFIG_PRINCIPAL_ID)).toBe(false); + expect(scopes.get('ADMIN')?.name).toBe('ADMIN'); + }); + + it('fetches names only for group principalIds', async () => { + await getScopesByPrincipalId(); + const groupFetchUrls = mockedApiFetch.mock.calls + .map(([url]) => url) + .filter((url) => url.startsWith('/api/admin/groups')); + expect(groupFetchUrls.sort()).toEqual( + [ + `/api/admin/groups/${IN_WINDOW_GROUP._id}`, + `/api/admin/groups/${BEYOND_WINDOW_GROUP._id}`, + `/api/admin/groups/${DELETED_GROUP_ID}`, + ].sort(), + ); + }); +}); + +describe('fetchGroupNamesByIds', () => { + it('deduplicates ids and omits failed lookups', async () => { + const nameMap = await fetchGroupNamesByIds([ + IN_WINDOW_GROUP._id, + IN_WINDOW_GROUP._id, + DELETED_GROUP_ID, + ]); + expect(nameMap.get(IN_WINDOW_GROUP._id)).toBe(IN_WINDOW_GROUP.name); + expect(nameMap.has(DELETED_GROUP_ID)).toBe(false); + expect(mockedApiFetch).toHaveBeenCalledTimes(2); + }); + + it('resolves batches larger than the batch size', async () => { + const manyIds = Array.from({ length: 30 }, (_, i) => `many-${i}`); + for (const id of manyIds) { + groupsById.set(id, { _id: id, name: `Many ${id}` }); + } + const nameMap = await fetchGroupNamesByIds(manyIds); + expect(nameMap.size).toBe(30); + expect(nameMap.get('many-29')).toBe('Many many-29'); + for (const id of manyIds) { + groupsById.delete(id); + } + }); +}); diff --git a/src/server/scopes.ts b/src/server/scopes.ts index 6e654437..6fa8d8fc 100644 --- a/src/server/scopes.ts +++ b/src/server/scopes.ts @@ -16,18 +16,19 @@ import type { AdminConfig, } from '@librechat/data-schemas'; import type * as t from '@/types'; -import { isInterfacePermissionPath } from '@/utils/interfacePermissions'; -import { stripSecretPreviewValues } from '@/utils'; -import { BASE_CONFIG_PRINCIPAL_ID } from './constants'; -import { requireAnyCapability } from './capabilities'; -import { safeFieldPath } from './utils/validation'; -import { apiFetch } from './utils/api'; import { normalizeAppServiceKeys, parseIndexedArrayPath, mergeConfigArraySources, getSchemaPathSet, } from './config'; +import { isInterfacePermissionPath } from '@/utils/interfacePermissions'; +import { BASE_CONFIG_PRINCIPAL_ID } from './constants'; +import { requireAnyCapability } from './capabilities'; +import { stripSecretPreviewValues } from '@/utils'; +import { safeFieldPath } from './utils/validation'; +import { fetchGroupNamesByIds } from './groups'; +import { apiFetch } from './utils/api'; // ── Dot-path helpers ───────────────────────────────────────────────── @@ -134,20 +135,18 @@ function apiConfigToScope(config: AdminConfig, nameMap?: Map): t * Fetch all available scopes (all config overrides in the DB). */ export const getAvailableScopesFn = createServerFn({ method: 'GET' }).handler(async () => { - const [configRes, groupsRes] = await Promise.all([ - apiFetch('/api/admin/config'), - apiFetch('/api/admin/groups?limit=200').catch(() => null), - ]); + const configRes = await apiFetch('/api/admin/config'); if (!configRes.ok) { throw new Error(`Failed to fetch scopes: ${configRes.status}`); } const { configs } = (await configRes.json()) as AdminConfigListResponse; - const nameMap = new Map(); - if (groupsRes?.ok) { - const { groups } = (await groupsRes.json()) as { groups: { _id: string; name: string }[] }; - for (const g of groups) nameMap.set(g._id, g.name); - } + const groupIds = configs + .filter( + (c) => c.principalType === PrincipalType.GROUP && c.principalId !== BASE_CONFIG_PRINCIPAL_ID, + ) + .map((c) => c.principalId); + const nameMap = await fetchGroupNamesByIds(groupIds).catch(() => new Map()); const scopes: t.ConfigScope[] = configs .filter((c) => c.principalId !== BASE_CONFIG_PRINCIPAL_ID) From 95aa1ac54573cb77c974921619913427c27a0084 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:18:57 -0700 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=90=9B=20fix:=20Clamp=20group=20searc?= =?UTF-8?q?h=20strings=20to=20the=20backend's=20200=20character=20limit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/hooks/useGroupSearch.test.tsx | 17 +++++++++++++++++ src/server/groups.ts | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/hooks/useGroupSearch.test.tsx b/src/hooks/useGroupSearch.test.tsx index 3f0e255a..31e46493 100644 --- a/src/hooks/useGroupSearch.test.tsx +++ b/src/hooks/useGroupSearch.test.tsx @@ -115,6 +115,23 @@ describe('useGroupSearch', () => { ]); }); + it('truncates search strings to the backend limit instead of triggering a 400', async () => { + const { result } = renderHook(() => useGroupSearch(), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + act(() => result.current.onSearchChange('x'.repeat(250))); + + await waitFor(() => { + const searchUrls = mockedApiFetch.mock.calls + .map(([url]) => url) + .filter((url) => url.includes('search=')); + expect(searchUrls).toHaveLength(1); + }); + const url = mockedApiFetch.mock.calls.map(([u]) => u).find((u) => u.includes('search=')); + const sent = new URLSearchParams(url?.split('?')[1] ?? '').get('search') ?? ''; + expect(sent).toHaveLength(200); + }); + it('does not fetch until enabled', async () => { const { result, rerender } = renderHook(({ enabled }) => useGroupSearch(enabled), { wrapper: createWrapper(), diff --git a/src/server/groups.ts b/src/server/groups.ts index c2bc4076..68550134 100644 --- a/src/server/groups.ts +++ b/src/server/groups.ts @@ -47,6 +47,9 @@ const ALL_GROUPS_LIMIT = 200; const NAME_LOOKUP_BATCH_SIZE = 25; +/** Backend rejects search strings longer than 200 characters with a 400. */ +const MAX_SEARCH_LENGTH = 200; + const GROUP_SOURCE_LOCAL = 'local' as const; // ── Server functions ───────────────────────────────────────────────── @@ -66,7 +69,7 @@ export const getGroupsFn = createServerFn({ method: 'GET' }) data: { search?: string; limit?: number; offset?: number }; }): Promise<{ groups: AdminGroup[]; total: number }> => { const params = new URLSearchParams(); - if (data.search) params.set('search', data.search); + if (data.search) params.set('search', data.search.slice(0, MAX_SEARCH_LENGTH)); if (data.limit != null) params.set('limit', String(data.limit)); if (data.offset != null) params.set('offset', String(data.offset)); const qs = params.toString(); From 80c996a097a5a6c3c3ea067ec2c0a3d9feb37267 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:34:31 -0700 Subject: [PATCH 3/7] =?UTF-8?q?=F0=9F=90=9B=20fix:=20Keep=20Group=20Pagina?= =?UTF-8?q?tion=20Truthful=20in=20the=20Scope=20Create=20View?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../configuration/ScopeSelector.test.tsx | 186 ++++++++++++++++++ .../configuration/ScopeSelector.tsx | 74 ++++--- src/hooks/useGroupSearch.ts | 16 +- src/locales/en/translation.json | 2 +- src/types/hooks.ts | 13 ++ 5 files changed, 247 insertions(+), 44 deletions(-) create mode 100644 src/components/configuration/ScopeSelector.test.tsx diff --git a/src/components/configuration/ScopeSelector.test.tsx b/src/components/configuration/ScopeSelector.test.tsx new file mode 100644 index 00000000..c78d8921 --- /dev/null +++ b/src/components/configuration/ScopeSelector.test.tsx @@ -0,0 +1,186 @@ +import { describe, it, expect, vi } from 'vitest'; +import { PrincipalType } from 'librechat-data-provider'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { ReactNode } from 'react'; + +const PAGE_SIZE = 50; +const TOTAL_GROUPS = 60; + +const allGroups = Array.from({ length: TOTAL_GROUPS }, (_, i) => ({ + _id: `grp-${i + 1}`, + name: `Group ${String(i + 1).padStart(2, '0')}`, + memberIds: [], + source: 'entra', +})); + +/** Every group on the first page already has a configuration; page two has eligible groups. */ +const configuredGroups = allGroups.slice(0, PAGE_SIZE); + +const configs = configuredGroups.map((group) => ({ + _id: `cfg-${group._id}`, + principalType: PrincipalType.GROUP, + principalId: group._id, + priority: 20, + isActive: true, + overrides: {}, +})); + +vi.mock('@tanstack/react-start', () => ({ + createServerFn: () => ({ + inputValidator: () => ({ + handler: (fn: (...args: unknown[]) => unknown) => fn, + }), + handler: (fn: (...args: unknown[]) => unknown) => fn, + }), + createServerOnlyFn: (fn: (...args: unknown[]) => unknown) => fn, +})); + +vi.mock('@/server/utils/api', () => ({ + apiFetch: vi.fn(async (url: string) => { + const json = (body: object) => ({ ok: true, status: 200, json: async () => body }); + if (url === '/api/admin/config') return json({ configs }); + if (url.startsWith('/api/admin/roles')) return json({ roles: [], total: 0 }); + if (url.startsWith('/api/admin/groups?')) { + const params = new URLSearchParams(url.split('?')[1]); + const limit = Number(params.get('limit') ?? 200); + const offset = Number(params.get('offset') ?? 0); + return json({ groups: allGroups.slice(offset, offset + limit), total: allGroups.length }); + } + const byId = allGroups.find((group) => url === `/api/admin/groups/${group._id}`); + if (byId) return json({ group: byId }); + return { ok: false, status: 404, json: async () => ({ error: 'Not found' }) }; + }), + extractApiError: vi.fn(async (_res: unknown, msg: string) => { + throw new Error(msg); + }), +})); + +vi.mock('@/hooks/useLocalize', () => ({ + useLocalize: () => (key: string) => key, +})); + +interface ChildrenProps { + children?: ReactNode; +} +interface CommandDialogProps extends ChildrenProps { + open: boolean; +} +interface CommandItemProps extends ChildrenProps { + onSelect?: () => void; +} +interface MockButtonProps { + label: string; + onClick?: () => void; +} +interface MockIconProps { + name: string; +} +interface MockSearchFieldProps { + value: string; + onChange: (value: string) => void; + placeholder?: string; + 'aria-label'?: string; +} +interface MockPaginationProps { + currentPage: number; + totalPages: number; + onChange: (page: number) => void; +} + +vi.mock('cmdk', () => { + const Passthrough = ({ children }: ChildrenProps) =>
{children}
; + return { + Command: Object.assign(Passthrough, { + Dialog: ({ open, children }: CommandDialogProps) => + open ?
{children}
: null, + Input: () => , + List: Passthrough, + Group: Passthrough, + Item: ({ children, onSelect }: CommandItemProps) => ( +
onSelect?.()}>{children}
+ ), + Empty: Passthrough, + }), + }; +}); + +vi.mock('@clickhouse/click-ui', () => ({ + Button: ({ label, onClick }: MockButtonProps) => ( + + ), + Icon: ({ name }: MockIconProps) => , + SearchField: (props: MockSearchFieldProps) => ( + props.onChange(e.target.value)} + /> + ), + Pagination: ({ currentPage, totalPages, onChange }: MockPaginationProps) => ( + + ), +})); + +vi.mock('@radix-ui/react-visually-hidden', () => ({ + VisuallyHidden: ({ children }: ChildrenProps) => , +})); + +vi.mock('@radix-ui/react-dialog', () => ({ + Title: ({ children }: ChildrenProps) => {children}, + Description: ({ children }: ChildrenProps) => {children}, +})); + +import { ScopeSelector } from './ScopeSelector'; + +async function renderCreateView() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + render( + + {}} + currentSelection={{ type: 'BASE' }} + onSelect={() => {}} + permissions={{ canView: true, canEdit: true, canAssign: true }} + /> + , + ); + fireEvent.click(await screen.findByText('com_scope_create')); + await screen.findByText('Group 01'); +} + +describe('ScopeSelector create view group pagination', () => { + it('renders a fully configured page as disabled entries instead of a global empty state', async () => { + await renderCreateView(); + + expect(screen.queryByText('com_access_groups_empty')).toBeNull(); + expect(screen.queryByText('com_scope_no_matching_groups')).toBeNull(); + + const configuredButton = screen.getByText('Group 01').closest('button'); + expect(configuredButton).toBeDisabled(); + expect(screen.getAllByText('com_scope_already_configured')).toHaveLength(PAGE_SIZE); + }); + + it('keeps pagination truthful so eligible groups on later pages stay reachable', async () => { + await renderCreateView(); + + const pagination = screen.getByTestId('pagination'); + expect(pagination.dataset.total).toBe('2'); + + fireEvent.click(screen.getByText('next-page')); + + const eligibleButton = (await screen.findByText('Group 51')).closest('button'); + expect(eligibleButton).toBeEnabled(); + expect(screen.queryByText('com_scope_already_configured')).toBeNull(); + }); +}); diff --git a/src/components/configuration/ScopeSelector.tsx b/src/components/configuration/ScopeSelector.tsx index aa69ccf3..aaf76305 100644 --- a/src/components/configuration/ScopeSelector.tsx +++ b/src/components/configuration/ScopeSelector.tsx @@ -102,10 +102,9 @@ export function ScopeSelector({ [allRoles, existingScopeKeys], ); - const availableGroups = useMemo( - () => - groupSearch.groups.filter((g) => !existingScopeKeys.has(`${PrincipalType.GROUP}:${g.id}`)), - [groupSearch.groups, existingScopeKeys], + const isGroupConfigured = useCallback( + (group: AdminGroup) => existingScopeKeys.has(`${PrincipalType.GROUP}:${group.id}`), + [existingScopeKeys], ); const handleCreateForRole = useCallback( @@ -259,10 +258,10 @@ export function ScopeSelector({ if (showCreate) { const noRoles = availableRoles.length === 0; - const noGroups = availableGroups.length === 0; + const noGroups = groupSearch.groups.length === 0; const groupsEmptyKey = groupSearch.search ? 'com_scope_no_matching_groups' - : 'com_scope_no_available_groups'; + : 'com_access_groups_empty'; const renderGroupsContent = () => { if (groupSearch.isLoading) { @@ -283,30 +282,47 @@ export function ScopeSelector({ } return (
- {availableGroups.map((group) => ( - - ))} + > + + + + + {group.name} + + {configured && ( + + {localize('com_scope_already_configured')} + + )} + + {group.description && ( + + {group.description} + + )} + + + ); + })}
); }; diff --git a/src/hooks/useGroupSearch.ts b/src/hooks/useGroupSearch.ts index aa3ac6a1..0606eb6b 100644 --- a/src/hooks/useGroupSearch.ts +++ b/src/hooks/useGroupSearch.ts @@ -1,25 +1,13 @@ import { useCallback, useState } from 'react'; import { keepPreviousData, useQuery } from '@tanstack/react-query'; -import type { AdminGroup } from '@librechat/data-schemas'; +import type * as t from '@/types'; import { groupsQueryOptions, GROUPS_PAGE_SIZE } from '@/server'; import { useDebouncedFilter } from './useDebouncedFilter'; -interface GroupSearch { - readonly search: string; - readonly onSearchChange: (next: string) => void; - readonly groups: AdminGroup[]; - readonly total: number; - readonly totalPages: number; - readonly page: number; - readonly setPage: (page: number) => void; - readonly isLoading: boolean; - readonly isFetching: boolean; -} - /** Debounced server-side group search with offset pagination, so consumers * can browse every group instead of a single capped page. Searching resets * to the first page; previous results are kept while a new page loads. */ -export function useGroupSearch(enabled = true): GroupSearch { +export function useGroupSearch(enabled = true): t.GroupSearch { const [page, setPage] = useState(1); const resetPage = useCallback(() => setPage(1), []); const { value, debouncedValue, onChange } = useDebouncedFilter('', resetPage); diff --git a/src/locales/en/translation.json b/src/locales/en/translation.json index b13b38ec..ba87f5f1 100644 --- a/src/locales/en/translation.json +++ b/src/locales/en/translation.json @@ -855,8 +855,8 @@ "com_scope_creating": "Creating...", "com_scope_create_error": "Failed to create configuration", "com_scope_no_available_roles": "All roles already have configurations", - "com_scope_no_available_groups": "All groups already have configurations", "com_scope_no_matching_groups": "No groups match your search", + "com_scope_already_configured": "Already configured", "com_scope_preview": "Preview configuration", "com_scope_delete": "Delete configuration", "com_scope_delete_confirm": "Delete configuration for \"{{name}}\"? This removes all profile overrides.", diff --git a/src/types/hooks.ts b/src/types/hooks.ts index e70c52a4..c4f8f775 100644 --- a/src/types/hooks.ts +++ b/src/types/hooks.ts @@ -1,3 +1,4 @@ +import type { AdminGroup } from '@librechat/data-schemas'; import type { UseMutationResult } from '@tanstack/react-query'; import type { PrincipalType } from 'librechat-data-provider'; @@ -32,6 +33,18 @@ export interface UseProfileMutationsReturn { saving: boolean; } +export interface GroupSearch { + readonly search: string; + readonly onSearchChange: (next: string) => void; + readonly groups: AdminGroup[]; + readonly total: number; + readonly totalPages: number; + readonly page: number; + readonly setPage: (page: number) => void; + readonly isLoading: boolean; + readonly isFetching: boolean; +} + export interface ReorderVoiceover { item: (position: number) => string; lifted: (position: number) => string; From afdb4de2eee07ae8ba52214d2d47109d19da7998 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:42:57 -0700 Subject: [PATCH 4/7] =?UTF-8?q?=F0=9F=90=9B=20fix:=20Reset=20Group=20Searc?= =?UTF-8?q?h=20Synchronously=20When=20Leaving=20the=20Create=20View?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../configuration/ScopeSelector.test.tsx | 33 ++++++++++++++++++- .../configuration/ScopeSelector.tsx | 15 ++++++--- src/hooks/useDebouncedFilter.ts | 10 +++++- src/hooks/useGroupSearch.test.tsx | 25 ++++++++++++++ src/hooks/useGroupSearch.ts | 8 ++++- src/locales/en/translation.json | 1 + src/types/hooks.ts | 2 ++ 7 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/components/configuration/ScopeSelector.test.tsx b/src/components/configuration/ScopeSelector.test.tsx index c78d8921..2d130cfa 100644 --- a/src/components/configuration/ScopeSelector.test.tsx +++ b/src/components/configuration/ScopeSelector.test.tsx @@ -43,9 +43,13 @@ vi.mock('@/server/utils/api', () => ({ if (url.startsWith('/api/admin/roles')) return json({ roles: [], total: 0 }); if (url.startsWith('/api/admin/groups?')) { const params = new URLSearchParams(url.split('?')[1]); + const search = params.get('search')?.toLowerCase() ?? ''; const limit = Number(params.get('limit') ?? 200); const offset = Number(params.get('offset') ?? 0); - return json({ groups: allGroups.slice(offset, offset + limit), total: allGroups.length }); + const matches = search + ? allGroups.filter((group) => group.name.toLowerCase().includes(search)) + : allGroups; + return json({ groups: matches.slice(offset, offset + limit), total: matches.length }); } const byId = allGroups.find((group) => url === `/api/admin/groups/${group._id}`); if (byId) return json({ group: byId }); @@ -139,6 +143,9 @@ vi.mock('@radix-ui/react-dialog', () => ({ })); import { ScopeSelector } from './ScopeSelector'; +import { apiFetch } from '@/server/utils/api'; + +const mockedApiFetch = vi.mocked(apiFetch); async function renderCreateView() { const queryClient = new QueryClient({ @@ -183,4 +190,28 @@ describe('ScopeSelector create view group pagination', () => { expect(eligibleButton).toBeEnabled(); expect(screen.queryByText('com_scope_already_configured')).toBeNull(); }); + + it('reopening the create view after a search starts from page 1 unfiltered', async () => { + await renderCreateView(); + + fireEvent.change(screen.getByLabelText('com_access_search_groups'), { + target: { value: 'Group 51' }, + }); + await screen.findByText('Group 51'); + expect(screen.queryByText('Group 01')).toBeNull(); + + fireEvent.click(screen.getByLabelText('com_scope_create_back')); + const callsAfterBack = mockedApiFetch.mock.calls.length; + fireEvent.click(screen.getByText('com_scope_create')); + + await screen.findByText('Group 01'); + expect(screen.getByLabelText('com_access_search_groups')).toHaveValue(''); + expect(screen.queryByText('Group 51')).toBeNull(); + + const staleUrls = mockedApiFetch.mock.calls + .slice(callsAfterBack) + .map(([url]) => url) + .filter((url) => url.includes('search=')); + expect(staleUrls).toEqual([]); + }); }); diff --git a/src/components/configuration/ScopeSelector.tsx b/src/components/configuration/ScopeSelector.tsx index aaf76305..9dfbbfd2 100644 --- a/src/components/configuration/ScopeSelector.tsx +++ b/src/components/configuration/ScopeSelector.tsx @@ -48,20 +48,24 @@ export function ScopeSelector({ }); const groupSearch = useGroupSearch(open && showCreate); - const { onSearchChange: onGroupSearchChange } = groupSearch; + const { reset: resetGroupSearch } = groupSearch; const handleSearchChange = useCallback((value: string) => { setSearch(value); if (listRef.current) listRef.current.scrollTop = 0; }, []); - const resetState = useCallback(() => { + const closeCreate = useCallback(() => { setShowCreate(false); + resetGroupSearch(); + }, [resetGroupSearch]); + + const resetState = useCallback(() => { setCreating(false); setDeleteTarget(null); setDeleting(false); - onGroupSearchChange(''); - }, [onGroupSearchChange]); + closeCreate(); + }, [closeCreate]); const close = useCallback(() => { onOpenChange(false); @@ -342,7 +346,8 @@ export function ScopeSelector({
+ ), +})); + +vi.mock('@/components/shared', () => ({ + LoadingState: () =>
loading
, + SearchInput: ({ value, onChange, placeholder }: MockSearchInputProps) => ( + onChange(e.target.value)} + /> + ), + EmptyState: ({ message }: { message: string }) =>
{message}
, + Pagination: () => null, + TrashButton: () => null, +})); + +import { GroupsTab } from './GroupsTab'; +import { apiFetch } from '@/server/utils/api'; + +const mockedApiFetch = vi.mocked(apiFetch); + +function renderTab() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + return render( {}} />, { wrapper }); +} + +describe('GroupsTab search clamping', () => { + it('clamps the search input to the backend limit so displayed and queried text agree', async () => { + renderTab(); + await screen.findByText('Group A'); + + fireEvent.change(screen.getByLabelText('group-search'), { + target: { value: 'x'.repeat(250) }, + }); + + expect(screen.getByLabelText('group-search')).toHaveValue('x'.repeat(200)); + + await waitFor(() => { + const searchUrls = mockedApiFetch.mock.calls + .map(([url]) => url) + .filter((url) => url.includes('search=')); + expect(searchUrls).toHaveLength(1); + }); + const url = mockedApiFetch.mock.calls.map(([u]) => u).find((u) => u.includes('search=')); + const sent = new URLSearchParams(url?.split('?')[1] ?? '').get('search') ?? ''; + expect(sent).toHaveLength(200); + }); +}); diff --git a/src/components/access/GroupsTab.tsx b/src/components/access/GroupsTab.tsx index 2819f3c5..ef8e0a1d 100644 --- a/src/components/access/GroupsTab.tsx +++ b/src/components/access/GroupsTab.tsx @@ -10,7 +10,7 @@ import { Pagination, TrashButton, } from '@/components/shared'; -import { deleteGroupFn, groupsQueryOptions, GROUPS_PAGE_SIZE } from '@/server'; +import { deleteGroupFn, groupsQueryOptions, GROUPS_PAGE_SIZE, MAX_SEARCH_LENGTH } from '@/server'; import { cn, notifySuccess, notifyError } from '@/utils'; import { useCapabilities, useLocalize } from '@/hooks'; import { EditGroupDialog } from './EditGroupDialog'; @@ -34,10 +34,11 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { }, []); const handleSearchChange = (value: string) => { - setSearch(value); + const clamped = value.slice(0, MAX_SEARCH_LENGTH); + setSearch(clamped); clearTimeout(debounceRef.current); debounceRef.current = setTimeout(() => { - setDebouncedSearch(value); + setDebouncedSearch(clamped); setPage(1); }, 300); }; diff --git a/src/components/configuration/ScopeSelector.test.tsx b/src/components/configuration/ScopeSelector.test.tsx index a61ef401..6a937093 100644 --- a/src/components/configuration/ScopeSelector.test.tsx +++ b/src/components/configuration/ScopeSelector.test.tsx @@ -10,6 +10,9 @@ const TOTAL_GROUPS = 60; /** When set, group list requests block until the promise resolves. */ const mockListGate: { promise: Promise | null } = { promise: null }; +/** When set, group list requests fail with a 500. */ +const mockListFail = { fail: false }; + const allGroups = Array.from({ length: TOTAL_GROUPS }, (_, i) => ({ _id: `grp-${i + 1}`, name: `Group ${String(i + 1).padStart(2, '0')}`, @@ -46,6 +49,9 @@ vi.mock('@/server/utils/api', () => ({ if (url.startsWith('/api/admin/roles')) return json({ roles: [], total: 0 }); if (url.startsWith('/api/admin/groups?')) { if (mockListGate.promise) await mockListGate.promise; + if (mockListFail.fail) { + return { ok: false, status: 500, json: async () => ({ error: 'boom' }) }; + } const params = new URLSearchParams(url.split('?')[1]); const search = params.get('search')?.toLowerCase() ?? ''; const limit = Number(params.get('limit') ?? 200); @@ -151,7 +157,7 @@ import { apiFetch } from '@/server/utils/api'; const mockedApiFetch = vi.mocked(apiFetch); -async function renderCreateView() { +async function openCreateView() { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }); @@ -167,11 +173,17 @@ async function renderCreateView() { , ); fireEvent.click(await screen.findByText('com_scope_create')); +} + +async function renderCreateView() { + await openCreateView(); await screen.findByText('Group 01'); } beforeEach(() => { mockListGate.promise = null; + mockListFail.fail = false; + mockedApiFetch.mockClear(); }); describe('ScopeSelector create view group pagination', () => { @@ -238,6 +250,52 @@ describe('ScopeSelector create view group pagination', () => { expect(screen.getByText('Group 51').closest('button')).toBeDisabled(); }); + it('ignores pagination clicks while a search request is in flight', async () => { + await renderCreateView(); + expect(screen.getByTestId('pagination')).toBeInTheDocument(); + + let release = () => {}; + mockListGate.promise = new Promise((resolve) => { + release = resolve; + }); + fireEvent.change(screen.getByLabelText('com_access_search_groups'), { + target: { value: 'Group 01' }, + }); + await waitFor(() => + expect(mockedApiFetch.mock.calls.some(([url]) => url.includes('search='))).toBe(true), + ); + + fireEvent.click(screen.getByText('next-page')); + + release(); + mockListGate.promise = null; + await screen.findByText('com_scope_already_configured'); + expect(screen.getByText('Group 01').closest('button')).toBeInTheDocument(); + expect(screen.queryByText('com_scope_no_matching_groups')).toBeNull(); + expect(screen.queryByTestId('pagination')).toBeNull(); + + const strandedUrls = mockedApiFetch.mock.calls + .map(([url]) => url) + .filter((url) => url.includes('search=') && url.includes('offset=50')); + expect(strandedUrls).toEqual([]); + }); + + it('renders a retryable error state instead of an empty list when the fetch fails', async () => { + mockListFail.fail = true; + await openCreateView(); + + await screen.findByText('com_error_load_groups'); + expect(screen.queryByText('com_access_groups_empty')).toBeNull(); + expect(screen.queryByText('com_scope_no_matching_groups')).toBeNull(); + expect(screen.queryByTestId('pagination')).toBeNull(); + + mockListFail.fail = false; + fireEvent.click(screen.getByText('com_ui_retry')); + + await screen.findByText('Group 01'); + expect(screen.queryByText('com_error_load_groups')).toBeNull(); + }); + it('reopening the create view after a search starts from page 1 unfiltered', async () => { await renderCreateView(); diff --git a/src/components/configuration/ScopeSelector.tsx b/src/components/configuration/ScopeSelector.tsx index 4737c707..f8214816 100644 --- a/src/components/configuration/ScopeSelector.tsx +++ b/src/components/configuration/ScopeSelector.tsx @@ -263,10 +263,15 @@ export function ScopeSelector({ if (showCreate) { const noRoles = availableRoles.length === 0; const noGroups = groupSearch.groups.length === 0; + const groupsBusy = groupSearch.isFetching || groupSearch.isSearchPending; const groupsEmptyKey = groupSearch.search ? 'com_scope_no_matching_groups' : 'com_access_groups_empty'; + const handleGroupPageChange = (page: number) => { + if (!groupsBusy) groupSearch.setPage(page); + }; + const renderGroupsContent = () => { if (groupSearch.isLoading) { return ( @@ -277,6 +282,20 @@ export function ScopeSelector({
); } + if (groupSearch.isError) { + return ( +
+

+ {localize('com_error_load_groups')} +

+
+ ); + } if (noGroups) { return (

@@ -285,12 +304,7 @@ export function ScopeSelector({ ); } return ( -

+
{groupSearch.groups.map((group) => { const configured = isGroupConfigured(group); return ( @@ -298,9 +312,7 @@ export function ScopeSelector({ key={group.id} type="button" onClick={() => handleCreateForGroup(group)} - disabled={ - creating || configured || groupSearch.isFetching || groupSearch.isSearchPending - } + disabled={creating || configured || groupsBusy} className={cn( 'scope-item w-full text-left', creating && 'pointer-events-none opacity-50', @@ -422,12 +434,17 @@ export function ScopeSelector({ />
{renderGroupsContent()} - {groupSearch.totalPages > 1 && ( -
+ {!groupSearch.isError && groupSearch.totalPages > 1 && ( +
)} diff --git a/src/hooks/useGroupSearch.test.tsx b/src/hooks/useGroupSearch.test.tsx index 6fbad22b..6b0d9c49 100644 --- a/src/hooks/useGroupSearch.test.tsx +++ b/src/hooks/useGroupSearch.test.tsx @@ -171,6 +171,16 @@ describe('useGroupSearch', () => { expect(result.current.total).toBe(1); }); + it('clamps the page back to the last valid page when the total shrinks below it', async () => { + const { result } = renderHook(() => useGroupSearch(), { wrapper: createWrapper() }); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + act(() => result.current.setPage(10)); + + await waitFor(() => expect(result.current.page).toBe(3)); + await waitFor(() => expect(result.current.groups[0]?.name).toBe('Group 101')); + }); + it('does not fetch until enabled', async () => { const { result, rerender } = renderHook(({ enabled }) => useGroupSearch(enabled), { wrapper: createWrapper(), diff --git a/src/hooks/useGroupSearch.ts b/src/hooks/useGroupSearch.ts index f77947f4..14caa70b 100644 --- a/src/hooks/useGroupSearch.ts +++ b/src/hooks/useGroupSearch.ts @@ -1,4 +1,4 @@ -import { useCallback, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { keepPreviousData, useQuery } from '@tanstack/react-query'; import type * as t from '@/types'; import { groupsQueryOptions, GROUPS_PAGE_SIZE, MAX_SEARCH_LENGTH } from '@/server'; @@ -22,24 +22,34 @@ export function useGroupSearch(enabled = true): t.GroupSearch { setPage(1); }, [resetFilter]); - const { data, isLoading, isFetching } = useQuery({ + const { data, isLoading, isFetching, isError, refetch } = useQuery({ ...groupsQueryOptions(page, debouncedValue), placeholderData: keepPreviousData, enabled, }); const total = data?.total ?? 0; + const totalPages = Math.ceil(total / GROUPS_PAGE_SIZE); + + useEffect(() => { + if (!data) return; + const lastPage = Math.max(1, totalPages); + if (page > lastPage) setPage(lastPage); + }, [data, page, totalPages]); + return { search: value, onSearchChange, reset, groups: data?.groups ?? [], total, - totalPages: Math.ceil(total / GROUPS_PAGE_SIZE), + totalPages, page, setPage, isLoading, isFetching, + isError, + refetch, isSearchPending: value !== debouncedValue, }; } diff --git a/src/types/hooks.ts b/src/types/hooks.ts index 9d8ac185..23b78ff6 100644 --- a/src/types/hooks.ts +++ b/src/types/hooks.ts @@ -45,6 +45,8 @@ export interface GroupSearch { readonly setPage: (page: number) => void; readonly isLoading: boolean; readonly isFetching: boolean; + readonly isError: boolean; + readonly refetch: () => void; /** True while the typed search has not yet been committed by the debounce. */ readonly isSearchPending: boolean; }