ENG-2309 UI bug fixes for better rendering metrics and checks that ha…

…ve no value (#934)

* UI bug fixes to better handle rendering checks and metrics that have no values due to failed workflow execution.

src/ui/common/src/components/tables/CheckTableItem.tsx

@@ -9,42 +9,57 @@ import Box from '@mui/material/Box';
 import React from 'react';
 
 import { theme } from '../../styles/theme/theme';
+import { stringToExecutionStatus } from '../../utils/shared';
+import { StatusIndicator } from '../workflows/workflowStatus';
 
 interface CheckTableItemProps {
   checkValue: string;
+  status?: string;
 }
 
 export const CheckTableItem: React.FC<CheckTableItemProps> = ({
   checkValue,
+  status,
 }) => {
   let iconColor = theme.palette.black;
   let checkIcon = faMinus;
 
-  switch (checkValue.toLowerCase()) {
-    case 'true': {
-      checkIcon = faCircleCheck;
-      iconColor = theme.palette.Success;
-      break;
-    }
-    case 'false': {
-      checkIcon = faCircleExclamation;
-      iconColor = theme.palette.Error;
-      break;
-    }
-    case 'warning': {
-      checkIcon = faTriangleExclamation;
-      iconColor = theme.palette.Warning;
-      break;
-    }
-    case 'none': {
-      checkIcon = faMinus;
-      iconColor = theme.palette.black;
-      break;
-    }
-    default: {
-      // None of the icon cases met, just fall through and render table value.
-      return <>{checkValue}</>;
+  if (checkValue) {
+    switch (checkValue.toLowerCase()) {
+      case 'true': {
+        checkIcon = faCircleCheck;
+        iconColor = theme.palette.Success;
+        break;
+      }
+      case 'false': {
+        checkIcon = faCircleExclamation;
+        iconColor = theme.palette.Error;
+        break;
+      }
+      case 'warning': {
+        checkIcon = faTriangleExclamation;
+        iconColor = theme.palette.Warning;
+        break;
+      }
+      case 'none': {
+        checkIcon = faMinus;
+        iconColor = theme.palette.black;
+        break;
+      }
+      default: {
+        // None of the icon cases met, just fall through and render table value.
+        return <>{checkValue}</>;
+      }
     }
+  } else {
+    // Check value not found, render the status indicator for this check.
+    return (
+      <StatusIndicator
+        status={stringToExecutionStatus(status)}
+        size={'16px'}
+        monochrome={false}
+      />
+    );
   }
 
   return (

src/ui/common/src/components/tables/MetricTableItem.tsx

@@ -0,0 +1,38 @@
+import { Typography } from '@mui/material';
+import React from 'react';
+
+import { stringToExecutionStatus } from '../../utils/shared';
+import { StatusIndicator } from '../workflows/workflowStatus';
+
+interface MetricTableItemProps {
+  metricValue?: string;
+  // ExecutionStatus serialized as a string.
+  status?: string;
+}
+
+export const MetricTableItem: React.FC<MetricTableItemProps> = ({
+  metricValue,
+  status,
+}) => {
+  if (!metricValue) {
+    return (
+      <StatusIndicator
+        status={stringToExecutionStatus(status)}
+        size={'16px'}
+        monochrome={false}
+      />
+    );
+  }
+
+  return (
+    <Typography
+      sx={{
+        fontWeight: 300,
+      }}
+    >
+      {metricValue.toString()}
+    </Typography>
+  );
+};
+
+export default MetricTableItem;

src/ui/common/src/components/tables/OperatorExecStateTable.tsx

@@ -9,6 +9,7 @@ import * as React from 'react';
 
 import { Data, DataSchema } from '../../utils/data';
 import { CheckTableItem } from './CheckTableItem';
+import MetricTableItem from './MetricTableItem';
 
 export enum OperatorExecStateTableType {
   Metric = 'metric',
@@ -44,6 +45,37 @@ export const OperatorExecStateTable: React.FC<OperatorExecStateTableProps> = ({
   tableAlign = 'left',
   tableType,
 }) => {
+  const getTableItem = (tableType, columnName, value, status) => {
+    // return title text in bold.
+    if (columnName === 'title') {
+      return (
+        <Typography
+          sx={{
+            fontWeight: 400,
+          }}
+        >
+          {value.toString()}
+        </Typography>
+      );
+    } else if (tableType === OperatorExecStateTableType.Metric) {
+      // Send off to the MetricTableItem component.
+      return <MetricTableItem metricValue={value as string} status={status} />;
+    } else if (tableType === OperatorExecStateTableType.Check) {
+      return <CheckTableItem checkValue={value as string} status={status} />;
+    }
+
+    // Default case, code here shouldn't get hit assuming this table is just used to render metrics and cheecks.
+    return (
+      <Typography
+        sx={{
+          fontWeight: 300,
+        }}
+      >
+        {value.toString()}
+      </Typography>
+    );
+  };
+
   return (
     <TableContainer sx={{ maxHeight, height, width }}>
       <Table
@@ -75,25 +107,12 @@ export const OperatorExecStateTable: React.FC<OperatorExecStateTableProps> = ({
                   const columnName = column.name.toLowerCase();
                   const value = row[columnName];
 
-                  // For title columns we should just render the text.
-                  // For a check's value column, we should render the appropriate icon.
                   return (
                     <TableCell
                       key={`cell-${rowIndex}-${columnIndex}`}
                       align={tableAlign as TableCellProps['align']}
                     >
-                      {tableType === OperatorExecStateTableType.Metric ||
-                      columnName === 'title' ? (
-                        <Typography
-                          sx={{
-                            fontWeight: columnName === 'title' ? 400 : 300,
-                          }}
-                        >
-                          {value.toString()}
-                        </Typography>
-                      ) : (
-                        <CheckTableItem checkValue={value as string} />
-                      )}
+                      {getTableItem(tableType, columnName, value, row.status)}
                     </TableCell>
                   );
                 })}

src/ui/common/src/components/workflows/artifact/check/history.tsx

@@ -5,7 +5,9 @@ import React from 'react';
 import { ArtifactResultsWithLoadingStatus } from '../../../../reducers/artifactResults';
 import { theme } from '../../../../styles/theme/theme';
 import { Data, DataSchema } from '../../../../utils/data';
-import ExecutionStatus from '../../../../utils/shared';
+import ExecutionStatus, {
+  stringToExecutionStatus,
+} from '../../../../utils/shared';
 import { isFailed, isInitial, isLoading } from '../../../../utils/shared';
 import { StatusIndicator } from '../../workflowStatus';
 
@@ -49,13 +51,20 @@ const CheckHistory: React.FC<CheckHistoryProps> = ({
     schema: checkHistorySchema,
     data: (historyWithLoadingStatus.results?.results ?? []).map(
       (artifactStatusResult) => {
+        let timestamp = new Date(
+          artifactStatusResult.exec_state?.timestamps?.finished_at
+        ).toLocaleString();
+
+        // Checks that are canceled / fail to execute have no exec_state or finished_at time.
+        if (timestamp === 'Invalid Date') {
+          timestamp = 'Unknown';
+        }
+
         return {
           status: artifactStatusResult.exec_state?.status ?? 'Unknown',
           level: checkLevel ? checkLevel : 'undefined',
           value: artifactStatusResult.content_serialized,
-          timestamp: new Date(
-            artifactStatusResult.exec_state?.timestamps?.finished_at
-          ).toLocaleString(),
+          timestamp,
         };
       }
     ),
@@ -103,7 +112,11 @@ const CheckHistory: React.FC<CheckHistoryProps> = ({
             width="auto"
           >
             <Box sx={{ display: 'flex', alignItems: 'center' }}>
-              <StatusIndicator status={entry.status as ExecutionStatus} />
+              <StatusIndicator
+                status={stringToExecutionStatus(entry.status as string)}
+                size={'16px'}
+                monochrome={false}
+              />
 
               <Typography sx={{ ml: 1 }} variant="body2">
                 {entry.timestamp.toLocaleString()}

src/ui/common/src/components/workflows/artifact/metric/history.tsx

@@ -49,16 +49,24 @@ const MetricsHistory: React.FC<Props> = ({ historyWithLoadingStatus }) => {
     schema: metricHistorySchema,
     data: (historyWithLoadingStatus.results?.results ?? []).map(
       (artifactStatusResult) => {
+        let timestamp = new Date(
+          artifactStatusResult.exec_state?.timestamps?.finished_at
+        ).toLocaleString();
+
+        // Metrics that are canceled / fail to execute have no exec_state, and thus no date.
+        if (timestamp === 'Invalid Date') {
+          timestamp = 'Unknown';
+        }
+
         return {
           status: artifactStatusResult.exec_state?.status ?? 'Unknown',
-          timestamp: new Date(
-            artifactStatusResult.exec_state?.timestamps?.finished_at
-          ).toLocaleString(),
+          timestamp,
           value: artifactStatusResult.content_serialized,
         };
       }
     ),
   };
+
   const dataSortedByLatest = historicalData.data.sort(
     (x, y) =>
       Date.parse(y['timestamp'] as string) -
@@ -144,10 +152,12 @@ const MetricsHistory: React.FC<Props> = ({ historyWithLoadingStatus }) => {
                 <StatusIndicator status={entry.status as ExecutionStatus} />
 
                 <Typography sx={{ ml: 1 }} variant="body2">
-                  {entry.timestamp.toLocaleString()}
+                  {entry.timestamp}
                 </Typography>
               </Box>
-              <Typography variant="body1">{entry.value.toString()}</Typography>
+              <Typography variant="body1">
+                {entry.value ? entry.value.toString() : '-'}
+              </Typography>
             </Box>
           );
         })}

src/ui/common/src/components/workflows/artifact/metricsAndChecksOverview.tsx

@@ -28,18 +28,21 @@ export const MetricsOverview: React.FC<MetricsOverviewProps> = ({
 }) => {
   const metricTableEntries = {
     schema: schema,
-    data: metrics
-      .map((metricArtf) => {
-        let name = metricArtf.name;
-        if (name.endsWith('artifact') || name.endsWith('Aritfact')) {
-          name = name.slice(0, 0 - 'artifact'.length);
-        }
-        return {
-          title: name,
-          value: metricArtf.result?.content_serialized,
-        };
-      })
-      .filter((x) => !!x.value),
+    data: metrics.map((metricArtf) => {
+      let title = metricArtf.name;
+      if (title.endsWith('artifact') || title.endsWith('Aritfact')) {
+        title = title.slice(0, 0 - 'artifact'.length);
+      }
+
+      const value = metricArtf.result?.content_serialized;
+      const status = metricArtf.result?.exec_state?.status;
+
+      return {
+        title,
+        value,
+        status,
+      };
+    }),
   };
 
   return (
@@ -74,18 +77,16 @@ export type ChecksOverviewProps = {
 export const ChecksOverview: React.FC<ChecksOverviewProps> = ({ checks }) => {
   const checkTableEntries = {
     schema: schema,
-    data: checks
-      .map((checkArtf) => {
-        let name = checkArtf.name;
-        if (name.endsWith('artifact') || name.endsWith('Aritfact')) {
-          name = name.slice(0, 0 - 'artifact'.length);
-        }
-        return {
-          title: name,
-          value: checkArtf.result?.content_serialized,
-        };
-      })
-      .filter((x) => !!x.value),
+    data: checks.map((checkArtf) => {
+      let name = checkArtf.name;
+      if (name.endsWith('artifact') || name.endsWith('Aritfact')) {
+        name = name.slice(0, 0 - 'artifact'.length);
+      }
+      return {
+        title: name,
+        value: checkArtf.result?.content_serialized,
+      };
+    }),
   };
 
   return (

src/ui/common/src/handlers/responses/dag.ts

@@ -57,20 +57,26 @@ export function getMetricsAndChecksOnArtifact(
       opResult.spec?.type === OperatorType.Check
   );
 
-  const metricsArtfIds = metricsOp.flatMap((opResult) =>
-    opResult !== undefined ? opResult.outputs : []
-  );
+  const metricsArtfIds = metricsOp.flatMap((opResult) => {
+    return opResult !== undefined ? opResult.outputs : [];
+  });
+
   const metricsArtf = metricsArtfIds.map((id) => dagResult.artifacts[id]);
   const metricsDownstreamIds = metricsArtf.flatMap(
     (artfResult) => artfResult.to
   );
+
   const metricsDownstreamOps = metricsDownstreamIds.map(
     (id) => dagResult.operators[id]
   );
+
   checksOp.push(...metricsDownstreamOps);
+
   const checksArtfIds = checksOp.flatMap((opResult) =>
     opResult !== undefined ? opResult.outputs : []
   );
+
   const checksArtf = checksArtfIds.map((id) => dagResult.artifacts[id]);
+
   return { checks: checksArtf, metrics: metricsArtf };
 }

src/ui/common/src/utils/data.ts

@@ -42,6 +42,7 @@ export type Data = {
   // each key of the row object corresponds to a column.
   // column names must be unique (obviously ;) )
   data: TableRow[];
+  status?: string;
 };
 
 export type DataPreviewLoadSpec = {