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 0942e63ca5d19..06ef67102b7d4 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts @@ -37,6 +37,7 @@ describe('getActionPickerStatus', () => { const status = getActionPickerStatus({ inputValue: 'foo', + filters: [], filterActionsAttempt: makeSuccessAttempt([]), allClusters: [], actionAttempts: [makeSuccessAttempt([])], @@ -47,10 +48,10 @@ describe('getActionPickerStatus', () => { }), }); - expect(status.status).toBe('finished'); + expect(status.inputState).toBe('some-input'); const { clustersWithExpiredCerts, nonRetryableResourceSearchErrors } = - status.status === 'finished' && status; + status.inputState === 'some-input' && status; expect([...clustersWithExpiredCerts]).toEqual([retryableError.clusterUri]); expect(nonRetryableResourceSearchErrors).toEqual([nonRetryableError]); @@ -66,6 +67,7 @@ describe('getActionPickerStatus', () => { const status = getActionPickerStatus({ inputValue: 'foo', + filters: [], filterActionsAttempt: makeSuccessAttempt([]), allClusters: [offlineCluster], actionAttempts: [makeSuccessAttempt([])], @@ -76,8 +78,9 @@ describe('getActionPickerStatus', () => { }), }); - expect(status.status).toBe('finished'); - const { clustersWithExpiredCerts } = status.status === 'finished' && status; + expect(status.inputState).toBe('some-input'); + const { clustersWithExpiredCerts } = + status.inputState === 'some-input' && status; expect(clustersWithExpiredCerts.size).toBe(2); expect(clustersWithExpiredCerts).toContain(offlineCluster.uri); @@ -104,6 +107,7 @@ describe('getActionPickerStatus', () => { ]; const status = getActionPickerStatus({ inputValue: 'foo', + filters: [], filterActionsAttempt: makeSuccessAttempt([]), allClusters: [], actionAttempts: [makeSuccessAttempt([])], @@ -114,8 +118,46 @@ describe('getActionPickerStatus', () => { }), }); - expect(status.status).toBe('finished'); - const { clustersWithExpiredCerts } = status.status === 'finished' && status; + expect(status.inputState).toBe('some-input'); + const { clustersWithExpiredCerts } = + status.inputState === 'some-input' && status; expect([...clustersWithExpiredCerts]).toEqual(['/clusters/foo']); }); + + it('returns non-retryable errors when fetching a preview after selecting a filter fails', () => { + const nonRetryableError = new ResourceSearchError( + '/clusters/bar', + 'server', + new Error('non-retryable error') + ); + const resourceSearchErrors = [ + new ResourceSearchError( + '/clusters/foo', + 'server', + new Error('ssh: cert has expired') + ), + nonRetryableError, + ]; + const status = getActionPickerStatus({ + inputValue: '', + filters: [{ filter: 'resource-type', resourceType: 'servers' }], + filterActionsAttempt: makeSuccessAttempt([]), + allClusters: [], + actionAttempts: [makeSuccessAttempt([])], + resourceSearchAttempt: makeSuccessAttempt({ + errors: resourceSearchErrors, + results: [], + search: 'foo', + }), + }); + + expect(status.inputState).toBe('no-input'); + + const { searchMode } = status.inputState === 'no-input' && status; + expect(searchMode.kind).toBe('preview'); + + const { nonRetryableResourceSearchErrors } = + searchMode.kind === 'preview' && searchMode; + expect(nonRetryableResourceSearchErrors).toEqual([nonRetryableError]); + }); }); diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index 98367d01db9ac..5a70a4f00b79a 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -38,6 +38,7 @@ 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'; @@ -152,6 +153,7 @@ export function ActionPicker(props: { input: ReactElement }) { () => getActionPickerStatus({ inputValue, + filters, filterActionsAttempt, actionAttempts, resourceSearchAttempt, @@ -159,6 +161,7 @@ export function ActionPicker(props: { input: ReactElement }) { }), [ inputValue, + filters, filterActionsAttempt, actionAttempts, resourceSearchAttempt, @@ -243,18 +246,41 @@ const ExtraTopComponents = (props: { }) => { const { status, getClusterName, showErrorsInModal } = props; - switch (status.status) { + switch (status.inputState) { case 'no-input': { - return ( - - ); - } - case 'processing': { - return null; + switch (status.searchMode.kind) { + case 'no-search': { + return ; + } + case 'preview': { + const { + nonRetryableResourceSearchErrors, + hasNoRemainingFilterActions, + } = status.searchMode; + + return ( + <> + + {nonRetryableResourceSearchErrors.length > 0 && ( + { + showErrorsInModal(nonRetryableResourceSearchErrors); + }} + /> + )} + + ); + } + default: { + return assertUnreachable(status.searchMode); + } + } } - case 'finished': { + case 'some-input': { return ( <> {status.nonRetryableResourceSearchErrors.length > 0 && ( @@ -281,11 +307,39 @@ const ExtraTopComponents = (props: { } }; +/** + * ActionPickerStatus helps with displaying ExtraTopComponents. It has two goals: + * + * * Encapsulate business logic so that anything that ExtraTopComponents renders can just read + * ActionPickerStatus fields. + * * Represent only valid UI states. For example, inputState 'no-input' doesn't have hasNoResults + * field as this field would make no sense in a situation where no search requests were made. + * + * As you may notice, ActionPickerStatus doesn't say whether the search request is in progress or + * not, simply because displaying the progress bar is handled by another component. The questions + * answered by ActionPickerStatus are valid to ask no matter what the state of the request is. + */ type ActionPickerStatus = - | { status: 'no-input'; hasNoRemainingFilterActions: boolean } - | { status: 'processing' } | { - status: 'finished'; + // no-input: The input is empty. + inputState: 'no-input'; + searchMode: + | { + // no-search: The search bar is pristine, that is the input and the filters are empty. + kind: 'no-search'; + } + | { + // preview: At least one filter is selected. The search bar is fetching or shows + // a preview of results matching the filters. + kind: 'preview'; + hasNoRemainingFilterActions: boolean; + nonRetryableResourceSearchErrors: ResourceSearchError[]; + }; + } + | { + // some-input: The input is not empty. The search bar is fetching or shows results matching + // the query and filters. + inputState: 'some-input'; hasNoResults: boolean; nonRetryableResourceSearchErrors: ResourceSearchError[]; clustersWithExpiredCerts: Set; @@ -293,18 +347,31 @@ 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 didNotSelectAnyFilters = filters.length === 0; + + // If the input is empty, we fetch the preview only after the user selected some filters. + // So at this point we know that no search request was sent. + if (didNotSelectAnyFilters) { + return { + inputState: 'no-input', + searchMode: { kind: 'no-search' }, + }; + } + // 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 @@ -316,30 +383,46 @@ export function getActionPickerStatus({ filterActionsAttempt.status === 'success' && filterActionsAttempt.data.length === 0; + const nonRetryableResourceSearchErrors = + resourceSearchAttempt.status === 'success' + ? resourceSearchAttempt.data.errors.filter( + err => !isRetryable(err.cause) + ) + : []; + return { - status: 'no-input', - hasNoRemainingFilterActions, + inputState: 'no-input', + searchMode: { + kind: 'preview', + hasNoRemainingFilterActions, + nonRetryableResourceSearchErrors, + }, }; } + const nonRetryableResourceSearchErrors = []; + const clustersWithExpiredCerts = new Set( + allClusters.filter(c => !c.connected).map(c => c.uri) + ); const haveActionAttemptsFinished = actionAttempts.every(attempt => hasFinished(attempt) ); if (!haveActionAttemptsFinished) { return { - status: 'processing', + inputState: 'some-input', + hasNoResults: false, + nonRetryableResourceSearchErrors, + clustersWithExpiredCerts, }; } const hasNoResults = actionAttempts.every( attempt => attempt.data.length === 0 ); - const clustersWithExpiredCerts = new Set( - allClusters.filter(c => !c.connected).map(c => c.uri) - ); - const nonRetryableResourceSearchErrors = []; + // We could assume that resourceSearchAttempt has finished since action attempts depend on it and + // we know that they all finished at this point. But we check status explicitly anyway. if (resourceSearchAttempt.status === 'success') { resourceSearchAttempt.data.errors.forEach(err => { if (isRetryable(err.cause)) { @@ -351,7 +434,7 @@ export function getActionPickerStatus({ } return { - status: 'finished', + inputState: 'some-input', hasNoResults, clustersWithExpiredCerts, nonRetryableResourceSearchErrors, diff --git a/web/packages/teleterm/src/ui/Search/pickers/results.story.tsx b/web/packages/teleterm/src/ui/Search/pickers/results.story.tsx index 6513f3a94f169..3e47c02431858 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/results.story.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/results.story.tsx @@ -357,6 +357,10 @@ const AuxiliaryItems = () => ( addWindowEventListener={() => ({ cleanup: () => {} })} ExtraTopComponent={ <> +