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
+ );
  });
 });