fix: show dashed lines for empty values instead of removing them (#79)

flyteorg · Jul 1, 2020 · 0eaaecd · 0eaaecd
1 parent b532c7b
commit 0eaaecd
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 29 deletions.
diff --git a/src/common/constants.ts b/src/common/constants.ts
@@ -10,6 +10,7 @@ export const detailsPanelId = 'details-panel';
 export const navBarContentId = 'nav-bar-content';
 
 export const unknownValueString = '(unknown)';
+export const dashedValueString = '----';
 export const noDescriptionString = '(No description)';
 export const noneString = '(none)';
 export const noExecutionsFoundString = 'No executions found.';

diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx
@@ -1,7 +1,7 @@
 import { Typography } from '@material-ui/core';
 import { makeStyles, Theme } from '@material-ui/core/styles';
 import * as classnames from 'classnames';
-import { unknownValueString } from 'common/constants';
+import { dashedValueString } from 'common/constants';
 import { formatDateUTC, protobufDurationToHMS } from 'common/formatters';
 import { timestampToDate } from 'common/utils';
 import { useCommonStyles } from 'components/common/styles';
@@ -66,7 +66,7 @@ export const ExecutionMetadata: React.FC<{
     const { domain } = execution.id;
     const { duration, error, startedAt, workflowId } = execution.closure;
     const { systemMetadata } = execution.spec.metadata;
-    const cluster = systemMetadata?.executionCluster ?? unknownValueString;
+    const cluster = systemMetadata?.executionCluster ?? dashedValueString;
 
     const details: DetailItem[] = [
         { label: ExecutionMetadataLabels.domain, value: domain },
@@ -78,20 +78,20 @@ export const ExecutionMetadata: React.FC<{
         {
             label: ExecutionMetadataLabels.cluster,
             value: cluster
-        }
-    ];
-    if (startedAt) {
-        details.push({
+        },
+        {
             label: ExecutionMetadataLabels.time,
-            value: formatDateUTC(timestampToDate(startedAt))
-        });
-    }
-    if (duration) {
-        details.push({
+            value: startedAt
+                ? formatDateUTC(timestampToDate(startedAt))
+                : dashedValueString
+        },
+        {
             label: ExecutionMetadataLabels.duration,
-            value: protobufDurationToHMS(duration)
-        });
-    }
+            value: duration
+                ? protobufDurationToHMS(duration)
+                : dashedValueString
+        }
+    ];
 
     return (
         <div className={styles.container}>

diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx
@@ -1,12 +1,14 @@
 import { render } from '@testing-library/react';
-import { unknownValueString } from 'common/constants';
+import { dashedValueString } from 'common/constants';
 import { Execution } from 'models';
 import { createMockExecution } from 'models/__mocks__/executionsData';
 import * as React from 'react';
 import { ExecutionMetadataLabels } from '../constants';
 import { ExecutionMetadata } from '../ExecutionMetadata';
 
 const clusterTestId = `metadata-${ExecutionMetadataLabels.cluster}`;
+const startTimeTestId = `metadata-${ExecutionMetadataLabels.time}`;
+const durationTestId = `metadata-${ExecutionMetadataLabels.duration}`;
 
 describe('ExecutionMetadata', () => {
     let execution: Execution;
@@ -28,31 +30,31 @@ describe('ExecutionMetadata', () => {
         );
     });
 
-    it('shows unknown string for cluster if no metadata', () => {
+    it('shows empty string for cluster if no metadata', () => {
         delete execution.spec.metadata.systemMetadata;
         const { getByTestId } = renderMetadata();
-        expect(getByTestId(clusterTestId)).toHaveTextContent(
-            unknownValueString
-        );
+        expect(getByTestId(clusterTestId)).toHaveTextContent(dashedValueString);
     });
 
-    it('shows unknown string for cluster if no cluster name', () => {
+    it('shows empty string for cluster if no cluster name', () => {
         delete execution.spec.metadata.systemMetadata?.executionCluster;
         const { getByTestId } = renderMetadata();
-        expect(getByTestId(clusterTestId)).toHaveTextContent(
-            unknownValueString
-        );
+        expect(getByTestId(clusterTestId)).toHaveTextContent(dashedValueString);
     });
 
-    it('does not show start time if not available', () => {
+    it('shows empty string for start time if not available', () => {
         delete execution.closure.startedAt;
-        const { queryByText } = renderMetadata();
-        expect(queryByText(ExecutionMetadataLabels.time)).toBeNull;
+        const { getByTestId } = renderMetadata();
+        expect(getByTestId(startTimeTestId)).toHaveTextContent(
+            dashedValueString
+        );
     });
 
-    it('does not show duration if not available', () => {
+    it('shows empty string for duration if not available', () => {
         delete execution.closure.duration;
-        const { queryByText } = renderMetadata();
-        expect(queryByText(ExecutionMetadataLabels.duration)).toBeNull;
+        const { getByTestId } = renderMetadata();
+        expect(getByTestId(durationTestId)).toHaveTextContent(
+            dashedValueString
+        );
     });
 });