diff --git a/web/packages/shared/utils/getDurationText.ts b/web/packages/shared/utils/getDurationText.ts index 14c2b23222d58..41a552a2054dc 100644 --- a/web/packages/shared/utils/getDurationText.ts +++ b/web/packages/shared/utils/getDurationText.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { pluralize } from 'teleport/lib/util'; +import { pluralize } from './text'; export function getDurationText(hrs: number, mins: number, secs: number) { if (!hrs && !mins) { diff --git a/web/packages/shared/utils/text.ts b/web/packages/shared/utils/text.ts new file mode 100644 index 0000000000000..e4772876df209 --- /dev/null +++ b/web/packages/shared/utils/text.ts @@ -0,0 +1,30 @@ +/** + * 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. + */ + +// If you ever need to pluralize a word which cannot be pluralized by appending 's', just add a +// third optional argument which is the pluralized noun. +// https://api.rubyonrails.org/v7.0.4.2/classes/ActionView/Helpers/TextHelper.html#method-i-pluralize + +/** + * pluralize adds an 's' to the given word if num is bigger than 1. + */ +export function pluralize(num: number, word: string) { + if (num > 1) { + return `${word}s`; + } + + return word; +} diff --git a/web/packages/teleport/src/lib/util.ts b/web/packages/teleport/src/lib/util.ts index 83d70301101de..e9409ea11ae45 100644 --- a/web/packages/teleport/src/lib/util.ts +++ b/web/packages/teleport/src/lib/util.ts @@ -16,6 +16,15 @@ limitations under the License. import { AuthType } from 'teleport/services/user'; +// TODO(ravicious): Refactor teleport.e and teleterm.e to import pluralize from shared/utils/text +// and remove this temporary reexport. +export { + /** + * @deprecated Import pluralize from `shared/utils/text` instead. + */ + pluralize, +} from 'shared/utils/text'; + export const openNewTab = (url: string) => { const element = document.createElement('a'); element.setAttribute('href', `${url}`); @@ -27,14 +36,6 @@ export const openNewTab = (url: string) => { document.body.removeChild(element); }; -export function pluralize(num: number, word: string) { - if (num > 1) { - return `${word}s`; - } - - return word; -} - // Adapted from https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest#converting_a_digest_to_a_hex_string export async function Sha256Digest( message: string, diff --git a/web/packages/teleterm/src/ui/Search/searchResultTestHelpers.ts b/web/packages/teleterm/src/services/tshd/testHelpers.ts similarity index 67% rename from web/packages/teleterm/src/ui/Search/searchResultTestHelpers.ts rename to web/packages/teleterm/src/services/tshd/testHelpers.ts index af48315d466d1..051d6a8f38fe6 100644 --- a/web/packages/teleterm/src/ui/Search/searchResultTestHelpers.ts +++ b/web/packages/teleterm/src/services/tshd/testHelpers.ts @@ -14,11 +14,9 @@ * limitations under the License. */ -import { ResourceSearchResult } from './searchResult'; +import type * as tsh from './types'; -import type * as tsh from 'teleterm/services/tshd/types'; - -export const makeServer = (props: Partial): tsh.Server => ({ +export const makeServer = (props: Partial = {}): tsh.Server => ({ uri: '/clusters/teleport-local/servers/178ef081-259b-4aa5-a018-449b5ea7e694', tunnel: false, name: '178ef081-259b-4aa5-a018-449b5ea7e694', @@ -28,7 +26,9 @@ export const makeServer = (props: Partial): tsh.Server => ({ ...props, }); -export const makeDatabase = (props: Partial): tsh.Database => ({ +export const makeDatabase = ( + props: Partial = {} +): tsh.Database => ({ uri: '/clusters/teleport-local/dbs/foo', name: 'foo', protocol: 'postgres', @@ -40,7 +40,7 @@ export const makeDatabase = (props: Partial): tsh.Database => ({ ...props, }); -export const makeKube = (props: Partial): tsh.Kube => ({ +export const makeKube = (props: Partial = {}): tsh.Kube => ({ name: 'foo', labelsList: [], uri: '/clusters/bar/kubes/foo', @@ -49,15 +49,3 @@ export const makeKube = (props: Partial): tsh.Kube => ({ export const makeLabelsList = (labels: Record): tsh.Label[] => Object.entries(labels).map(([name, value]) => ({ name, value })); - -export const makeResourceResult = ( - props: Partial & { - kind: ResourceSearchResult['kind']; - resource: ResourceSearchResult['resource']; - } -): ResourceSearchResult => ({ - score: 0, - labelMatches: [], - resourceMatches: [], - ...props, -}); diff --git a/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx b/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx index dd420c49cf784..1cebd130e3c50 100644 --- a/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx +++ b/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx @@ -23,6 +23,8 @@ import { DocumentsReopen } from 'teleterm/ui/DocumentsReopen'; import { Dialog } from 'teleterm/ui/services/modals'; import ClusterLogout from '../ClusterLogout/ClusterLogout'; +import { ResourceSearchErrors } from '../Search/ResourceSearchErrors'; +import { assertUnreachable } from '../utils'; import { UsageData } from './modals/UsageData'; import { UserJobRole } from './modals/UserJobRole'; @@ -117,8 +119,25 @@ function renderDialog(dialog: Dialog, handleClose: () => void) { ); } - default: { + case 'resource-search-errors': { + return ( + { + handleClose(); + dialog.onCancel(); + }} + /> + ); + } + + case 'none': { return null; } + + default: { + return assertUnreachable(dialog); + } } } diff --git a/web/packages/teleterm/src/ui/Search/ResourceSearchErrors.story.tsx b/web/packages/teleterm/src/ui/Search/ResourceSearchErrors.story.tsx new file mode 100644 index 0000000000000..8b20cc3857901 --- /dev/null +++ b/web/packages/teleterm/src/ui/Search/ResourceSearchErrors.story.tsx @@ -0,0 +1,77 @@ +/** + * 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 React from 'react'; + +import { routing } from 'teleterm/ui/uri'; +import { ResourceSearchError } from 'teleterm/ui/services/resources'; + +import { ResourceSearchErrors } from './ResourceSearchErrors'; + +export default { + title: 'Teleterm/ModalsHost/ResourceSearchErrors', +}; + +export const Story = () => ( + {}} + errors={[ + new ResourceSearchError( + '/clusters/foo', + 'server', + new Error( + '14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"' + ) + ), + new ResourceSearchError( + '/clusters/bar', + 'database', + new Error( + '2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host' + ) + ), + new ResourceSearchError( + '/clusters/baz', + 'kube', + new Error( + '14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"' + ) + ), + new ResourceSearchError( + '/clusters/foo', + 'server', + new Error( + '2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host' + ) + ), + new ResourceSearchError( + '/clusters/baz', + 'kube', + new Error( + '14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"' + ) + ), + new ResourceSearchError( + '/clusters/foo', + 'server', + new Error( + '2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host' + ) + ), + ]} + /> +); diff --git a/web/packages/teleterm/src/ui/Search/ResourceSearchErrors.tsx b/web/packages/teleterm/src/ui/Search/ResourceSearchErrors.tsx new file mode 100644 index 0000000000000..1d59fa62d9291 --- /dev/null +++ b/web/packages/teleterm/src/ui/Search/ResourceSearchErrors.tsx @@ -0,0 +1,84 @@ +/** + * 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 React from 'react'; +import DialogConfirmation, { + DialogContent, + DialogFooter, + DialogHeader, +} from 'design/DialogConfirmation'; +import { ButtonIcon, ButtonSecondary, Text } from 'design'; +import { Close } from 'design/Icon'; + +import { ResourceSearchError } from 'teleterm/ui/services/resources'; + +import type * as uri from 'teleterm/ui/uri'; + +export function ResourceSearchErrors(props: { + errors: ResourceSearchError[]; + getClusterName: (resourceUri: uri.ClusterOrResourceUri) => string; + onCancel: () => void; +}) { + const formattedErrorText = props.errors + .map(error => error.messageAndCauseWithClusterName(props.getClusterName)) + .join('\n\n'); + + return ( + ({ + maxWidth: '800px', + width: '100%', + })} + > + + + Resource search errors + + + + + + + + 
 props.theme.space[2]}px;
+              background-color: ${props => props.theme.colors.levels.sunken};
+              color: ${props => props.theme.colors.text.primary};
+
+              white-space: pre-wrap;
+              max-height: calc(${props => props.theme.space[6]}px * 10);
+              overflow-y: auto;
+            `}
+          >
+            {formattedErrorText}
+
+ + + + + Close + + + + ); +} diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx index 80d1f7e17ee30..b71170771c982 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx @@ -15,48 +15,49 @@ */ import React from 'react'; -import { render, screen } from 'design/utils/testing'; +import { render, screen, waitFor } from 'design/utils/testing'; import { makeSuccessAttempt } from 'shared/hooks/useAsync'; import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; +import { ResourceSearchError } from 'teleterm/ui/services/resources'; +import ModalsHost from 'teleterm/ui/ModalsHost'; import * as pickers from './pickers/pickers'; -import * as useSearchAttempts from './pickers/useSearchAttempts'; +import * as useActionAttempts from './pickers/useActionAttempts'; import * as SearchContext from './SearchContext'; import { SearchBarConnected } from './SearchBar'; +beforeEach(() => { + jest.restoreAllMocks(); +}); + it('does not display empty results copy after selecting two filters', () => { const appContext = new MockAppContext(); appContext.workspacesService.setState(draft => { draft.rootClusterUri = '/clusters/foo'; }); - const mockAttempts = { + const mockActionAttempts = { filterActionsAttempt: makeSuccessAttempt([]), resourceActionsAttempt: makeSuccessAttempt([]), + resourceSearchAttempt: makeSuccessAttempt({ + results: [], + errors: [], + search: '', + }), }; jest - .spyOn(useSearchAttempts, 'useSearchAttempts') - .mockImplementation(() => mockAttempts); + .spyOn(useActionAttempts, 'useActionAttempts') + .mockImplementation(() => mockActionAttempts); jest.spyOn(SearchContext, 'useSearchContext').mockImplementation(() => ({ + ...getMockedSearchContext(), filters: [ { filter: 'cluster', clusterUri: '/clusters/foo' }, { filter: 'resource-type', resourceType: 'servers' }, ], inputValue: '', - setFilter: () => {}, - removeFilter: () => {}, - opened: true, - open: () => {}, - close: () => {}, - closeAndResetInput: () => {}, - resetInput: () => {}, - changeActivePicker: () => {}, - onInputValueChange: () => {}, - activePicker: pickers.actionPicker, - inputRef: undefined, })); render( @@ -75,16 +76,21 @@ it('does display empty results copy after providing search query for which there draft.rootClusterUri = '/clusters/foo'; }); - const mockAttempts = { + const mockActionAttempts = { filterActionsAttempt: makeSuccessAttempt([]), resourceActionsAttempt: makeSuccessAttempt([]), + resourceSearchAttempt: makeSuccessAttempt({ + results: [], + errors: [], + search: '', + }), }; jest - .spyOn(useSearchAttempts, 'useSearchAttempts') - .mockImplementation(() => mockAttempts); + .spyOn(useActionAttempts, 'useActionAttempts') + .mockImplementation(() => mockActionAttempts); jest .spyOn(SearchContext, 'useSearchContext') - .mockImplementation(() => mockedSearchContext); + .mockImplementation(getMockedSearchContext); render( @@ -114,16 +120,21 @@ it('does display empty results copy and excluded clusters after providing search draft.rootClusterUri = '/clusters/foo'; }); - const mockAttempts = { + const mockActionAttempts = { filterActionsAttempt: makeSuccessAttempt([]), resourceActionsAttempt: makeSuccessAttempt([]), + resourceSearchAttempt: makeSuccessAttempt({ + results: [], + errors: [], + search: '', + }), }; jest - .spyOn(useSearchAttempts, 'useSearchAttempts') - .mockImplementation(() => mockAttempts); + .spyOn(useActionAttempts, 'useActionAttempts') + .mockImplementation(() => mockActionAttempts); jest .spyOn(SearchContext, 'useSearchContext') - .mockImplementation(() => mockedSearchContext); + .mockImplementation(getMockedSearchContext); render( @@ -138,13 +149,129 @@ it('does display empty results copy and excluded clusters after providing search ); }); -const mockedSearchContext = { +it('notifies about resource search errors and allows to display details', () => { + const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.rootClusterUri = '/clusters/foo'; + }); + + const resourceSearchError = new ResourceSearchError( + '/clusters/foo', + 'server', + new Error('whoops') + ); + + const mockActionAttempts = { + filterActionsAttempt: makeSuccessAttempt([]), + resourceActionsAttempt: makeSuccessAttempt([]), + resourceSearchAttempt: makeSuccessAttempt({ + results: [], + errors: [resourceSearchError], + search: '', + }), + }; + jest + .spyOn(useActionAttempts, 'useActionAttempts') + .mockImplementation(() => mockActionAttempts); + const mockedSearchContext = { + ...getMockedSearchContext(), + inputValue: 'foo', + }; + jest + .spyOn(SearchContext, 'useSearchContext') + .mockImplementation(() => mockedSearchContext); + jest.spyOn(appContext.modalsService, 'openRegularDialog'); + jest.spyOn(mockedSearchContext, 'lockOpen'); + + render( + + + + ); + + const results = screen.getByRole('menu'); + expect(results).toHaveTextContent( + 'Some of the search results are incomplete.' + ); + expect(results).toHaveTextContent('Could not fetch servers from foo'); + expect(results).not.toHaveTextContent(resourceSearchError.cause['message']); + + screen.getByText('Show details').click(); + + expect(appContext.modalsService.openRegularDialog).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'resource-search-errors', + errors: [resourceSearchError], + }) + ); + expect(mockedSearchContext.lockOpen).toHaveBeenCalled(); +}); + +it('maintains focus on the search input after closing a resource search error modal', async () => { + const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.rootClusterUri = '/clusters/foo'; + }); + + const resourceSearchError = new ResourceSearchError( + '/clusters/foo', + 'server', + new Error('whoops') + ); + + const mockActionAttempts = { + filterActionsAttempt: makeSuccessAttempt([]), + resourceActionsAttempt: makeSuccessAttempt([]), + resourceSearchAttempt: makeSuccessAttempt({ + results: [], + errors: [resourceSearchError], + search: '', + }), + }; + jest + .spyOn(useActionAttempts, 'useActionAttempts') + .mockImplementation(() => mockActionAttempts); + + render( + + + + + ); + + screen.getByRole('searchbox').focus(); + expect(screen.getByRole('menu')).toHaveTextContent( + 'Some of the search results are incomplete.' + ); + screen.getByText('Show details').click(); + + const modal = screen.getByTestId('Modal'); + expect(modal).toHaveTextContent('Resource search errors'); + expect(modal).toHaveTextContent('whoops'); + + // Lose focus on the search input. + screen.getByText('Close').focus(); + screen.getByText('Close').click(); + + // Need to await this since some state updates in SearchContext are done after the modal closes. + // Otherwise we'd get a warning about missing `act`. + await waitFor(() => { + expect(modal).not.toBeInTheDocument(); + }); + + expect(screen.getByRole('searchbox')).toHaveFocus(); + // Verify that the search bar wasn't closed. + expect(screen.getByRole('menu')).toBeInTheDocument(); +}); + +const getMockedSearchContext = () => ({ inputValue: 'foo', filters: [], setFilter: () => {}, removeFilter: () => {}, - opened: true, + isOpen: true, open: () => {}, + lockOpen: async () => {}, close: () => {}, closeAndResetInput: () => {}, resetInput: () => {}, @@ -152,4 +279,4 @@ const mockedSearchContext = { onInputValueChange: () => {}, activePicker: pickers.actionPicker, inputRef: undefined, -}; +}); diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.tsx index 6fb2d8b88fce8..6c1ae5a51df65 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.tsx @@ -55,7 +55,7 @@ function SearchBar() { inputValue, onInputValueChange, inputRef, - opened, + isOpen, open, close, } = useSearchContext(); @@ -74,11 +74,11 @@ function SearchBar() { close(); } }; - if (opened) { + if (isOpen) { window.addEventListener('click', onClickOutside); return () => window.removeEventListener('click', onClickOutside); } - }, [close, opened]); + }, [close, isOpen]); function handleOnFocus(e: React.FocusEvent) { open(e.relatedTarget); @@ -117,13 +117,13 @@ function SearchBar() { ref={containerRef} onFocus={handleOnFocus} > - {!opened && ( + {!isOpen && ( <> {getAccelerator(OPEN_SEARCH_BAR_SHORTCUT_ACTION)} )} - {opened && ( + {isOpen && ( } diff --git a/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx b/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx new file mode 100644 index 0000000000000..1d610ec2062a4 --- /dev/null +++ b/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx @@ -0,0 +1,124 @@ +/** + * 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 React from 'react'; +import '@testing-library/jest-dom'; +import { render, screen } from '@testing-library/react'; +import { renderHook, act } from '@testing-library/react-hooks'; + +import { SearchContextProvider, useSearchContext } from './SearchContext'; + +describe('lockOpen', () => { + let resolveSuccessAction, rejectFailureAction; + const successAction = new Promise(resolve => { + resolveSuccessAction = resolve; + }); + const failureAction = new Promise((resolve, reject) => { + rejectFailureAction = reject; + }); + + test.each([ + { + name: 'prevents the search bar from being closed for the duration of the action', + action: successAction, + finishAction: resolveSuccessAction, + }, + { + name: 'properly cleans up the ref even after the action fails', + action: failureAction, + finishAction: rejectFailureAction, + }, + ])('$name', async ({ action, finishAction }) => { + const inputFocus = jest.fn(); + const { result } = renderHook(() => useSearchContext(), { + wrapper: ({ children }) => ( + {children} + ), + }); + result.current.inputRef.current = { + focus: inputFocus, + } as unknown as HTMLInputElement; + + act(() => { + result.current.open(); + }); + + let lockOpenPromise: Promise; + act(() => { + lockOpenPromise = result.current.lockOpen(action); + }); + + // Closing while the search bar is locked open should be a noop. + act(() => { + result.current.close(); + }); + expect(result.current.isOpen).toBe(true); + + await act(async () => { + finishAction(); + try { + await lockOpenPromise; + } catch { + // Ignore the error happening when `finishAction` rejects `action`. + } + }); + + // The search bar should be no longer locked open, so close should behave as expected. + act(() => { + result.current.close(); + }); + expect(result.current.isOpen).toBe(false); + + // Called the first time during `open`, then again after `lockOpen` finishes. + expect(inputFocus).toHaveBeenCalledTimes(2); + }); +}); + +describe('open', () => { + it('manages the focus properly when called with no arguments', () => { + const SearchInput = () => { + const { inputRef, open, close } = useSearchContext(); + + return ( + <> + +