diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx index 00652ec466598..258bf6f3c0d7e 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx @@ -395,7 +395,8 @@ const AuxiliaryItems = () => ( ), ]} /> - + + } /> diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index 58855d16f0c24..a86c385b2db55 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -251,7 +251,11 @@ const ExtraTopComponents = (props: { switch (status.status) { case 'no-input': { - return status.hasNoRemainingFilterActions && ; + return ( + + ); } case 'processing': { return null; @@ -623,11 +627,17 @@ export function NoResultsItem(props: { ); } -export function TypeToSearchItem() { +export function TypeToSearchItem({ + hasNoRemainingFilterActions, +}: { + hasNoRemainingFilterActions: boolean; +}) { return ( - - Type something to search. + + Enter space-separated search terms. + {hasNoRemainingFilterActions || + ' Select a filter to narrow down the search.'} ); diff --git a/web/packages/teleterm/src/ui/Search/useSearch.test.tsx b/web/packages/teleterm/src/ui/Search/useSearch.test.tsx index 3113914329024..16d8aa62e21ee 100644 --- a/web/packages/teleterm/src/ui/Search/useSearch.test.tsx +++ b/web/packages/teleterm/src/ui/Search/useSearch.test.tsx @@ -24,6 +24,7 @@ import { makeServer, makeKube, makeLabelsList, + makeRootCluster, } from 'teleterm/services/tshd/testHelpers'; import { MockAppContextProvider } from '../fixtures/MockAppContextProvider'; @@ -128,24 +129,18 @@ describe('rankResults', () => { }); describe('useResourceSearch', () => { - it('does not limit results', async () => { + it('does not limit results returned by ResourcesService', async () => { const appContext = new MockAppContext(); + const cluster = makeRootCluster(); + appContext.clustersService.setState(draftState => { + draftState.clusters.set(cluster.uri, cluster); + }); const servers: SearchResult[] = Array(20) .fill(undefined) .map(() => ({ kind: 'server' as const, resource: makeServer({}), })); - appContext.clustersService.setState(draftState => { - draftState.clusters.set('/clusters/teleport-local', { - connected: true, - authClusterId: '73c4746b-d956-4f16-9848-4e3469f70762', - leaf: false, - name: 'teleport-local', - proxyHost: 'localhost:3080', - uri: '/clusters/teleport-local', - }); - }); jest .spyOn(appContext.resourcesService, 'searchResources') .mockResolvedValue([{ status: 'fulfilled', value: servers }]); @@ -158,7 +153,69 @@ describe('useResourceSearch', () => { ), }); const searchResult = await result.current('foo', []); + expect(searchResult.results).toEqual(servers); + expect(appContext.resourcesService.searchResources).toHaveBeenCalledWith({ + clusterUri: cluster.uri, + search: 'foo', + filter: undefined, + limit: 100, + }); + expect(appContext.resourcesService.searchResources).toHaveBeenCalledTimes( + 1 + ); + }); + + it('fetches only a preview if search is empty and there is at least one filter selected', async () => { + const appContext = new MockAppContext(); + const cluster = makeRootCluster(); + appContext.clustersService.setState(draftState => { + draftState.clusters.set(cluster.uri, cluster); + }); + jest + .spyOn(appContext.resourcesService, 'searchResources') + .mockResolvedValue([{ status: 'fulfilled', value: [] }]); + + const { result } = renderHook(() => useResourceSearch(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + const filter = { filter: 'cluster' as const, clusterUri: cluster.uri }; + await result.current('', [filter]); + + expect(appContext.resourcesService.searchResources).toHaveBeenCalledWith({ + clusterUri: cluster.uri, + search: '', + filter: undefined, + limit: 5, + }); + expect(appContext.resourcesService.searchResources).toHaveBeenCalledTimes( + 1 + ); + }); + + it('does not fetch any resources if search is empty and there are no filters selected', async () => { + const appContext = new MockAppContext(); + const cluster = makeRootCluster(); + appContext.clustersService.setState(draftState => { + draftState.clusters.set(cluster.uri, cluster); + }); + jest + .spyOn(appContext.resourcesService, 'searchResources') + .mockResolvedValue([{ status: 'fulfilled', value: [] }]); + + const { result } = renderHook(() => useResourceSearch(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + await result.current('', []); + expect(appContext.resourcesService.searchResources).not.toHaveBeenCalled(); }); }); diff --git a/web/packages/teleterm/src/ui/Search/useSearch.ts b/web/packages/teleterm/src/ui/Search/useSearch.ts index 09162a771e560..12a29bf32422a 100644 --- a/web/packages/teleterm/src/ui/Search/useSearch.ts +++ b/web/packages/teleterm/src/ui/Search/useSearch.ts @@ -55,17 +55,36 @@ export function useResourceSearch() { search: string, filters: SearchFilter[] ): Promise => { - // useResourceSearch has to return _something_ when the input is empty. Imagine this scenario: - // - // 1. The user types in 'data' into the search bar. - // 2. The search bar immediately returns filters plus it starts a resource search for 'foo'. - // 3. The user selects one of the filters before the backend response comes back. - // - // The request for 'foo' that was in flight needs to be canceled somehow. We can do that by - // issuing another one for empty input and making `useResourceSearch` return an empty array - // in that scenario. - if (!search) { - return { results: [], errors: [], search }; + const searchMode = getResourceSearchMode(search, filters); + let limit = 100; + + switch (searchMode) { + // useResourceSearch has to return _something_ even when we don't want to perform a search. + // Imagine this scenario: + // + // 1. The user types in 'dat' into the search bar. + // 2. The search bar immediately returns filters and it starts a resource search for 'dat'. + // 3. The user selects the database filter before the backend response comes back. + // + // The request for 'dat' that was in flight needs to be canceled by useAsync somehow. We can + // do that by calling useResourceSearch again, even with empty input. + case 'no-search': { + return { results: [], errors: [], search }; + } + case 'preview': { + // In preview mode we know that the user didn't specify any search terms. So instead of + // fetching all 100 resources for each request, we fetch only a bunch of them to show + // example results in the UI. + limit = 5; + break; + } + case 'full-search': { + // noop, limit remains at 100. + break; + } + default: { + assertUnreachable(searchMode); + } } const clusterSearchFilter = filters.find( @@ -89,11 +108,12 @@ export function useResourceSearch() { const promiseResults = ( await Promise.all( clustersToSearch.map(cluster => - resourcesService.searchResources( - cluster.uri, + resourcesService.searchResources({ + clusterUri: cluster.uri, search, - resourceTypeSearchFilter - ) + filter: resourceTypeSearchFilter, + limit, + }) ) ) ).flat(); @@ -335,6 +355,27 @@ function calculateScore( return { ...searchResult, labelMatches, score: searchResultScore }; } +type ResourceSearchMode = 'no-search' | 'preview' | 'full-search'; + +function getResourceSearchMode( + search: string, + filters: SearchFilter[] +): ResourceSearchMode { + // Trim the search to avoid sending requests with limit set to 100 if the user just pressed some + // spaces. + const trimmedSearch = search.trim(); + + if (!trimmedSearch) { + // The preview should be fetched only when at least one filter is selected. Otherwise we'd send + // three requests for each connected cluster when the search bar gets open. + if (filters.length >= 1) { + return 'preview'; + } + return 'no-search'; + } + return 'full-search'; +} + function getLengthScore(searchTerm: string, matchedValue: string): number { return Math.floor((searchTerm.length / matchedValue.length) * 100); } diff --git a/web/packages/teleterm/src/ui/services/resources/resourcesService.test.ts b/web/packages/teleterm/src/ui/services/resources/resourcesService.test.ts index 42b76b9041747..b053095ee3e4d 100644 --- a/web/packages/teleterm/src/ui/services/resources/resourcesService.test.ts +++ b/web/packages/teleterm/src/ui/services/resources/resourcesService.test.ts @@ -120,11 +120,12 @@ describe('searchResources', () => { }; const service = new ResourcesService(tshClient as tsh.TshClient); - const searchResults = await service.searchResources( - '/clusters/foo', - '', - undefined - ); + const searchResults = await service.searchResources({ + clusterUri: '/clusters/foo', + search: '', + filter: undefined, + limit: 10, + }); expect(searchResults).toHaveLength(3); const [actualServers, actualDatabases, actualKubes] = searchResults; @@ -153,9 +154,14 @@ describe('searchResources', () => { }; const service = new ResourcesService(tshClient as tsh.TshClient); - const searchResults = await service.searchResources('/clusters/foo', '', { - filter: 'resource-type', - resourceType: 'servers', + const searchResults = await service.searchResources({ + clusterUri: '/clusters/foo', + search: '', + filter: { + filter: 'resource-type', + resourceType: 'servers', + }, + limit: 10, }); expect(searchResults).toHaveLength(1); @@ -175,11 +181,12 @@ describe('searchResources', () => { }; const service = new ResourcesService(tshClient as tsh.TshClient); - const searchResults = await service.searchResources( - '/clusters/foo', - '', - undefined - ); + const searchResults = await service.searchResources({ + clusterUri: '/clusters/foo', + search: '', + filter: undefined, + limit: 10, + }); expect(searchResults).toHaveLength(3); const [actualServers, actualDatabases, actualKubes] = searchResults; diff --git a/web/packages/teleterm/src/ui/services/resources/resourcesService.ts b/web/packages/teleterm/src/ui/services/resources/resourcesService.ts index 2108490911995..fd9e03d1613b0 100644 --- a/web/packages/teleterm/src/ui/services/resources/resourcesService.ts +++ b/web/packages/teleterm/src/ui/services/resources/resourcesService.ts @@ -70,14 +70,20 @@ export class ResourcesService { * The results need to be wrapped in SearchResult because if we returned raw types (Server, * Database, Kube) then there would be no easy way to differentiate between them on type level. */ - async searchResources( - clusterUri: uri.ClusterUri, - search: string, + async searchResources({ + clusterUri, + search, + filter, + limit, + }: { + clusterUri: uri.ClusterUri; + search: string; // TODO(ravicious): Accept just `server | database | kube` as searchFilter here, wrap it in a // variant of a discriminated union in searchResult.ts. - searchFilter: ResourceTypeSearchFilter | undefined - ): Promise[]> { - const params = { search, clusterUri, sort: null, limit: 100 }; + filter: ResourceTypeSearchFilter | undefined; + limit: number; + }): Promise[]> { + const params = { search, clusterUri, sort: null, limit }; const getServers = () => this.fetchServers(params).then( @@ -109,11 +115,11 @@ export class ResourcesService { err => Promise.reject(new ResourceSearchError(clusterUri, 'kube', err)) ); - const promises = searchFilter + const promises = filter ? [ - searchFilter.resourceType === 'servers' && getServers(), - searchFilter.resourceType === 'databases' && getDatabases(), - searchFilter.resourceType === 'kubes' && getKubes(), + filter.resourceType === 'servers' && getServers(), + filter.resourceType === 'databases' && getDatabases(), + filter.resourceType === 'kubes' && getKubes(), ].filter(Boolean) : [getServers(), getDatabases(), getKubes()];