From ae6fda90a3fa6e0bc823875dde39ba9dcc7fd6c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 24 Apr 2023 13:02:39 +0200 Subject: [PATCH 1/5] Render ExtraTopComponent based on computed action picker status --- .../teleterm/src/ui/Search/SearchBar.test.tsx | 28 +-- .../ui/Search/pickers/ActionPicker.story.tsx | 20 +- .../ui/Search/pickers/ActionPicker.test.ts | 121 ++++++++++ .../src/ui/Search/pickers/ActionPicker.tsx | 228 +++++++++++++----- .../teleterm/src/ui/Search/useSearch.ts | 12 +- 5 files changed, 313 insertions(+), 96 deletions(-) create mode 100644 web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx index 64996b898cedf..02c7a8f5e69bd 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx @@ -78,7 +78,7 @@ it('does not display empty results copy after selecting two filters', () => { expect(results).not.toHaveTextContent('No matching results found'); }); -it('does display empty results copy after providing search query for which there is no results', () => { +it('displays empty results copy after providing search query for which there is no results', () => { const appContext = new MockAppContext(); appContext.workspacesService.setState(draft => { draft.rootClusterUri = '/clusters/foo'; @@ -110,22 +110,14 @@ it('does display empty results copy after providing search query for which there expect(results).toHaveTextContent('No matching results found.'); }); -it('does display empty results copy and excluded clusters after providing search query for which there is no results', () => { +it('includes offline cluster names in the empty results copy', () => { const appContext = new MockAppContext(); - jest - .spyOn(appContext.clustersService, 'getRootClusters') - .mockImplementation(() => [ - { - uri: '/clusters/teleport-12-ent.asteroid.earth', - name: 'teleport-12-ent.asteroid.earth', - connected: false, - leaf: false, - proxyHost: 'test:3030', - authClusterId: '73c4746b-d956-4f16-9848-4e3469f70762', - }, - ]); + const cluster = makeRootCluster({ connected: false }); + appContext.clustersService.setState(draftState => { + draftState.clusters.set(cluster.uri, cluster); + }); appContext.workspacesService.setState(draft => { - draft.rootClusterUri = '/clusters/foo'; + draft.rootClusterUri = cluster.uri; }); const mockActionAttempts = { @@ -153,7 +145,7 @@ it('does display empty results copy and excluded clusters after providing search const results = screen.getByRole('menu'); expect(results).toHaveTextContent('No matching results found.'); expect(results).toHaveTextContent( - 'The cluster teleport-12-ent.asteroid.earth was excluded from the search because you are not logged in to it.' + `The cluster ${cluster.name} was excluded from the search because you are not logged in to it.` ); }); @@ -216,6 +208,7 @@ it('notifies about resource search errors and allows to display details', () => }); it('maintains focus on the search input after closing a resource search error modal', async () => { + const user = userEvent.setup(); const appContext = new MockAppContext(); appContext.workspacesService.setState(draft => { draft.rootClusterUri = '/clusters/foo'; @@ -247,7 +240,8 @@ it('maintains focus on the search input after closing a resource search error mo ); - screen.getByRole('searchbox').focus(); + await user.type(screen.getByRole('searchbox'), 'foo'); + expect(screen.getByRole('menu')).toHaveTextContent( 'Some of the search results are incomplete.' ); 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 5cbfeeb85ecf4..00652ec466598 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx @@ -355,20 +355,16 @@ const AuxiliaryItems = () => ( ExtraTopComponent={ <> + window.alert('Error details')} + showErrorsInModal={() => window.alert('Error details')} errors={[ new ResourceSearchError( '/clusters/foo', @@ -381,7 +377,7 @@ const AuxiliaryItems = () => ( /> window.alert('Error details')} + showErrorsInModal={() => window.alert('Error details')} errors={[ new ResourceSearchError( '/clusters/bar', diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts new file mode 100644 index 0000000000000..160c10bd28a67 --- /dev/null +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts @@ -0,0 +1,121 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { makeSuccessAttempt } from 'shared/hooks/useAsync'; + +import { makeRootCluster } from 'teleterm/services/tshd/testHelpers'; +import { ResourceSearchError } from 'teleterm/ui/services/resources'; + +import { getActionPickerStatus } from './ActionPicker'; + +describe('getActionPickerStatus', () => { + it('partitions resource search errors into clusters with expired certs and non-retryable errors', () => { + const retryableError = new ResourceSearchError( + '/clusters/foo', + 'server', + new Error('ssh: cert has expired') + ); + + const nonRetryableError = new ResourceSearchError( + '/clusters/bar', + 'database', + new Error('whoops') + ); + + const status = getActionPickerStatus( + 'foo', + [], + [], + [makeSuccessAttempt([])], + makeSuccessAttempt({ + errors: [retryableError, nonRetryableError], + results: [], + search: 'foo', + }) + ); + + expect(status.status).toBe('finished'); + + const { clustersWithExpiredCerts, nonRetryableResourceSearchErrors } = + status.status === 'finished' && status; + + expect([...clustersWithExpiredCerts]).toEqual([retryableError.clusterUri]); + expect(nonRetryableResourceSearchErrors).toEqual([nonRetryableError]); + }); + + it('merges non-connected clusters with clusters that returned retryable errors', () => { + const offlineCluster = makeRootCluster({ connected: false }); + const retryableError = new ResourceSearchError( + '/clusters/foo', + 'server', + new Error('ssh: cert has expired') + ); + + const status = getActionPickerStatus( + 'foo', + [], + [offlineCluster], + [makeSuccessAttempt([])], + makeSuccessAttempt({ + errors: [retryableError], + results: [], + search: 'foo', + }) + ); + + expect(status.status).toBe('finished'); + const { clustersWithExpiredCerts } = status.status === 'finished' && status; + + expect(clustersWithExpiredCerts.size).toBe(2); + expect(clustersWithExpiredCerts).toContain(offlineCluster.uri); + expect(clustersWithExpiredCerts).toContain(retryableError.clusterUri); + }); + + it('includes a cluster with expired cert only once even if multiple requests fail with retryable errors', () => { + const retryableErrors = [ + new ResourceSearchError( + '/clusters/foo', + 'server', + new Error('ssh: cert has expired') + ), + new ResourceSearchError( + '/clusters/foo', + 'database', + new Error('ssh: cert has expired') + ), + new ResourceSearchError( + '/clusters/foo', + 'kube', + new Error('ssh: cert has expired') + ), + ]; + const status = getActionPickerStatus( + 'foo', + [], + [], + [makeSuccessAttempt([])], + makeSuccessAttempt({ + errors: retryableErrors, + results: [], + search: 'foo', + }) + ); + + expect(status.status).toBe('finished'); + const { clustersWithExpiredCerts } = status.status === 'finished' && status; + expect([...clustersWithExpiredCerts]).toEqual(['/clusters/foo']); + }); +}); diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index 5c0f85f688e59..a7d6c8622bb3e 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ReactElement, useCallback } from 'react'; +import React, { ReactElement, useCallback, useMemo } from 'react'; import styled from 'styled-components'; import { Box, @@ -26,7 +26,7 @@ import { } from 'design'; import * as icons from 'design/Icon'; import { Highlight } from 'shared/components/Highlight'; -import { hasFinished } from 'shared/hooks/useAsync'; +import { Attempt, hasFinished } from 'shared/hooks/useAsync'; import { useAppContext } from 'teleterm/ui/appContextProvider'; import { @@ -38,13 +38,17 @@ import { SearchResultServer, SearchResultCluster, SearchResultResourceType, + SearchFilter, } from 'teleterm/ui/Search/searchResult'; import * as tsh from 'teleterm/services/tshd/types'; import * as uri from 'teleterm/ui/uri'; import { ResourceSearchError } from 'teleterm/ui/services/resources'; +import { isRetryable } from 'teleterm/ui/utils/retryWithRelogin'; +import { assertUnreachable } from 'teleterm/ui/utils'; import { SearchAction } from '../actions'; import { useSearchContext } from '../SearchContext'; +import { CrossClusterResourceSearchResult } from '../useSearch'; import { useActionAttempts } from './useActionAttempts'; import { getParameterPicker } from './pickers'; @@ -77,6 +81,11 @@ export function ActionPicker(props: { input: ReactElement }) { resourceSearchAttempt, } = useActionAttempts(); const totalCountOfClusters = clustersService.getClusters().length; + // The order of attempts is important. Filter actions should be displayed before resource actions. + const actionAttempts = useMemo( + () => [filterActionsAttempt, resourceActionsAttempt], + [filterActionsAttempt, resourceActionsAttempt] + ); const getClusterName = useCallback( (resourceUri: uri.ClusterOrResourceUri) => { @@ -146,55 +155,38 @@ export function ActionPicker(props: { input: ReactElement }) { } } - let ExtraTopComponent = null; - // The order of attempts is important. Filter actions should be displayed before resource actions. - const actionAttempts = [filterActionsAttempt, resourceActionsAttempt]; - const attemptsHaveFinishedWithoutActions = actionAttempts.every( - a => hasFinished(a) && a.data.length === 0 + const actionPickerStatus = useMemo( + () => + getActionPickerStatus( + inputValue, + filters, + clustersService.getClusters(), + actionAttempts, + resourceSearchAttempt + ), + [ + inputValue, + filters, + clustersService, + actionAttempts, + resourceSearchAttempt, + ] ); - const noRemainingFilters = - filterActionsAttempt.status === 'success' && - filterActionsAttempt.data.length === 0; - - if (inputValue && attemptsHaveFinishedWithoutActions) { - ExtraTopComponent = ( - - ); - } - - if (!inputValue && noRemainingFilters) { - ExtraTopComponent = ; - } - - if ( - resourceSearchAttempt.status === 'success' && - resourceSearchAttempt.data.errors.length > 0 - ) { - const showErrorsInModal = () => { + const showErrorsInModal = useCallback( + errors => pauseUserInteraction( () => new Promise(resolve => { modalsService.openRegularDialog({ kind: 'resource-search-errors', - errors: resourceSearchAttempt.data.errors, + errors, getClusterName, onCancel: () => resolve(undefined), }); }) - ); - }; - - ExtraTopComponent = ( - <> - - {ExtraTopComponent} - - ); - } + ), + [pauseUserInteraction, modalsService, getClusterName] + ); return ( @@ -222,7 +214,13 @@ export function ActionPicker(props: { input: ReactElement }) { ), }; }} - ExtraTopComponent={ExtraTopComponent} + ExtraTopComponent={ + + } /> ); @@ -245,6 +243,109 @@ export const InputWrapper = styled(Flex).attrs({ px: 2 })` } `; +const ExtraTopComponents = (props: { + status: ActionPickerStatus; + getClusterName: (resourceUri: uri.ClusterOrResourceUri) => string; + showErrorsInModal: (errors: ResourceSearchError[]) => void; +}) => { + const { status, getClusterName, showErrorsInModal } = props; + + switch (status.status) { + case 'no-input': { + return status.hasSelectedAllFilters && ; + } + case 'processing': { + return null; + } + case 'finished': { + return ( + <> + {status.nonRetryableResourceSearchErrors.length > 0 && ( + { + showErrorsInModal(status.nonRetryableResourceSearchErrors); + }} + /> + )} + {status.hasNoResults && ( + + )} + + ); + } + default: { + assertUnreachable(status); + } + } +}; + +type ActionPickerStatus = + | { status: 'no-input'; hasSelectedAllFilters: boolean } + | { status: 'processing' } + | { + status: 'finished'; + hasNoResults: boolean; + nonRetryableResourceSearchErrors: ResourceSearchError[]; + clustersWithExpiredCerts: Set; + }; + +export function getActionPickerStatus( + inputValue: string, + filters: SearchFilter[], + allClusters: tsh.Cluster[], + actionAttempts: Attempt[], + resourceSearchAttempt: Attempt +): ActionPickerStatus { + if (!inputValue) { + const hasSelectedAllFilters = filters.length === 2; + + return { + status: 'no-input', + hasSelectedAllFilters, + }; + } + + const haveActionAttemptsFinished = actionAttempts.every(attempt => + hasFinished(attempt) + ); + + if (!haveActionAttemptsFinished) { + return { + status: 'processing', + }; + } + + const hasNoResults = actionAttempts.every( + attempt => attempt.data.length === 0 + ); + const clustersWithExpiredCerts = new Set( + allClusters.filter(c => !c.connected).map(c => c.uri) + ); + let nonRetryableResourceSearchErrors = []; + + if (resourceSearchAttempt.status === 'success') { + resourceSearchAttempt.data.errors.forEach(err => { + if (isRetryable(err.cause)) { + clustersWithExpiredCerts.add(err.clusterUri); + } else { + nonRetryableResourceSearchErrors.push(err); + } + }); + } + + return { + status: 'finished', + hasNoResults, + clustersWithExpiredCerts, + nonRetryableResourceSearchErrors, + }; +} + export const ComponentMap: Record< SearchResult['kind'], React.FC> @@ -478,15 +579,31 @@ export function KubeItem(props: SearchResultItem) { ); } -export function NoResultsItem(props: { clusters: tsh.Cluster[] }) { - const excludedClustersCopy = getExcludedClustersCopy(props.clusters); +export function NoResultsItem(props: { + clustersWithExpiredCerts: Set; + getClusterName: (resourceUri: uri.ClusterOrResourceUri) => string; +}) { + const clustersWithExpiredCerts = Array.from( + props.clustersWithExpiredCerts, + clusterUri => props.getClusterName(clusterUri) + ); + clustersWithExpiredCerts.sort(); + let expiredCertsCopy = ''; + + if (clustersWithExpiredCerts.length === 1) { + expiredCertsCopy = `The cluster ${clustersWithExpiredCerts[0]} was excluded from the search because you are not logged in to it.`; + } + + if (clustersWithExpiredCerts.length > 1) { + // prettier-ignore + expiredCertsCopy = `The following clusters were excluded from the search because you are not logged in to them: ${clustersWithExpiredCerts.join( ', ')}.`; + } + return ( No matching results found. - {excludedClustersCopy && ( - {excludedClustersCopy} - )} + {expiredCertsCopy && {expiredCertsCopy}} ); @@ -505,7 +622,7 @@ export function TypeToSearchItem() { export function ResourceSearchErrorsItem(props: { errors: ResourceSearchError[]; getClusterName: (resourceUri: uri.ClusterOrResourceUri) => string; - onShowDetails: () => void; + showErrorsInModal: () => void; }) { const { errors, getClusterName } = props; @@ -547,7 +664,7 @@ export function ResourceSearchErrorsItem(props: { css={` flex-shrink: 0; `} - onClick={props.onShowDetails} + onClick={props.showErrorsInModal} > Show details @@ -557,19 +674,6 @@ export function ResourceSearchErrorsItem(props: { ); } -function getExcludedClustersCopy(allClusters: tsh.Cluster[]): string { - // TODO(ravicious): Include leaf clusters. - const excludedClusters = allClusters.filter(c => !c.connected); - const excludedClustersString = excludedClusters.map(c => c.name).join(', '); - if (excludedClusters.length === 0) { - return ''; - } - if (excludedClusters.length === 1) { - return `The cluster ${excludedClustersString} was excluded from the search because you are not logged in to it.`; - } - return `Clusters ${excludedClustersString} were excluded from the search because you are not logged in to them.`; -} - function Labels( props: React.PropsWithChildren<{ searchResult: ResourceSearchResult; diff --git a/web/packages/teleterm/src/ui/Search/useSearch.ts b/web/packages/teleterm/src/ui/Search/useSearch.ts index 7a261669314ae..09162a771e560 100644 --- a/web/packages/teleterm/src/ui/Search/useSearch.ts +++ b/web/packages/teleterm/src/ui/Search/useSearch.ts @@ -34,6 +34,12 @@ import { import type * as resourcesServiceTypes from 'teleterm/ui/services/resources'; +export type CrossClusterResourceSearchResult = { + results: resourcesServiceTypes.SearchResult[]; + errors: resourcesServiceTypes.ResourceSearchError[]; + search: string; +}; + /** * useResourceSearch returns a function which searches for the given list of space-separated keywords across * all root and leaf clusters that the user is currently logged in to. @@ -48,11 +54,7 @@ export function useResourceSearch() { async ( search: string, filters: SearchFilter[] - ): Promise<{ - results: resourcesServiceTypes.SearchResult[]; - errors: resourcesServiceTypes.ResourceSearchError[]; - search: string; - }> => { + ): Promise => { // useResourceSearch has to return _something_ when the input is empty. Imagine this scenario: // // 1. The user types in 'data' into the search bar. From 0457769cee3af19d9d59d705cb4e1b166c4041d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 24 Apr 2023 13:54:39 +0200 Subject: [PATCH 2/5] Fix formatting --- web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index a7d6c8622bb3e..d72f49ab73e1e 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -596,7 +596,7 @@ export function NoResultsItem(props: { if (clustersWithExpiredCerts.length > 1) { // prettier-ignore - expiredCertsCopy = `The following clusters were excluded from the search because you are not logged in to them: ${clustersWithExpiredCerts.join( ', ')}.`; + expiredCertsCopy = `The following clusters were excluded from the search because you are not logged in to them: ${clustersWithExpiredCerts.join(', ')}.`; } return ( From a889718c7e284448569254fa004ca8f93bb64bf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 25 Apr 2023 11:45:47 +0200 Subject: [PATCH 3/5] Use named args for getActionPickerStatus --- .../ui/Search/pickers/ActionPicker.test.ts | 48 +++++++++---------- .../src/ui/Search/pickers/ActionPicker.tsx | 28 ++++++----- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts index 160c10bd28a67..6d965ae0796f0 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts @@ -35,17 +35,17 @@ describe('getActionPickerStatus', () => { new Error('whoops') ); - const status = getActionPickerStatus( - 'foo', - [], - [], - [makeSuccessAttempt([])], - makeSuccessAttempt({ + const status = getActionPickerStatus({ + inputValue: 'foo', + filters: [], + allClusters: [], + actionAttempts: [makeSuccessAttempt([])], + resourceSearchAttempt: makeSuccessAttempt({ errors: [retryableError, nonRetryableError], results: [], search: 'foo', - }) - ); + }), + }); expect(status.status).toBe('finished'); @@ -64,17 +64,17 @@ describe('getActionPickerStatus', () => { new Error('ssh: cert has expired') ); - const status = getActionPickerStatus( - 'foo', - [], - [offlineCluster], - [makeSuccessAttempt([])], - makeSuccessAttempt({ + const status = getActionPickerStatus({ + inputValue: 'foo', + filters: [], + allClusters: [offlineCluster], + actionAttempts: [makeSuccessAttempt([])], + resourceSearchAttempt: makeSuccessAttempt({ errors: [retryableError], results: [], search: 'foo', - }) - ); + }), + }); expect(status.status).toBe('finished'); const { clustersWithExpiredCerts } = status.status === 'finished' && status; @@ -102,17 +102,17 @@ describe('getActionPickerStatus', () => { new Error('ssh: cert has expired') ), ]; - const status = getActionPickerStatus( - 'foo', - [], - [], - [makeSuccessAttempt([])], - makeSuccessAttempt({ + const status = getActionPickerStatus({ + inputValue: 'foo', + filters: [], + allClusters: [], + actionAttempts: [makeSuccessAttempt([])], + resourceSearchAttempt: makeSuccessAttempt({ errors: retryableErrors, results: [], search: 'foo', - }) - ); + }), + }); expect(status.status).toBe('finished'); const { clustersWithExpiredCerts } = status.status === 'finished' && status; diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index d72f49ab73e1e..ba3ff5724c02a 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -157,13 +157,13 @@ export function ActionPicker(props: { input: ReactElement }) { const actionPickerStatus = useMemo( () => - getActionPickerStatus( + getActionPickerStatus({ inputValue, filters, - clustersService.getClusters(), actionAttempts, - resourceSearchAttempt - ), + resourceSearchAttempt, + allClusters: clustersService.getClusters(), + }), [ inputValue, filters, @@ -294,13 +294,19 @@ type ActionPickerStatus = clustersWithExpiredCerts: Set; }; -export function getActionPickerStatus( - inputValue: string, - filters: SearchFilter[], - allClusters: tsh.Cluster[], - actionAttempts: Attempt[], - resourceSearchAttempt: Attempt -): ActionPickerStatus { +export function getActionPickerStatus({ + inputValue, + filters, + allClusters, + actionAttempts, + resourceSearchAttempt, +}: { + inputValue: string; + filters: SearchFilter[]; + allClusters: tsh.Cluster[]; + actionAttempts: Attempt[]; + resourceSearchAttempt: Attempt; +}): ActionPickerStatus { if (!inputValue) { const hasSelectedAllFilters = filters.length === 2; From 8974ecd91e06b8c2b6011a12a13110c4ed2b2786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 25 Apr 2023 11:46:41 +0200 Subject: [PATCH 4/5] Use const for nonRetryableResourceSearchErrors --- web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index ba3ff5724c02a..7b44684f5be69 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -332,7 +332,7 @@ export function getActionPickerStatus({ const clustersWithExpiredCerts = new Set( allClusters.filter(c => !c.connected).map(c => c.uri) ); - let nonRetryableResourceSearchErrors = []; + const nonRetryableResourceSearchErrors = []; if (resourceSearchAttempt.status === 'success') { resourceSearchAttempt.data.errors.forEach(err => { From 83643bdd757a37c572497380885529eee5c3ea5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 25 Apr 2023 12:01:33 +0200 Subject: [PATCH 5/5] Fix logic behind determining remaining filters --- .../ui/Search/pickers/ActionPicker.test.ts | 6 ++-- .../src/ui/Search/pickers/ActionPicker.tsx | 28 ++++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts index 6d965ae0796f0..0942e63ca5d19 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts @@ -37,7 +37,7 @@ describe('getActionPickerStatus', () => { const status = getActionPickerStatus({ inputValue: 'foo', - filters: [], + filterActionsAttempt: makeSuccessAttempt([]), allClusters: [], actionAttempts: [makeSuccessAttempt([])], resourceSearchAttempt: makeSuccessAttempt({ @@ -66,7 +66,7 @@ describe('getActionPickerStatus', () => { const status = getActionPickerStatus({ inputValue: 'foo', - filters: [], + filterActionsAttempt: makeSuccessAttempt([]), allClusters: [offlineCluster], actionAttempts: [makeSuccessAttempt([])], resourceSearchAttempt: makeSuccessAttempt({ @@ -104,7 +104,7 @@ describe('getActionPickerStatus', () => { ]; const status = getActionPickerStatus({ inputValue: 'foo', - filters: [], + filterActionsAttempt: makeSuccessAttempt([]), allClusters: [], actionAttempts: [makeSuccessAttempt([])], resourceSearchAttempt: makeSuccessAttempt({ diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index 7b44684f5be69..b399bff3d2088 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -38,7 +38,6 @@ import { SearchResultServer, SearchResultCluster, SearchResultResourceType, - SearchFilter, } from 'teleterm/ui/Search/searchResult'; import * as tsh from 'teleterm/services/tshd/types'; import * as uri from 'teleterm/ui/uri'; @@ -159,17 +158,17 @@ export function ActionPicker(props: { input: ReactElement }) { () => getActionPickerStatus({ inputValue, - filters, + filterActionsAttempt, actionAttempts, resourceSearchAttempt, allClusters: clustersService.getClusters(), }), [ inputValue, - filters, - clustersService, + filterActionsAttempt, actionAttempts, resourceSearchAttempt, + clustersService, ] ); const showErrorsInModal = useCallback( @@ -252,7 +251,7 @@ const ExtraTopComponents = (props: { switch (status.status) { case 'no-input': { - return status.hasSelectedAllFilters && ; + return status.hasNoRemainingFilterActions && ; } case 'processing': { return null; @@ -285,7 +284,7 @@ const ExtraTopComponents = (props: { }; type ActionPickerStatus = - | { status: 'no-input'; hasSelectedAllFilters: boolean } + | { status: 'no-input'; hasNoRemainingFilterActions: boolean } | { status: 'processing' } | { status: 'finished'; @@ -296,23 +295,32 @@ type ActionPickerStatus = export function getActionPickerStatus({ inputValue, - filters, + filterActionsAttempt, allClusters, actionAttempts, resourceSearchAttempt, }: { inputValue: string; - filters: SearchFilter[]; + filterActionsAttempt: Attempt; allClusters: tsh.Cluster[]; actionAttempts: Attempt[]; resourceSearchAttempt: Attempt; }): ActionPickerStatus { if (!inputValue) { - const hasSelectedAllFilters = filters.length === 2; + // The number of available filters the user can select changes dynamically based on how many + // clusters are in the state. That's why instead of inspecting the filters array from + // SearchContext, we inspect the actual filter actions attempt to see if any further filter + // suggestions will be shown to the user. + // + // We also know that this attempt is always successful as filters are calculated in a sync way. + // They're converted into an attempt only to conform to the interface of ResultList. + const hasNoRemainingFilterActions = + filterActionsAttempt.status === 'success' && + filterActionsAttempt.data.length === 0; return { status: 'no-input', - hasSelectedAllFilters, + hasNoRemainingFilterActions, }; }