From d7b4ccf9b187b5177dfa61497ff99b7a1a912e53 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 18 Nov 2024 13:42:48 +0100 Subject: [PATCH 01/10] Pass `rootClusterUri` to functions in `ResourcesContext` --- .../DocumentConnectMyComputer/Setup.tsx | 8 ++-- .../connectMyComputerContext.tsx | 2 +- .../DocumentAccessRequests/useAssumeAccess.ts | 8 ++-- .../DocumentCluster/resourcesContext.test.tsx | 7 ++- .../ui/DocumentCluster/resourcesContext.tsx | 46 +++++++++++-------- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx index 5cc895509d37d..bbcc5dc0cd88f 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx @@ -279,10 +279,10 @@ function AgentSetup() { throw error; } - // Now that the node has joined the server, let's refresh all open DocumentCluster instances - // to show the new node. - requestResourcesRefresh(); - }, [startAgent, requestResourcesRefresh]) + // Now that the node has joined the server, let's refresh open DocumentCluster + // instances in the workspace to show the new node. + requestResourcesRefresh(rootClusterUri); + }, [startAgent, requestResourcesRefresh, rootClusterUri]) ); const steps: SetupStep[] = [ diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx index 723950eb47549..99b1584475281 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -322,7 +322,7 @@ export const ConnectMyComputerContextProvider: FC< } if (hasNodeRemovalSucceeded) { - requestResourcesRefresh(); + requestResourcesRefresh(rootClusterUri); } ctx.notificationsService.notifyInfo( diff --git a/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts b/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts index 741d5337e3405..eea143cf41d17 100644 --- a/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts +++ b/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts @@ -35,8 +35,8 @@ export function useAssumeAccess() { const [assumeRoleAttempt, runAssumeRole] = useAsync((requestId: string) => retryWithRelogin(ctx, clusterUri, async () => { await ctx.clustersService.assumeRoles(rootClusterUri, [requestId]); - // refresh the current resource tabs - requestResourcesRefresh(); + // Refresh the current resource tabs in the workspace. + requestResourcesRefresh(rootClusterUri); }) ); @@ -58,8 +58,8 @@ export function useAssumeAccess() { return; } - // refresh the current resource tabs - requestResourcesRefresh(); + // Refresh the current resource tabs in the workspace. + requestResourcesRefresh(rootClusterUri); // open new cluster tab const clusterDocument = documentsService.createClusterDocument({ diff --git a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx index 0430156b5c6e9..3427619e6aa64 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx @@ -19,6 +19,8 @@ import React from 'react'; import { renderHook } from '@testing-library/react'; +import { rootClusterUri } from 'teleterm/services/tshd/testHelpers'; + import { ResourcesContextProvider, useResourcesContext, @@ -33,9 +35,10 @@ describe('requestResourcesRefresh', () => { const listener = jest.fn(); result.current.onResourcesRefreshRequest(listener); - result.current.requestResourcesRefresh(); + result.current.requestResourcesRefresh(rootClusterUri); expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(rootClusterUri); }); }); @@ -50,7 +53,7 @@ describe('onResourcesRefreshRequest cleanup function', () => { const { cleanup } = result.current.onResourcesRefreshRequest(listener); cleanup(); - result.current.requestResourcesRefresh(); + result.current.requestResourcesRefresh(rootClusterUri); expect(listener).not.toHaveBeenCalled(); }); diff --git a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx index f7961f9dfce0f..151bef27b4153 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx @@ -18,7 +18,7 @@ import { EventEmitter } from 'events'; -import React, { +import { createContext, FC, PropsWithChildren, @@ -27,20 +27,26 @@ import React, { useRef, } from 'react'; +import { RootClusterUri } from 'teleterm/ui/uri'; + export interface ResourcesContext { /** - * requestResourcesRefresh makes all DocumentCluster instances within the workspace refresh the - * resource list with current filters. + * requestResourcesRefresh makes all DocumentCluster instances within the workspace + * (specified by `rootClusterUri`) refresh the resource list with current filters. * - * Its main purpose is to refresh the resource list in existing DocumentCluster tabs after a - * Connect My Computer node is set up. + * Its purpose is to refresh the resource list in existing DocumentCluster tabs after a + * Connect My Computer node is set up, or after assuming/dropping an access request. */ - requestResourcesRefresh: () => void; + requestResourcesRefresh: (rootClusterUri: RootClusterUri) => void; /** * onResourcesRefreshRequest registers a listener that will be called any time a refresh is * requested. Typically called from useEffect, for this purpose it returns a cleanup function. */ - onResourcesRefreshRequest: (listener: () => void) => { cleanup: () => void }; + onResourcesRefreshRequest: ( + listener: (rootClusterUri: RootClusterUri) => void + ) => { + cleanup: () => void; + }; } const ResourcesContext = createContext(null); @@ -51,24 +57,24 @@ export const ResourcesContextProvider: FC = props => { emitterRef.current = new EventEmitter(); } - // This function could be expanded to emit a cluster URI so that a request refresh for a root - // cluster doesn't trigger refreshes of leaf DocumentCluster instances and vice versa. - // However, the implementation should be good enough for now since it's used only in Connect My - // Computer setup anyway. const requestResourcesRefresh = useCallback( - () => emitterRef.current.emit('refresh'), + (rootClusterUri: RootClusterUri) => + emitterRef.current.emit('refresh', rootClusterUri), [] ); - const onResourcesRefreshRequest = useCallback(listener => { - emitterRef.current.addListener('refresh', listener); + const onResourcesRefreshRequest = useCallback( + (listener: (rootClusterUri: RootClusterUri) => void) => { + emitterRef.current.addListener('refresh', listener); - return { - cleanup: () => { - emitterRef.current.removeListener('refresh', listener); - }, - }; - }, []); + return { + cleanup: () => { + emitterRef.current.removeListener('refresh', listener); + }, + }; + }, + [] + ); return ( Date: Mon, 18 Nov 2024 13:43:34 +0100 Subject: [PATCH 02/10] Support refreshing resources per cluster URI in unified resources --- .../DocumentCluster/UnifiedResources.test.tsx | 158 +++++++++++++++++- .../ui/DocumentCluster/UnifiedResources.tsx | 10 +- 2 files changed, 159 insertions(+), 9 deletions(-) diff --git a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx index eb537ce8335e5..28ff780357491 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +import { useImperativeHandle, forwardRef, createRef } from 'react'; import { render, screen } from 'design/utils/testing'; import { mockIntersectionObserver } from 'jsdom-testing-mocks'; import { act } from '@testing-library/react'; @@ -30,7 +31,10 @@ import { ShowResources } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb' import { UnifiedResources } from 'teleterm/ui/DocumentCluster/UnifiedResources'; import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; -import { ResourcesContextProvider } from 'teleterm/ui/DocumentCluster/resourcesContext'; +import { + ResourcesContextProvider, + useResourcesContext, +} from 'teleterm/ui/DocumentCluster/resourcesContext'; import { ConnectMyComputerContextProvider } from 'teleterm/ui/ConnectMyComputer'; import { MockWorkspaceContextProvider } from 'teleterm/ui/fixtures/MockWorkspaceContextProvider'; import { makeDocumentCluster } from 'teleterm/ui/services/workspacesService/documentsService/testHelpers'; @@ -38,14 +42,16 @@ import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; import { makeRootCluster, rootClusterUri, + makeServer, } from 'teleterm/services/tshd/testHelpers'; import { getEmptyPendingAccessRequest } from 'teleterm/ui/services/workspacesService/accessRequestsService'; - import { MockedUnaryCall } from 'teleterm/services/tshd/cloneableClient'; +import { DocumentClusterQueryParams } from 'teleterm/ui/services/workspacesService'; +import * as uri from 'teleterm/ui/uri'; const mio = mockIntersectionObserver(); -const tests = [ +test.each([ { name: 'fetches only available resources if cluster does not support access requests', conditions: { @@ -166,9 +172,7 @@ const tests = [ includeRequestable: false, }, }, -]; - -test.each(tests)('$name', async testCase => { +])('$name', async testCase => { const doc = makeDocumentCluster(); const appContext = new MockAppContext({ platform: 'darwin' }); @@ -270,3 +274,145 @@ test.each(tests)('$name', async testCase => { new AbortController().signal ); }); + +test.each([ + { + name: 'refreshes resources when the document cluster URI matches the requested cluster URI', + conditions: { + documentClusterUri: '/clusters/teleport-local', + rootClusterUriToRefresh: '/clusters/teleport-local', + }, + expect: { + resourcesRefreshed: true, + }, + }, + { + name: 'refreshes resources when the document cluster URI is a leaf of the requested cluster URI', + conditions: { + documentClusterUri: '/clusters/teleport-local/leaves/leaf', + rootClusterUriToRefresh: '/clusters/teleport-local', + }, + expect: { + resourcesRefreshed: true, + }, + }, + + { + name: 'does not refresh resources when the document cluster URI is not related to the requested cluster URI', + conditions: { + documentClusterUri: '/clusters/teleport-remote', + rootClusterUriToRefresh: '/clusters/teleport-local', + }, + expect: { + resourcesRefreshed: false, + }, + }, +])('$name', async testCase => { + const doc = makeDocumentCluster({ + clusterUri: testCase.conditions.documentClusterUri, + }); + const rootCluster = makeRootCluster({ + uri: uri.routing.ensureRootClusterUri(doc.clusterUri), + }); + const serverResource = makeServer(); + const appContext = new MockAppContext(); + appContext.clustersService.setState(draft => { + draft.clusters.set(rootCluster.uri, rootCluster); + }); + + appContext.workspacesService.setState(draftState => { + draftState.rootClusterUri = rootCluster.uri; + draftState.workspaces[rootCluster.uri] = { + localClusterUri: rootCluster.uri, + documents: [doc], + location: doc.uri, + accessRequests: { + pending: getEmptyPendingAccessRequest(), + isBarCollapsed: true, + }, + }; + }); + + jest + .spyOn(appContext.resourcesService, 'listUnifiedResources') + .mockResolvedValue({ + resources: [ + { + kind: 'server', + resource: serverResource, + requiresRequest: false, + }, + ], + nextKey: '', + }); + + const ref = createRef<{ + requestResourcesRefresh: (rootClusterUri: uri.RootClusterUri) => void; + }>(); + + render( + + + + + + + + + + ); + + act(mio.enterAll); + + // Wait for resources to render. + await expect( + screen.findByText(serverResource.hostname) + ).resolves.toBeInTheDocument(); + expect( + appContext.resourcesService.listUnifiedResources + ).toHaveBeenCalledTimes(1); + + act(() => + ref.current.requestResourcesRefresh( + testCase.conditions.rootClusterUriToRefresh + ) + ); + + // Wait for resources to (potentially) re-render. + await expect( + screen.findByText(serverResource.hostname) + ).resolves.toBeInTheDocument(); + expect( + appContext.resourcesService.listUnifiedResources + // When resources are refreshed, we have two calls to the API. + ).toHaveBeenCalledTimes(testCase.expect.resourcesRefreshed ? 2 : 1); +}); + +const UnifiedResourcesWithRefreshExposed = forwardRef< + { + requestResourcesRefresh: (rootClusterUri: uri.RootClusterUri) => void; + }, + { + clusterUri: uri.ClusterUri; + docUri: uri.DocumentUri; + queryParams: DocumentClusterQueryParams; + } +>((props, ref) => { + const resourcesContext = useResourcesContext(); + useImperativeHandle(ref, () => ({ + requestResourcesRefresh: resourcesContext.requestResourcesRefresh, + })); + + return ( + + ); +}); diff --git a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx index 1d2f6e7613701..599bc01828f7d 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx @@ -328,11 +328,15 @@ const Resources = memo( const { onResourcesRefreshRequest } = props; useEffect(() => { - const { cleanup } = onResourcesRefreshRequest(() => { - fetch({ clear: true }); + const { cleanup } = onResourcesRefreshRequest(rootClusterUri => { + // Refresh root and leaf cluster documents. + if (!uri.routing.belongsToProfile(rootClusterUri, props.clusterUri)) { + return; + } + void fetch({ clear: true }); }); return cleanup; - }, [onResourcesRefreshRequest, fetch]); + }, [onResourcesRefreshRequest, fetch, props.clusterUri]); const { getAccessRequestButton } = props; // The action callback in the requestAccess action has access to From 845a1719c71f59cc21e89186ff96e7dbd33e0e69 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 18 Nov 2024 13:44:37 +0100 Subject: [PATCH 03/10] Move `ResourcesContextProvider` higher in component hierarchy --- web/packages/teleterm/src/ui/App.tsx | 17 +++++---- .../src/ui/Documents/DocumentsRenderer.tsx | 35 ++++++++----------- .../teleterm/src/ui/TabHost/TabHost.test.tsx | 5 ++- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/web/packages/teleterm/src/ui/App.tsx b/web/packages/teleterm/src/ui/App.tsx index 9236ac9222e11..204a3e467ce04 100644 --- a/web/packages/teleterm/src/ui/App.tsx +++ b/web/packages/teleterm/src/ui/App.tsx @@ -29,6 +29,7 @@ import AppContext from './appContext'; import { ThemeProvider } from './ThemeProvider'; import { VnetContextProvider } from './Vnet/vnetContext'; import { ConnectionsContextProvider } from './TopBar/Connections/connectionsContext'; +import { ResourcesContextProvider } from './DocumentCluster/resourcesContext'; export const App: React.FC<{ ctx: AppContext }> = ({ ctx }) => { return ( @@ -36,13 +37,15 @@ export const App: React.FC<{ ctx: AppContext }> = ({ ctx }) => { - - - - - - - + + + + + + + + + diff --git a/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx b/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx index f3cda92bd922f..0be864d2748f6 100644 --- a/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx +++ b/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx @@ -45,8 +45,6 @@ import { DocumentGatewayApp } from 'teleterm/ui/DocumentGatewayApp'; import Document from 'teleterm/ui/Document'; import { RootClusterUri, isDatabaseUri, isAppUri } from 'teleterm/ui/uri'; -import { ResourcesContextProvider } from '../DocumentCluster/resourcesContext'; - import { WorkspaceContextProvider } from './workspaceContext'; import { KeyboardShortcutsPanel } from './KeyboardShortcutsPanel'; @@ -87,25 +85,22 @@ export function DocumentsRenderer(props: { key={workspace.rootClusterUri} > - {/* ConnectMyComputerContext depends on ResourcesContext. */} - - - {workspace.documentsService.getDocuments().length ? ( - renderDocuments(workspace.documentsService) - ) : ( - + + {workspace.documentsService.getDocuments().length ? ( + renderDocuments(workspace.documentsService) + ) : ( + + )} + {workspace.rootClusterUri === + workspacesService.getRootClusterUri() && + props.topBarContainerRef.current && + createPortal( + , + props.topBarContainerRef.current )} - {workspace.rootClusterUri === - workspacesService.getRootClusterUri() && - props.topBarContainerRef.current && - createPortal( - , - props.topBarContainerRef.current - )} - - + ))} diff --git a/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx b/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx index 7410532aff073..55b3cfc9abd35 100644 --- a/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx +++ b/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx @@ -31,6 +31,7 @@ import { rootClusterUri, } from 'teleterm/services/tshd/testHelpers'; import { routing } from 'teleterm/ui/uri'; +import { ResourcesContextProvider } from 'teleterm/ui/DocumentCluster/resourcesContext'; function getMockDocuments(): Document[] { return [ @@ -86,7 +87,9 @@ async function getTestSetup({ documents }: { documents: Document[] }) { render( - + + + ); From b3c295a72ff58656bb6ff93e0f60159f44f1a533 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 18 Nov 2024 13:46:56 +0100 Subject: [PATCH 04/10] Refresh resources after dropping request --- .../useAssumedRolesBar.test.tsx | 86 +++++++++++++++++++ .../useAssumedRolesBar.ts | 21 +++-- 2 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx new file mode 100644 index 0000000000000..48c5c7b309257 --- /dev/null +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx @@ -0,0 +1,86 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { useEffect } from 'react'; +import { renderHook, act } from '@testing-library/react'; + +import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; +import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; +import { makeRootCluster } from 'teleterm/services/tshd/testHelpers'; +import { + ResourcesContextProvider, + useResourcesContext, +} from 'teleterm/ui/DocumentCluster/resourcesContext'; +import { RootClusterUri } from 'teleterm/ui/uri'; + +import { useAssumedRolesBar } from './useAssumedRolesBar'; + +test('dropping a request refreshes resources', async () => { + const appContext = new MockAppContext(); + const cluster = makeRootCluster(); + appContext.workspacesService.setState(draftState => { + draftState.rootClusterUri = cluster.uri; + draftState.workspaces[cluster.uri] = { + localClusterUri: cluster.uri, + documents: [], + location: undefined, + accessRequests: undefined, + }; + }); + jest.spyOn(appContext.clustersService, 'dropRoles'); + const refreshListener = jest.fn(); + + const wrapper = ({ children }) => ( + + + + {children} + + + ); + + const { result } = renderHook( + () => + useAssumedRolesBar({ + roles: [], + id: 'mocked-request-id', + expires: new Date(), + }), + { wrapper } + ); + + await act(() => result.current.dropRequest()); + expect(appContext.clustersService.dropRoles).toHaveBeenCalledTimes(1); + expect(appContext.clustersService.dropRoles).toHaveBeenCalledWith( + cluster.uri, + ['mocked-request-id'] + ); + expect(refreshListener).toHaveBeenCalledTimes(1); + expect(refreshListener).toHaveBeenCalledWith(cluster.uri); +}); + +function RequestRefreshSubscriber(props: { + onResourcesRefreshRequest: (rootClusterUri: RootClusterUri) => void; +}) { + const { onResourcesRefreshRequest } = useResourcesContext(); + useEffect(() => { + onResourcesRefreshRequest(props.onResourcesRefreshRequest); + }, [onResourcesRefreshRequest, props.onResourcesRefreshRequest]); + + return null; +} diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts index 3bce0793dc8ea..a37fb3bd9b7aa 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts @@ -32,10 +32,12 @@ import { useInterval } from 'shared/hooks'; import { useAppContext } from 'teleterm/ui/appContextProvider'; import { retryWithRelogin } from 'teleterm/ui/utils'; import { AssumedRequest } from 'teleterm/services/tshd/types'; +import { useResourcesContext } from 'teleterm/ui/DocumentCluster/resourcesContext'; export function useAssumedRolesBar(assumedRequest: AssumedRequest) { const ctx = useAppContext(); const rootClusterUri = ctx.workspacesService?.getRootClusterUri(); + const { requestResourcesRefresh } = useResourcesContext(); const [duration, setDuration] = useState(() => getDurationFromNow({ @@ -46,21 +48,18 @@ export function useAssumedRolesBar(assumedRequest: AssumedRequest) { getRefreshInterval(duration) ); - const [dropRequestAttempt, dropRequest] = useAsync(() => { - return retryWithRelogin( - ctx, - rootClusterUri, - () => ctx.clustersService.dropRoles(rootClusterUri, [assumedRequest.id]) - // TODO(gzdunek): We should refresh the resources, - // the same as after assuming a role in `useAssumeAccess`. - // Unfortunately, we can't do this because we don't have access to `ResourcesContext`. - // Consider moving it into `ResourcesService`. - ).catch(err => { + const [dropRequestAttempt, dropRequest] = useAsync(async () => { + try { + await retryWithRelogin(ctx, rootClusterUri, () => + ctx.clustersService.dropRoles(rootClusterUri, [assumedRequest.id]) + ); + requestResourcesRefresh(rootClusterUri); + } catch (err) { ctx.notificationsService.notifyError({ title: 'Could not switch back the role', description: err.message, }); - }); + } }); const updateDurationAndInterval = useCallback(() => { From 7b26448da3df4498a250b1b01be53f8641b5090b Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 18 Nov 2024 14:02:09 +0100 Subject: [PATCH 05/10] Fix casing of Assume Roles button --- .../src/ui/DocumentAccessRequests/RequestList/RequestList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/DocumentAccessRequests/RequestList/RequestList.tsx b/web/packages/teleterm/src/ui/DocumentAccessRequests/RequestList/RequestList.tsx index 40607a7ee17be..f2c52fb16ff82 100644 --- a/web/packages/teleterm/src/ui/DocumentAccessRequests/RequestList/RequestList.tsx +++ b/web/packages/teleterm/src/ui/DocumentAccessRequests/RequestList/RequestList.tsx @@ -167,7 +167,7 @@ const renderActionCell = ( onClick={() => assumeRole(request)} width="108px" > - {flags.isAssumed ? 'assumed' : 'assume roles'} + {flags.isAssumed ? 'Assumed' : 'Assume Roles'} ); } else { From dfc74894b1b8e6d13c05d7cf97429e6470829042 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 19 Nov 2024 17:26:38 +0100 Subject: [PATCH 06/10] Add `rootClusterUri` as an argument to `useResourcesContext` --- .../useAssumedRolesBar.test.tsx | 13 ++++-- .../useAssumedRolesBar.ts | 4 +- .../connectMyComputerContext.tsx | 4 +- .../DocumentAccessRequests/useAssumeAccess.ts | 6 +-- .../DocumentCluster/UnifiedResources.test.tsx | 45 ++++--------------- .../ui/DocumentCluster/UnifiedResources.tsx | 12 ++--- .../DocumentCluster/resourcesContext.test.tsx | 32 +++++++++---- .../ui/DocumentCluster/resourcesContext.tsx | 23 +++++++++- 8 files changed, 74 insertions(+), 65 deletions(-) diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx index 48c5c7b309257..9bcef0bc06467 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.test.tsx @@ -48,7 +48,10 @@ test('dropping a request refreshes resources', async () => { const wrapper = ({ children }) => ( - + {children} @@ -71,13 +74,15 @@ test('dropping a request refreshes resources', async () => { ['mocked-request-id'] ); expect(refreshListener).toHaveBeenCalledTimes(1); - expect(refreshListener).toHaveBeenCalledWith(cluster.uri); }); function RequestRefreshSubscriber(props: { - onResourcesRefreshRequest: (rootClusterUri: RootClusterUri) => void; + rootClusterUri: RootClusterUri; + onResourcesRefreshRequest: () => void; }) { - const { onResourcesRefreshRequest } = useResourcesContext(); + const { onResourcesRefreshRequest } = useResourcesContext( + props.rootClusterUri + ); useEffect(() => { onResourcesRefreshRequest(props.onResourcesRefreshRequest); }, [onResourcesRefreshRequest, props.onResourcesRefreshRequest]); diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts index a37fb3bd9b7aa..d71b092bd5f81 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts @@ -37,7 +37,7 @@ import { useResourcesContext } from 'teleterm/ui/DocumentCluster/resourcesContex export function useAssumedRolesBar(assumedRequest: AssumedRequest) { const ctx = useAppContext(); const rootClusterUri = ctx.workspacesService?.getRootClusterUri(); - const { requestResourcesRefresh } = useResourcesContext(); + const { requestResourcesRefresh } = useResourcesContext(rootClusterUri); const [duration, setDuration] = useState(() => getDurationFromNow({ @@ -53,7 +53,7 @@ export function useAssumedRolesBar(assumedRequest: AssumedRequest) { await retryWithRelogin(ctx, rootClusterUri, () => ctx.clustersService.dropRoles(rootClusterUri, [assumedRequest.id]) ); - requestResourcesRefresh(rootClusterUri); + requestResourcesRefresh(); } catch (err) { ctx.notificationsService.notifyError({ title: 'Could not switch back the role', diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx index 99b1584475281..e6c6929ef25fa 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -131,7 +131,7 @@ export const ConnectMyComputerContextProvider: FC< workspacesService, usageService, } = ctx; - const { requestResourcesRefresh } = useResourcesContext(); + const { requestResourcesRefresh } = useResourcesContext(rootClusterUri); clustersService.useState(); const [ @@ -322,7 +322,7 @@ export const ConnectMyComputerContextProvider: FC< } if (hasNodeRemovalSucceeded) { - requestResourcesRefresh(rootClusterUri); + requestResourcesRefresh(); } ctx.notificationsService.notifyInfo( diff --git a/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts b/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts index eea143cf41d17..8493052ba7b5b 100644 --- a/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts +++ b/web/packages/teleterm/src/ui/DocumentAccessRequests/useAssumeAccess.ts @@ -30,13 +30,13 @@ export function useAssumeAccess() { rootClusterUri, documentsService, } = useWorkspaceContext(); - const { requestResourcesRefresh } = useResourcesContext(); + const { requestResourcesRefresh } = useResourcesContext(rootClusterUri); const [assumeRoleAttempt, runAssumeRole] = useAsync((requestId: string) => retryWithRelogin(ctx, clusterUri, async () => { await ctx.clustersService.assumeRoles(rootClusterUri, [requestId]); // Refresh the current resource tabs in the workspace. - requestResourcesRefresh(rootClusterUri); + requestResourcesRefresh(); }) ); @@ -59,7 +59,7 @@ export function useAssumeAccess() { } // Refresh the current resource tabs in the workspace. - requestResourcesRefresh(rootClusterUri); + requestResourcesRefresh(); // open new cluster tab const clusterDocument = documentsService.createClusterDocument({ diff --git a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx index 28ff780357491..a69d79f3eb9ea 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.test.tsx @@ -46,7 +46,6 @@ import { } from 'teleterm/services/tshd/testHelpers'; import { getEmptyPendingAccessRequest } from 'teleterm/ui/services/workspacesService/accessRequestsService'; import { MockedUnaryCall } from 'teleterm/services/tshd/cloneableClient'; -import { DocumentClusterQueryParams } from 'teleterm/ui/services/workspacesService'; import * as uri from 'teleterm/ui/uri'; const mio = mockIntersectionObserver(); @@ -280,7 +279,6 @@ test.each([ name: 'refreshes resources when the document cluster URI matches the requested cluster URI', conditions: { documentClusterUri: '/clusters/teleport-local', - rootClusterUriToRefresh: '/clusters/teleport-local', }, expect: { resourcesRefreshed: true, @@ -290,23 +288,11 @@ test.each([ name: 'refreshes resources when the document cluster URI is a leaf of the requested cluster URI', conditions: { documentClusterUri: '/clusters/teleport-local/leaves/leaf', - rootClusterUriToRefresh: '/clusters/teleport-local', }, expect: { resourcesRefreshed: true, }, }, - - { - name: 'does not refresh resources when the document cluster URI is not related to the requested cluster URI', - conditions: { - documentClusterUri: '/clusters/teleport-remote', - rootClusterUriToRefresh: '/clusters/teleport-local', - }, - expect: { - resourcesRefreshed: false, - }, - }, ])('$name', async testCase => { const doc = makeDocumentCluster({ clusterUri: testCase.conditions.documentClusterUri, @@ -347,7 +333,7 @@ test.each([ }); const ref = createRef<{ - requestResourcesRefresh: (rootClusterUri: uri.RootClusterUri) => void; + requestResourcesRefresh: () => void; }>(); render( @@ -355,8 +341,8 @@ test.each([ - + - ref.current.requestResourcesRefresh( - testCase.conditions.rootClusterUriToRefresh - ) - ); + act(() => ref.current.requestResourcesRefresh()); // Wait for resources to (potentially) re-render. await expect( @@ -393,26 +375,17 @@ test.each([ ).toHaveBeenCalledTimes(testCase.expect.resourcesRefreshed ? 2 : 1); }); -const UnifiedResourcesWithRefreshExposed = forwardRef< +const Refresher = forwardRef< { - requestResourcesRefresh: (rootClusterUri: uri.RootClusterUri) => void; + requestResourcesRefresh: () => void; }, { - clusterUri: uri.ClusterUri; - docUri: uri.DocumentUri; - queryParams: DocumentClusterQueryParams; + rootClusterUri: uri.RootClusterUri; } >((props, ref) => { - const resourcesContext = useResourcesContext(); + const resourcesContext = useResourcesContext(props.rootClusterUri); useImperativeHandle(ref, () => ({ requestResourcesRefresh: resourcesContext.requestResourcesRefresh, })); - - return ( - - ); + return null; }); diff --git a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx index 599bc01828f7d..66764c2184e08 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx @@ -74,7 +74,7 @@ import { ConnectAppActionButton, AccessRequestButton, } from './ActionButtons'; -import { useResourcesContext, ResourcesContext } from './resourcesContext'; +import { useResourcesContext } from './resourcesContext'; import { useUserPreferences } from './useUserPreferences'; export function UnifiedResources(props: { @@ -102,7 +102,7 @@ export function UnifiedResources(props: { [rootClusterUri] ) ); - const { onResourcesRefreshRequest } = useResourcesContext(); + const { onResourcesRefreshRequest } = useResourcesContext(rootClusterUri); const loggedInUser = useWorkspaceLoggedInUser(); const { unifiedResourcePreferences } = userPreferences; @@ -263,7 +263,7 @@ const Resources = memo( canAddResources: boolean; canUseConnectMyComputer: boolean; openConnectMyComputerDocument(): void; - onResourcesRefreshRequest: ResourcesContext['onResourcesRefreshRequest']; + onResourcesRefreshRequest(listener: () => void): { cleanup(): void }; discoverUrl: string; getAccessRequestButton: (resource: UnifiedResourceResponse) => JSX.Element; getAddedItemsCount: () => number; @@ -328,11 +328,7 @@ const Resources = memo( const { onResourcesRefreshRequest } = props; useEffect(() => { - const { cleanup } = onResourcesRefreshRequest(rootClusterUri => { - // Refresh root and leaf cluster documents. - if (!uri.routing.belongsToProfile(rootClusterUri, props.clusterUri)) { - return; - } + const { cleanup } = onResourcesRefreshRequest(() => { void fetch({ clear: true }); }); return cleanup; diff --git a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx index 3427619e6aa64..37d1339db9611 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.test.tsx @@ -31,14 +31,28 @@ describe('requestResourcesRefresh', () => { const wrapper = ({ children }) => ( {children} ); - const { result } = renderHook(() => useResourcesContext(), { wrapper }); + const { result } = renderHook( + () => ({ + clusterUri1: useResourcesContext('/clusters/teleport-local'), + clusterUri2: useResourcesContext('/clusters/teleport-remote'), + }), + { + wrapper, + } + ); - const listener = jest.fn(); - result.current.onResourcesRefreshRequest(listener); - result.current.requestResourcesRefresh(rootClusterUri); + const listener1 = jest.fn(); + const listener2 = jest.fn(); + result.current.clusterUri1.onResourcesRefreshRequest(listener1); + result.current.clusterUri2.onResourcesRefreshRequest(listener2); + + result.current.clusterUri1.requestResourcesRefresh(); + expect(listener1).toHaveBeenCalledTimes(1); + expect(listener2).not.toHaveBeenCalled(); - expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith(rootClusterUri); + result.current.clusterUri2.requestResourcesRefresh(); + expect(listener1).toHaveBeenCalledTimes(1); // it has not been called again + expect(listener2).toHaveBeenCalledTimes(1); }); }); @@ -47,13 +61,15 @@ describe('onResourcesRefreshRequest cleanup function', () => { const wrapper = ({ children }) => ( {children} ); - const { result } = renderHook(() => useResourcesContext(), { wrapper }); + const { result } = renderHook(() => useResourcesContext(rootClusterUri), { + wrapper, + }); const listener = jest.fn(); const { cleanup } = result.current.onResourcesRefreshRequest(listener); cleanup(); - result.current.requestResourcesRefresh(rootClusterUri); + result.current.requestResourcesRefresh(); expect(listener).not.toHaveBeenCalled(); }); diff --git a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx index 151bef27b4153..8cf37d4a82654 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx @@ -84,7 +84,7 @@ export const ResourcesContextProvider: FC = props => { ); }; -export const useResourcesContext = () => { +export const useResourcesContext = (rootClusterUri: RootClusterUri) => { const context = useContext(ResourcesContext); if (!context) { @@ -93,5 +93,24 @@ export const useResourcesContext = () => { ); } - return context; + const { + requestResourcesRefresh: requestResourcesRefreshContext, + onResourcesRefreshRequest: onResourcesRefreshRequestContext, + } = context; + + return { + requestResourcesRefresh: useCallback( + () => requestResourcesRefreshContext(rootClusterUri), + [requestResourcesRefreshContext, rootClusterUri] + ), + onResourcesRefreshRequest: useCallback( + (listener: () => void) => + onResourcesRefreshRequestContext(requestedRootClusterUri => { + if (requestedRootClusterUri === rootClusterUri) { + listener(); + } + }), + [onResourcesRefreshRequestContext, rootClusterUri] + ), + }; }; From f6c69a930edb1cbc3cc2f17d10043194cd66d791 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 19 Nov 2024 17:27:30 +0100 Subject: [PATCH 07/10] Change the error message --- .../teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts index d71b092bd5f81..b038ec8d6b283 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAssumedRolesBar.ts @@ -56,7 +56,7 @@ export function useAssumedRolesBar(assumedRequest: AssumedRequest) { requestResourcesRefresh(); } catch (err) { ctx.notificationsService.notifyError({ - title: 'Could not switch back the role', + title: 'Could not drop role', description: err.message, }); } From e5176804ff1255c3770139e836a07ed89c2c0d26 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 19 Nov 2024 17:49:09 +0100 Subject: [PATCH 08/10] Change one more usage of `requestResourcesRefresh(rootClusterUri)` --- .../ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx index bbcc5dc0cd88f..8e8c7ba3c3e33 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputer/Setup.tsx @@ -210,7 +210,7 @@ function AgentSetup() { setDownloadAgentAttempt, agentProcessState, } = useConnectMyComputerContext(); - const { requestResourcesRefresh } = useResourcesContext(); + const { requestResourcesRefresh } = useResourcesContext(rootClusterUri); const rootCluster = ctx.clustersService.findCluster(rootClusterUri); // The verify agent step checks if we can execute the binary. This triggers OS-level checks, such @@ -281,8 +281,8 @@ function AgentSetup() { // Now that the node has joined the server, let's refresh open DocumentCluster // instances in the workspace to show the new node. - requestResourcesRefresh(rootClusterUri); - }, [startAgent, requestResourcesRefresh, rootClusterUri]) + requestResourcesRefresh(); + }, [startAgent, requestResourcesRefresh]) ); const steps: SetupStep[] = [ From 4d19bcbf83dbb89ef9d6477289537418dab60a33 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 20 Nov 2024 15:31:59 +0100 Subject: [PATCH 09/10] Move `onResourcesRefreshRequest` listener logic to `ResourcesContextProvider` --- .../ui/DocumentCluster/resourcesContext.tsx | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx index 8cf37d4a82654..a8794ba6533d1 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx @@ -40,10 +40,12 @@ export interface ResourcesContext { requestResourcesRefresh: (rootClusterUri: RootClusterUri) => void; /** * onResourcesRefreshRequest registers a listener that will be called any time a refresh is - * requested. Typically called from useEffect, for this purpose it returns a cleanup function. + * requested for a particular rootClusterUri. Typically called from useEffect, for this purpose it + * returns a cleanup function. */ onResourcesRefreshRequest: ( - listener: (rootClusterUri: RootClusterUri) => void + rootClusterUri: RootClusterUri, + listener: () => void ) => { cleanup: () => void; }; @@ -64,7 +66,15 @@ export const ResourcesContextProvider: FC = props => { ); const onResourcesRefreshRequest = useCallback( - (listener: (rootClusterUri: RootClusterUri) => void) => { + ( + targetRootClusterUri: RootClusterUri, + listenerWithoutRootClusterUri: () => void + ) => { + const listener = (rootClusterUri: RootClusterUri) => { + if (rootClusterUri === targetRootClusterUri) { + listenerWithoutRootClusterUri(); + } + }; emitterRef.current.addListener('refresh', listener); return { @@ -105,11 +115,7 @@ export const useResourcesContext = (rootClusterUri: RootClusterUri) => { ), onResourcesRefreshRequest: useCallback( (listener: () => void) => - onResourcesRefreshRequestContext(requestedRootClusterUri => { - if (requestedRootClusterUri === rootClusterUri) { - listener(); - } - }), + onResourcesRefreshRequestContext(rootClusterUri, listener), [onResourcesRefreshRequestContext, rootClusterUri] ), }; From 1d9784b16e312f12ceccd1c22b7726c3f8b7db6d Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 20 Nov 2024 15:33:49 +0100 Subject: [PATCH 10/10] Remove unnecessary `props.clusterUri` from deps --- .../teleterm/src/ui/DocumentCluster/UnifiedResources.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx index 66764c2184e08..b3f9de5fe0c73 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/UnifiedResources.tsx @@ -332,7 +332,7 @@ const Resources = memo( void fetch({ clear: true }); }); return cleanup; - }, [onResourcesRefreshRequest, fetch, props.clusterUri]); + }, [onResourcesRefreshRequest, fetch]); const { getAccessRequestButton } = props; // The action callback in the requestAccess action has access to