diff --git a/web/packages/teleterm/src/services/tshd/fixtures/mocks.ts b/web/packages/teleterm/src/services/tshd/fixtures/mocks.ts index 343af90cb640c..b9d138a700f43 100644 --- a/web/packages/teleterm/src/services/tshd/fixtures/mocks.ts +++ b/web/packages/teleterm/src/services/tshd/fixtures/mocks.ts @@ -81,7 +81,7 @@ export class MockTshClient implements TshClient { getCluster: (clusterUri: string) => Promise; getAuthSettings: (clusterUri: string) => Promise; - removeCluster: (clusterUri: string) => Promise; + removeCluster = () => Promise.resolve(); loginLocal: ( params: LoginLocalParams, abortSignal?: TshAbortSignal @@ -94,7 +94,7 @@ export class MockTshClient implements TshClient { params: LoginPasswordlessParams, abortSignal?: TshAbortSignal ) => Promise; - logout: (clusterUri: string) => Promise; + logout = () => Promise.resolve(); transferFile: () => undefined; reportUsageEvent: () => undefined; } diff --git a/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.story.tsx b/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.story.tsx index fc887bda3c140..6213a6fbf24a2 100644 --- a/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.story.tsx +++ b/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.story.tsx @@ -16,6 +16,8 @@ import React from 'react'; +import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; + import { ClusterLogout } from './ClusterLogout'; export default { @@ -24,13 +26,12 @@ export default { export const Story = () => { return ( - Promise.resolve([undefined, null])} - onClose={() => {}} - clusterUri="/clusters/foo" - clusterTitle="foo" - /> + + {}} + clusterUri="/clusters/foo" + clusterTitle="foo" + /> + ); }; diff --git a/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.tsx b/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.tsx index d893e74944f0e..3d156b0e710f6 100644 --- a/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.tsx +++ b/web/packages/teleterm/src/ui/ClusterLogout/ClusterLogout.tsx @@ -25,20 +25,32 @@ import { ButtonIcon, ButtonPrimary, Text } from 'design'; import { Close } from 'design/Icon'; -import { Props, State, useClusterLogout } from './useClusterLogout'; +import { RootClusterUri } from 'teleterm/ui/uri'; -export default function Container(props: Props) { - const state = useClusterLogout(props); - return ; +import { useClusterLogout } from './useClusterLogout'; + +interface ClusterLogoutProps { + clusterTitle: string; + clusterUri: RootClusterUri; + onClose(): void; } export function ClusterLogout({ - status, + clusterUri, onClose, - statusText, clusterTitle, - removeCluster, -}: State) { +}: ClusterLogoutProps) { + const { removeCluster, status, statusText } = useClusterLogout({ + clusterUri, + }); + + async function removeClusterAndClose(): Promise { + const [, err] = await removeCluster(); + if (!err) { + onClose(); + } + } + return ( { e.preventDefault(); - removeCluster(); + removeClusterAndClose(); }} > diff --git a/web/packages/teleterm/src/ui/ClusterLogout/useClusterLogout.ts b/web/packages/teleterm/src/ui/ClusterLogout/useClusterLogout.ts index 8443a87177220..71ba4eebd8955 100644 --- a/web/packages/teleterm/src/ui/ClusterLogout/useClusterLogout.ts +++ b/web/packages/teleterm/src/ui/ClusterLogout/useClusterLogout.ts @@ -15,13 +15,16 @@ */ import { useAsync } from 'shared/hooks/useAsync'; -import { useEffect } from 'react'; import { RootClusterUri } from 'teleterm/ui/uri'; import { useAppContext } from '../appContextProvider'; -export function useClusterLogout({ clusterUri, onClose, clusterTitle }: Props) { +export function useClusterLogout({ + clusterUri, +}: { + clusterUri: RootClusterUri; +}) { const ctx = useAppContext(); const [{ status, statusText }, removeCluster] = useAsync(async () => { await ctx.clustersService.logout(clusterUri); @@ -35,30 +38,18 @@ export function useClusterLogout({ clusterUri, onClose, clusterTitle }: Props) { await ctx.workspacesService.setActiveWorkspace(null); } } - ctx.workspacesService.removeWorkspace(clusterUri); + + // remove connections first, they depend both on the cluster and the workspace ctx.connectionTracker.removeItemsBelongingToRootCluster(clusterUri); + // remove the workspace next, because it depends on the cluster + ctx.workspacesService.removeWorkspace(clusterUri); + // remove the cluster, it does not depend on anything + await ctx.clustersService.removeClusterAndResources(clusterUri); }); - useEffect(() => { - if (status === 'success') { - onClose(); - } - }, [status]); - return { status, statusText, removeCluster, - onClose, - clusterUri, - clusterTitle, }; } - -export type Props = { - onClose?(): void; - clusterTitle?: string; - clusterUri: RootClusterUri; -}; - -export type State = ReturnType; diff --git a/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.test.tsx b/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.test.tsx index a016afb5df18b..ff5544106e90f 100644 --- a/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.test.tsx +++ b/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.test.tsx @@ -39,15 +39,14 @@ const documentsReopenDialog: DialogDocumentsReopen = { onCancel: () => {}, }; -jest.mock('teleterm/ui/ClusterLogout/ClusterLogout', () => { - const MockClusterLogout = () => ( +jest.mock('teleterm/ui/ClusterLogout', () => ({ + ClusterLogout: () => (
- ); - return MockClusterLogout; -}); + ), +})); jest.mock('teleterm/ui/DocumentsReopen', () => ({ DocumentsReopen: () => ( diff --git a/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx b/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx index 1cebd130e3c50..699f54f498479 100644 --- a/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx +++ b/web/packages/teleterm/src/ui/ModalsHost/ModalsHost.tsx @@ -22,7 +22,7 @@ import { ClusterConnect } from 'teleterm/ui/ClusterConnect'; import { DocumentsReopen } from 'teleterm/ui/DocumentsReopen'; import { Dialog } from 'teleterm/ui/services/modals'; -import ClusterLogout from '../ClusterLogout/ClusterLogout'; +import { ClusterLogout } from '../ClusterLogout'; import { ResourceSearchErrors } from '../Search/ResourceSearchErrors'; import { assertUnreachable } from '../utils'; diff --git a/web/packages/teleterm/src/ui/hooks/useLoggedInUser.ts b/web/packages/teleterm/src/ui/hooks/useLoggedInUser.ts index 73043b37ec92f..72875ab119def 100644 --- a/web/packages/teleterm/src/ui/hooks/useLoggedInUser.ts +++ b/web/packages/teleterm/src/ui/hooks/useLoggedInUser.ts @@ -25,8 +25,7 @@ import { LoggedInUser } from 'teleterm/services/tshd/types'; * It should be used within components that reside outside of WorkspaceContext, typically anything * that's outside of Document-type components. * - * It might return undefined if there's no active workspace or during the logout procedure because - * ClustersService state is cleared up before WorkspacesService state. + * It might return undefined if there's no active workspace. */ export function useLoggedInUser(): LoggedInUser | undefined { const { clustersService, workspacesService } = useAppContext(); @@ -51,10 +50,8 @@ export function useLoggedInUser(): LoggedInUser | undefined { * rendered inside of them. * * In general, the callsite should always assume that this function might return undefined. - * One case where it will for sure return undefined is during the logout process as - * ClustersService state is cleared up before WorkspacesService state. On top of that, each - * workspace is always rendered, even when the cluster is not connected, with at least the default - * document. In that scenario useWorkspaceLoggedInUser could return undefined when used within the + * Each workspace is always rendered, even when the cluster is not connected, with at least the default + * document. In that scenario, useWorkspaceLoggedInUser could return undefined when used within the * default document. */ export function useWorkspaceLoggedInUser(): LoggedInUser | undefined { diff --git a/web/packages/teleterm/src/ui/services/clusters/clustersService.test.ts b/web/packages/teleterm/src/ui/services/clusters/clustersService.test.ts index 6e2023175cf9f..b4443f07f0052 100644 --- a/web/packages/teleterm/src/ui/services/clusters/clustersService.test.ts +++ b/web/packages/teleterm/src/ui/services/clusters/clustersService.test.ts @@ -123,10 +123,7 @@ test('add cluster', async () => { }); test('remove cluster', async () => { - const { removeCluster } = getClientMocks(); - const service = createService({ - removeCluster, - }); + const service = createService({}); service.setState(draftState => { draftState.clusters = new Map([ @@ -135,9 +132,8 @@ test('remove cluster', async () => { ]); }); - await service.removeCluster(clusterUri); + await service.removeClusterAndResources(clusterUri); - expect(removeCluster).toHaveBeenCalledWith(clusterUri); expect(service.findCluster(clusterUri)).toBeUndefined(); expect(service.findCluster(leafClusterMock.uri)).toBeUndefined(); }); @@ -192,8 +188,9 @@ test('logout from cluster', async () => { await service.logout(clusterUri); expect(logout).toHaveBeenCalledWith(clusterUri); - expect(service.findCluster(clusterUri)).toBeUndefined(); - expect(service.findCluster(leafClusterMock.uri)).toBeUndefined(); + expect(removeCluster).toHaveBeenCalledWith(clusterUri); + expect(service.findCluster(clusterMock.uri).connected).toBe(false); + expect(service.findCluster(leafClusterMock.uri).connected).toBe(false); }); test('create a gateway', async () => { diff --git a/web/packages/teleterm/src/ui/services/clusters/clustersService.ts b/web/packages/teleterm/src/ui/services/clusters/clustersService.ts index 24bcddc01bb88..2806a02cfa89a 100644 --- a/web/packages/teleterm/src/ui/services/clusters/clustersService.ts +++ b/web/packages/teleterm/src/ui/services/clusters/clustersService.ts @@ -71,11 +71,27 @@ export class ClustersService extends ImmutableStore return cluster; } + /** + * Logs out of the cluster and removes the profile. + * Does not remove the cluster from the state, but sets the cluster and its leafs as disconnected. + * It needs to be done, because some code can operate on the cluster the intermediate period between logout + * and actually removing it from the state. + * A code that operates on that intermediate state is in `useClusterLogout.tsx`. + * After invoking `logout()`, it looks for the next workspace to switch to. If we hadn't marked the cluster as disconnected, + * the method might have returned us the same cluster we wanted to log out of. + */ async logout(clusterUri: uri.RootClusterUri) { // TODO(gzdunek): logout and removeCluster should be combined into a single acton in tshd await this.client.logout(clusterUri); - await this.removeCluster(clusterUri); - await this.removeClusterKubeConfigs(clusterUri); + await this.client.removeCluster(clusterUri); + + this.setState(draft => { + draft.clusters.forEach(cluster => { + if (routing.belongsToProfile(clusterUri, cluster.uri)) { + cluster.connected = false; + } + }); + }); } async loginLocal( @@ -301,23 +317,16 @@ export class ClustersService extends ImmutableStore return response; } - /** - * Removes cluster and its leaf clusters (if any) - */ - async removeCluster(clusterUri: uri.RootClusterUri) { - await this.client.removeCluster(clusterUri); - const leafClustersUris = this.getClusters() - .filter( - item => - item.leaf && routing.ensureRootClusterUri(item.uri) === clusterUri - ) - .map(cluster => cluster.uri); + /** Removes cluster, its leafs and other resources. */ + async removeClusterAndResources(clusterUri: uri.RootClusterUri) { this.setState(draft => { - draft.clusters.delete(clusterUri); - leafClustersUris.forEach(leafClusterUri => { - draft.clusters.delete(leafClusterUri); + draft.clusters.forEach(cluster => { + if (routing.belongsToProfile(clusterUri, cluster.uri)) { + draft.clusters.delete(cluster.uri); + } }); }); + await this.removeClusterKubeConfigs(clusterUri); } async getAuthSettings(clusterUri: uri.RootClusterUri) {