Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ const AuxiliaryItems = () => (
),
]}
/>
<TypeToSearchItem />
<TypeToSearchItem hasNoRemainingFilterActions={false} />
<TypeToSearchItem hasNoRemainingFilterActions={true} />
</>
}
/>
Expand Down
18 changes: 14 additions & 4 deletions web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,11 @@ const ExtraTopComponents = (props: {

switch (status.status) {
case 'no-input': {
return status.hasNoRemainingFilterActions && <TypeToSearchItem />;
return (
<TypeToSearchItem
hasNoRemainingFilterActions={status.hasNoRemainingFilterActions}
/>
);
}
case 'processing': {
return null;
Expand Down Expand Up @@ -623,11 +627,17 @@ export function NoResultsItem(props: {
);
}

export function TypeToSearchItem() {
export function TypeToSearchItem({
hasNoRemainingFilterActions,
}: {
hasNoRemainingFilterActions: boolean;
}) {
return (
<NonInteractiveItem>
<Text typography="body1" color="text.main">
Type something to search.
<Text typography="body2">
Enter space-separated search terms.
{hasNoRemainingFilterActions ||
' Select a filter to narrow down the search.'}
</Text>
</NonInteractiveItem>
);
Expand Down
79 changes: 68 additions & 11 deletions web/packages/teleterm/src/ui/Search/useSearch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
makeServer,
makeKube,
makeLabelsList,
makeRootCluster,
} from 'teleterm/services/tshd/testHelpers';

import { MockAppContextProvider } from '../fixtures/MockAppContextProvider';
Expand Down Expand Up @@ -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 }]);
Expand All @@ -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 }) => (
<MockAppContextProvider appContext={appContext}>
{children}
</MockAppContextProvider>
),
});
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 }) => (
<MockAppContextProvider appContext={appContext}>
{children}
</MockAppContextProvider>
),
});
await result.current('', []);
expect(appContext.resourcesService.searchResources).not.toHaveBeenCalled();
});
});

Expand Down
71 changes: 56 additions & 15 deletions web/packages/teleterm/src/ui/Search/useSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,36 @@ export function useResourceSearch() {
search: string,
filters: SearchFilter[]
): Promise<CrossClusterResourceSearchResult> => {
// 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(
Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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;
Expand Down
26 changes: 16 additions & 10 deletions web/packages/teleterm/src/ui/services/resources/resourcesService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PromiseSettledResult<SearchResult[]>[]> {
const params = { search, clusterUri, sort: null, limit: 100 };
filter: ResourceTypeSearchFilter | undefined;
limit: number;
}): Promise<PromiseSettledResult<SearchResult[]>[]> {
const params = { search, clusterUri, sort: null, limit };

const getServers = () =>
this.fetchServers(params).then(
Expand Down Expand Up @@ -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()];

Expand Down