From aa0d14be37226d1faf945caafe2d1a912d1eb5b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= <rafal.cieslak@goteleport.com>
Date: Thu, 28 Sep 2023 17:35:28 +0200
Subject: [PATCH] [v14] Make spacing of Connect My Computer status more
 consistent (#32736)

* Make spacing of Connect My Computer status more consistent

* Add server labels to Running story, add ErrorWithAlertAndLogs story

* Change copy depending on whether the agent is running

* Fix proxyVersion in story

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>

---------

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>
---
 .../CompatibilityPromise.tsx                  |  10 +-
 .../DocumentConnectMyComputerStatus.story.tsx | 109 +++++---
 .../DocumentConnectMyComputerStatus.tsx       | 237 ++++++++++--------
 .../src/ui/ConnectMyComputer/Logs.tsx         |   6 +-
 4 files changed, 206 insertions(+), 156 deletions(-)

diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx
index 58f6dc52a3cdb..76ab0b49428b5 100644
--- a/web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx
+++ b/web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx
@@ -15,8 +15,7 @@
  */
 
 import React from 'react';
-import { Text, ButtonPrimary, Alert } from 'design';
-
+import { Text, ButtonPrimary, Alert, Flex } from 'design';
 import Link from 'design/Link';
 
 import { useAppContext } from 'teleterm/ui/appContextProvider';
@@ -92,9 +91,9 @@ export function CompatibilityError(props: {
   }
 
   return (
-    <>
+    <Flex flexDirection="column" gap={2}>
       {!props.hideAlert && (
-        <Alert>
+        <Alert mb={0}>
           The agent version is not compatible with the cluster version
         </Alert>
       )}
@@ -110,7 +109,6 @@ export function CompatibilityError(props: {
         {$content}
       </Text>
       <ButtonPrimary
-        mt={3}
         mx="auto"
         type="button"
         as="a"
@@ -120,7 +118,7 @@ export function CompatibilityError(props: {
       >
         Visit the downloads page
       </ButtonPrimary>
-    </>
+    </Flex>
   );
 }
 
diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.story.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.story.tsx
index f7969ce189351..1bdc42c6e3c93 100644
--- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.story.tsx
+++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.story.tsx
@@ -14,11 +14,13 @@
  * limitations under the License.
  */
 
-import React from 'react';
+import React, { useLayoutEffect } from 'react';
 
-import { wait } from 'shared/utils/wait';
-
-import { makeRootCluster } from 'teleterm/services/tshd/testHelpers';
+import {
+  makeRootCluster,
+  makeServer,
+  makeLabelsList,
+} from 'teleterm/services/tshd/testHelpers';
 import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider';
 import AppContext from 'teleterm/ui/appContext';
 
@@ -30,6 +32,7 @@ import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks';
 import {
   AgentCompatibilityError,
   ConnectMyComputerContextProvider,
+  NodeWaitJoinTimeout,
 } from '../connectMyComputerContext';
 
 import { DocumentConnectMyComputerStatus } from './DocumentConnectMyComputerStatus';
@@ -43,7 +46,45 @@ export function NotStarted() {
 }
 
 export function Running() {
-  return <ShowState agentProcessState={{ status: 'running' }} />;
+  const appContext = new MockAppContext({ appVersion: '17.0.0' });
+
+  let agentUpdateListener: (state: AgentProcessState) => void;
+  appContext.mainProcessClient.subscribeToAgentUpdate = (
+    rootClusterUri,
+    listener
+  ) => {
+    agentUpdateListener = listener;
+    return { cleanup: () => undefined };
+  };
+  appContext.connectMyComputerService.isAgentConfigFileCreated = () =>
+    Promise.resolve(true);
+  appContext.connectMyComputerService.runAgent = async () => {
+    agentUpdateListener({ status: 'running' });
+  };
+  appContext.connectMyComputerService.waitForNodeToJoin = () =>
+    Promise.resolve(
+      makeServer({
+        hostname: 'staging-mac-mini',
+        labelsList: makeLabelsList({
+          hostname: 'staging-mac-mini',
+          'teleport.dev/connect-my-computer/owner': 'testuser@goteleport.com',
+        }),
+      })
+    );
+
+  useLayoutEffect(() => {
+    (
+      document.querySelector('[data-testid=start-agent]') as HTMLButtonElement
+    )?.click();
+  });
+
+  return (
+    <ShowState
+      appContext={appContext}
+      agentProcessState={{ status: 'not-started' }}
+      proxyVersion="17.0.0"
+    />
+  );
 }
 
 export function Errored() {
@@ -84,6 +125,30 @@ export function ExitedUnsuccessfully() {
   );
 }
 
+export function ErrorWithAlertAndLogs() {
+  const appContext = new MockAppContext({ appVersion: '17.0.0' });
+
+  appContext.connectMyComputerService.isAgentConfigFileCreated = () =>
+    Promise.resolve(true);
+  appContext.connectMyComputerService.waitForNodeToJoin = () =>
+    Promise.reject(
+      new NodeWaitJoinTimeout(
+        'teleport: error: unknown short flag -non-existing-flag'
+      )
+    );
+
+  return (
+    <ShowState
+      appContext={appContext}
+      agentProcessState={{
+        status: 'not-started',
+      }}
+      autoStart={true}
+      proxyVersion="17.0.0"
+    />
+  );
+}
+
 export function FailedToReadAgentConfigFile() {
   const appContext = new MockAppContext();
   appContext.connectMyComputerService.isAgentConfigFileCreated = async () => {
@@ -182,40 +247,6 @@ function ShowState(props: {
     new MockAppContext({ appVersion: cluster.proxyVersion });
 
   appContext.mainProcessClient.getAgentState = () => props.agentProcessState;
-  appContext.mainProcessClient.subscribeToAgentUpdate = (
-    rootClusterUri,
-    listener
-  ) => {
-    listener(props.agentProcessState);
-    return { cleanup: () => undefined };
-  };
-  appContext.connectMyComputerService.runAgent = async () => {
-    await wait(1_000);
-  };
-  appContext.connectMyComputerService.waitForNodeToJoin = async () => {
-    if (props.agentProcessState.status === 'running') {
-      await wait(2_000);
-      return {
-        uri: `${cluster.uri}/servers/178ef081-259b-4aa5-a018-449b5ea7e694`,
-        tunnel: false,
-        name: '178ef081-259b-4aa5-a018-449b5ea7e694',
-        hostname: 'staging-mac-mini',
-        addr: '127.0.0.1:3022',
-        labelsList: [
-          {
-            name: 'hostname',
-            value: 'staging-mac-mini',
-          },
-          {
-            name: 'teleport.dev/connect-my-computer',
-            value: 'testuser@goteleport.com',
-          },
-        ],
-      };
-    }
-    await wait(3_000);
-    throw new Error('TIMEOUT. Cannot find node.');
-  };
   appContext.configService.set('feature.connectMyComputer', true);
   appContext.clustersService.state.clusters.set(cluster.uri, cluster);
   appContext.workspacesService.setState(draftState => {
diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx
index 7a973f22eea8a..f36ed115685bc 100644
--- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx
+++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx
@@ -175,119 +175,140 @@ export function DocumentConnectMyComputerStatus(
           again.
         </Alert>
       )}
-      <Flex justifyContent="space-between" mb={3}>
-        <Text
-          typography="h3"
-          css={`
-            display: flex;
-          `}
-        >
-          <Laptop mr={2} />
-          {/** The node name can be changed, so it might be different from the system hostname. */}
-          {agentNode?.hostname || hostname}
-        </Text>
-        <MenuIcon
-          buttonIconProps={{
-            css: css`
-              border-radius: ${props => props.theme.space[1]}px;
-              background: ${props => props.theme.colors.spotBackground[0]};
-            `,
-          }}
-          menuProps={{
-            anchorOrigin: {
-              vertical: 'bottom',
-              horizontal: 'right',
-            },
-            transformOrigin: {
-              vertical: 'top',
-              horizontal: 'right',
-            },
-          }}
-        >
-          <MenuItem onClick={openAgentLogs}>Open agent logs directory</MenuItem>
-          <MenuItem onClick={removeAgentAndClose}>Remove agent</MenuItem>
-        </MenuIcon>
-      </Flex>
 
-      <Transition in={!!agentNode} timeout={1_800} mountOnEnter unmountOnExit>
-        {state => (
-          <LabelsContainer gap={1} className={state}>
-            {renderLabels(agentNode.labelsList)}
-          </LabelsContainer>
-        )}
-      </Transition>
-      <Flex
-        mt={3}
-        mb={2}
-        gap={1}
-        display="flex"
-        alignItems="center"
-        minHeight="32px"
-      >
-        {prettyCurrentAction.Icon && <prettyCurrentAction.Icon size="medium" />}
-        {prettyCurrentAction.title}
-        {showConnectAndStopAgentButtons && (
-          <ButtonSecondary
-            onClick={killAgent}
-            disabled={disableConnectAndStopAgentButtons}
-            ml={3}
+      <Flex flexDirection="column" gap={3}>
+        <Flex flexDirection="column" gap={1}>
+          <Flex justifyContent="space-between">
+            <Text
+              typography="h3"
+              css={`
+                display: flex;
+              `}
+            >
+              <Laptop mr={2} />
+              {/** The node name can be changed, so it might be different from the system hostname. */}
+              {agentNode?.hostname || hostname}
+            </Text>
+            <MenuIcon
+              buttonIconProps={{
+                css: css`
+                  border-radius: ${props => props.theme.space[1]}px;
+                  background: ${props => props.theme.colors.spotBackground[0]};
+                `,
+              }}
+              menuProps={{
+                anchorOrigin: {
+                  vertical: 'bottom',
+                  horizontal: 'right',
+                },
+                transformOrigin: {
+                  vertical: 'top',
+                  horizontal: 'right',
+                },
+              }}
+            >
+              <MenuItem onClick={openAgentLogs}>
+                Open agent logs directory
+              </MenuItem>
+              <MenuItem onClick={removeAgentAndClose}>Remove agent</MenuItem>
+            </MenuIcon>
+          </Flex>
+
+          <Transition
+            in={!!agentNode}
+            timeout={1_800}
+            mountOnEnter
+            unmountOnExit
           >
-            Stop Agent
-          </ButtonSecondary>
-        )}
-      </Flex>
-      {prettyCurrentAction.error && (
-        <Alert
-          css={`
-            white-space: pre-wrap;
-          `}
-        >
-          {prettyCurrentAction.error}
-        </Alert>
-      )}
-      {prettyCurrentAction.logs && <Logs logs={prettyCurrentAction.logs} />}
-
-      {isAgentIncompatible ? (
-        <CompatibilityError
-          // Hide the alert if the current action has failed. downloadAgent and startAgent already
-          // return an error message related to compatibility.
-          //
-          // Basically, we have to cover two use cases:
-          //
-          // * Auto start has failed due to compatibility promise, so the downloadAgent failed with
-          // an error.
-          // * Auto start wasn't enabled, so the current action has no errors, but the user should
-          // not be able to start the agent due to compatibility issues.
-          hideAlert={!!prettyCurrentAction.error}
-        />
-      ) : (
-        <>
-          <Text mb={4} mt={1}>
-            Connecting your computer will allow any cluster user with the role{' '}
-            <strong>{roleName}</strong> to access it as an SSH resource with the
-            user <strong>{systemUsername}</strong>.
-          </Text>
-          {showConnectAndStopAgentButtons ? (
-            <ButtonPrimary
-              block
-              disabled={disableConnectAndStopAgentButtons}
-              onClick={startSshSession}
-              size="large"
+            {state => (
+              <LabelsContainer gap={1} className={state}>
+                {renderLabels(agentNode.labelsList)}
+              </LabelsContainer>
+            )}
+          </Transition>
+        </Flex>
+
+        <Flex flexDirection="column" gap={2}>
+          <Flex gap={1} display="flex" alignItems="center" minHeight="32px">
+            {prettyCurrentAction.Icon && (
+              <prettyCurrentAction.Icon size="medium" />
+            )}
+            {prettyCurrentAction.title}
+            {showConnectAndStopAgentButtons && (
+              <ButtonSecondary
+                onClick={killAgent}
+                disabled={disableConnectAndStopAgentButtons}
+                ml={3}
+              >
+                Stop Agent
+              </ButtonSecondary>
+            )}
+          </Flex>
+          {prettyCurrentAction.error && (
+            <Alert
+              mb={0}
+              css={`
+                white-space: pre-wrap;
+              `}
             >
-              Connect
-            </ButtonPrimary>
+              {prettyCurrentAction.error}
+            </Alert>
+          )}
+          {prettyCurrentAction.logs && <Logs logs={prettyCurrentAction.logs} />}
+        </Flex>
+
+        <Flex flexDirection="column" gap={2}>
+          {isAgentIncompatible ? (
+            <CompatibilityError
+              // Hide the alert if the current action has failed. downloadAgent and startAgent already
+              // return an error message related to compatibility.
+              //
+              // Basically, we have to cover two use cases:
+              //
+              // * Auto start has failed due to compatibility promise, so the downloadAgent failed with
+              // an error.
+              // * Auto start wasn't enabled, so the current action has no errors, but the user should
+              // not be able to start the agent due to compatibility issues.
+              hideAlert={!!prettyCurrentAction.error}
+            />
           ) : (
-            <ButtonPrimary
-              block
-              disabled={disableStartAgentButton}
-              onClick={downloadAndStartAgent}
-              size="large"
-            >
-              Start Agent
-            </ButtonPrimary>
+            <>
+              {isRunning ? (
+                <Text>
+                  Any cluster user with the role <strong>{roleName}</strong> can
+                  now access your computer as <strong>{systemUsername}</strong>.
+                </Text>
+              ) : (
+                <Text>
+                  Connecting your computer will allow any cluster user with the
+                  role <strong>{roleName}</strong> to access it as an SSH
+                  resource with the user <strong>{systemUsername}</strong>.
+                </Text>
+              )}
+              {showConnectAndStopAgentButtons ? (
+                <ButtonPrimary
+                  block
+                  disabled={disableConnectAndStopAgentButtons}
+                  onClick={startSshSession}
+                  size="large"
+                >
+                  Connect
+                </ButtonPrimary>
+              ) : (
+                <ButtonPrimary
+                  block
+                  disabled={disableStartAgentButton}
+                  onClick={downloadAndStartAgent}
+                  size="large"
+                  data-testid="start-agent"
+                >
+                  Start Agent
+                </ButtonPrimary>
+              )}
+            </>
           )}
-        </>
-      )}
+        </Flex>
+      </Flex>
     </Box>
   );
 }
diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/Logs.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/Logs.tsx
index 426f7d7603181..613990a5ff95b 100644
--- a/web/packages/teleterm/src/ui/ConnectMyComputer/Logs.tsx
+++ b/web/packages/teleterm/src/ui/ConnectMyComputer/Logs.tsx
@@ -23,8 +23,8 @@ interface LogsProps {
 
 export function Logs(props: LogsProps): JSX.Element {
   return (
-    <>
-      <Text mb={2}>Last 10 lines of logs:</Text>
+    <Flex flexDirection="column" gap={1}>
+      <Text>Last 10 lines of logs:</Text>
       <Flex
         width="100%"
         color="light"
@@ -44,6 +44,6 @@ export function Logs(props: LogsProps): JSX.Element {
           {props.logs}
         </span>
       </Flex>
-    </>
+    </Flex>
   );
 }