diff --git a/src/features/organizations/queries/getAllOrganizations.test.ts b/src/features/organizations/queries/getAllOrganizations.test.ts index b785d1d40..81eb27459 100644 --- a/src/features/organizations/queries/getAllOrganizations.test.ts +++ b/src/features/organizations/queries/getAllOrganizations.test.ts @@ -44,15 +44,35 @@ describe('buildAllOrganizationsUrl', () => { expect(buildAllOrganizationsUrl(2, '')).toBe('/Admin/Organization/?status=ne=DELETED&sort(name)&limit(24,37)'); }); - it('puts the name filter before the status condition (the API ignores name when status comes first)', () => { + it('sends a filter as a relevance search, not a case-sensitive substring match', () => { expect(buildAllOrganizationsUrl(0, 'acme')).toBe( - '/Admin/Organization/?name=ct=acme&status=ne=DELETED&sort(name)&limit(0,13)', + '/Admin/Organization/?search=acme&status=ne=DELETED&limit(0,13)', + ); + }); + + it('omits sort(name) while searching, because the order is the relevance ranking', () => { + expect(buildAllOrganizationsUrl(0, 'acme')).not.toContain('sort('); + }); + + it('still sorts by name when browsing unfiltered', () => { + expect(buildAllOrganizationsUrl(0, '')).toContain('sort(name)'); + }); + + it('treats a whitespace-only filter as unfiltered browsing', () => { + expect(buildAllOrganizationsUrl(0, ' ')).toBe( + '/Admin/Organization/?status=ne=DELETED&sort(name)&limit(0,13)', + ); + }); + + it('trims surrounding whitespace off a real filter', () => { + expect(buildAllOrganizationsUrl(0, ' acme ')).toBe( + '/Admin/Organization/?search=acme&status=ne=DELETED&limit(0,13)', ); }); it('URI-encodes the filter value', () => { expect(buildAllOrganizationsUrl(0, 'a&b=c')).toBe( - '/Admin/Organization/?name=ct=a%26b%3Dc&status=ne=DELETED&sort(name)&limit(0,13)', + '/Admin/Organization/?search=a%26b%3Dc&status=ne=DELETED&limit(0,13)', ); }); @@ -140,13 +160,30 @@ describe('getAllOrganizationsQueryOptions', () => { expect(mockedGet).toHaveBeenNthCalledWith(2, '/Admin/Organization/org-7'); }); - it('runs a name search when the value is not an id', async () => { + it('runs a relevance search when the value is not an id', async () => { mockedGet.mockResolvedValue({ data: [] }); await getAllOrganizationsQueryOptions(0, 'Acme').queryFn!({} as never); - expect(mockedGet).toHaveBeenCalledWith( - '/Admin/Organization/?name=ct=Acme&status=ne=DELETED&sort(name)&limit(0,13)', - ); + expect(mockedGet).toHaveBeenCalledWith('/Admin/Organization/?search=Acme&status=ne=DELETED&limit(0,13)'); + }); + + it('finds an organization whatever case the operator types', async () => { + mockedGet.mockResolvedValue({ data: [] }); + + await getAllOrganizationsQueryOptions(0, 'acme').queryFn!({} as never); + + expect(mockedGet).toHaveBeenCalledWith('/Admin/Organization/?search=acme&status=ne=DELETED&limit(0,13)'); + }); + + it('shares one cache entry across equivalent filters, and requests the normalized term', () => { + const key = (v: string) => getAllOrganizationsQueryOptions(0, v).queryKey; + expect(key(' acme ')).toEqual(key('acme')); + expect(key(' ')).toEqual(key('')); + }); + + it('survives a caller that has no filter value at all', () => { + expect(() => getAllOrganizationsQueryOptions(0, undefined as unknown as string)).not.toThrow(); + expect(buildAllOrganizationsUrl(0, undefined as unknown as string)).toContain('sort(name)'); }); }); diff --git a/src/features/organizations/queries/getAllOrganizations.ts b/src/features/organizations/queries/getAllOrganizations.ts index 34d06aa3b..97265c4ed 100644 --- a/src/features/organizations/queries/getAllOrganizations.ts +++ b/src/features/organizations/queries/getAllOrganizations.ts @@ -18,21 +18,34 @@ export interface AllOrganizationsPage { * Builds the Harper REST query for one page of the admin organization list. * Harper collections return a bare array (no total count), so we request one * record past the page boundary to learn whether a next page exists. + * + * `search` is ranked by relevance server-side, so `sort(name)` is sent only + * when browsing unfiltered: with a term the order is the ranking, and the + * server ignores any sort. Expects an already-normalized term — see + * {@link normalizeSearchValue}. */ export function buildAllOrganizationsUrl(pageIndex: number, nameFilter: string): string { const start = pageIndex * ALL_ORGANIZATIONS_PAGE_SIZE; + const search = normalizeSearchValue(nameFilter); const conditions = [ - // The name filter must precede the status condition: with status first, - // the API ignores the name condition entirely. - ...(nameFilter ? [`name=ct=${encodeURIComponent(nameFilter)}`] : []), + // Kept ahead of the status condition, as the name filter had to be. + ...(search ? [`search=${encodeURIComponent(search)}`] : []), // Terminated organizations stick around with status DELETED; hide them. 'status=ne=DELETED', - 'sort(name)', + ...(search ? [] : ['sort(name)']), `limit(${start},${start + ALL_ORGANIZATIONS_PAGE_SIZE + 1})`, ]; return `/Admin/Organization/?${conditions.join('&')}`; } +/** + * Normalizes a raw filter value. Whitespace-only is unfiltered browsing, not a + * search for spaces. + */ +export function normalizeSearchValue(value: string | undefined | null): string { + return typeof value === 'string' ? value.trim() : ''; +} + export async function getAllOrganizations(pageIndex: number, nameFilter: string): Promise { const { data } = await apiClient.get(buildAllOrganizationsUrl(pageIndex, nameFilter) as '/Organization/'); return { @@ -86,15 +99,19 @@ export async function getOrganizationForClusterPage(clusterId: string): Promise< } /** - * Query for the admin organization search. When the search value looks like an - * organization or cluster id (a lowercase `org-`/`clu-` prefix — see - * {@link detectEntityId}), it resolves that id server-side instead of running a - * title/name search, so pasting an id jumps straight to the matching org. + * Query for the admin organization search. A value that looks like an + * organization or cluster id (see {@link detectEntityId}) is resolved by id + * instead, so pasting an id jumps straight to the matching org. */ export function getAllOrganizationsQueryOptions(pageIndex: number, searchValue: string) { - const entity = detectEntityId(searchValue); + // Normalized once, for both the cache key and the request. Keying on the raw + // value made `acme`, `acme ` and ` acme` three entries resolving to one + // identical request, so a pasted trailing space refetched a search the cache + // already held. + const search = normalizeSearchValue(searchValue); + const entity = detectEntityId(search); return queryOptions({ - queryKey: ['admin-all-organizations', searchValue, pageIndex], + queryKey: ['admin-all-organizations', search, pageIndex], queryFn: () => { if (entity?.kind === 'organization') { return getOrganizationByIdPage(entity.id); @@ -102,7 +119,7 @@ export function getAllOrganizationsQueryOptions(pageIndex: number, searchValue: if (entity?.kind === 'cluster') { return getOrganizationForClusterPage(entity.id); } - return getAllOrganizations(pageIndex, searchValue); + return getAllOrganizations(pageIndex, search); }, // Keep the previous page on screen while the next one loads. placeholderData: keepPreviousData,