#3669: fix pending brick status in extension overview (#3670)

src/pageEditor/hooks/useExtensionTrace.ts

@@ -15,12 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import {
-  getLatestRunByExtensionId,
-  TraceError,
-  TraceRecord,
-  TraceSuccess,
-} from "@/telemetry/trace";
+import { getLatestRunByExtensionId, TraceRecord } from "@/telemetry/trace";
 import useInterval from "@/hooks/useInterval";
 import { useDispatch, useSelector } from "react-redux";
 import runtimeSlice from "@/pageEditor/slices/runtimeSlice";
@@ -41,8 +36,7 @@ function selectTraceMetadata(record: TraceRecord) {
   return {
     runId: record.runId,
     timestamp: record.timestamp,
-    isError: Boolean((record as TraceError).error),
-    isSuccess: Boolean((record as TraceSuccess).output),
+    isFinal: record.isFinal,
   };
 }
 

src/pageEditor/tabs/editTab/editTabTypes.ts

@@ -39,6 +39,7 @@ export type FormikErrorTree = {
 
 export enum RunStatus {
   NONE,
+  PENDING,
   SUCCESS,
   SKIPPED,
   WARNING,

src/pageEditor/tabs/editTab/editorNodeLayout/EditorNodeLayout.tsx

@@ -24,14 +24,14 @@ import PipelineHeaderNode, {
 import PipelineFooterNode, {
   PipelineFooterNodeProps,
 } from "@/pageEditor/tabs/editTab/editorNodes/PipelineFooterNode";
-import { BlockPipeline } from "@/blocks/types";
+import { BlockConfig, BlockPipeline } from "@/blocks/types";
 import {
   BrickNodeContentProps,
   BrickNodeProps,
   FormikError,
   RunStatus,
 } from "@/pageEditor/tabs/editTab/editTabTypes";
-import { TraceError } from "@/telemetry/trace";
+import { TraceError, TraceRecord } from "@/telemetry/trace";
 import { IconProp } from "@fortawesome/fontawesome-svg-core";
 import { TypedBlockMap } from "@/blocks/registry";
 import { IBlock, OutputKey, UUID } from "@/core";
@@ -87,6 +87,45 @@ type SubPipeline = {
   subPipelinePath: string;
 };
 
+function decideBrickStatus({
+  index,
+  blockConfig,
+  pipelineErrors,
+  traceRecord,
+  errorTraceEntry,
+}: {
+  index: number;
+  blockConfig: BlockConfig;
+  pipelineErrors: FormikError;
+  traceRecord: TraceRecord;
+  errorTraceEntry: TraceError;
+}): RunStatus {
+  // If blockPipelineErrors is a string, it means the error is on the pipeline level
+  // eslint-disable-next-line security/detect-object-injection -- index is a number
+  if (typeof pipelineErrors !== "string" && Boolean(pipelineErrors?.[index])) {
+    return RunStatus.ERROR;
+  }
+
+  if (errorTraceEntry?.blockInstanceId === blockConfig.instanceId) {
+    return RunStatus.WARNING;
+  }
+
+  if (traceRecord?.skippedRun) {
+    return RunStatus.SKIPPED;
+  }
+
+  if (traceRecord == null) {
+    return RunStatus.NONE;
+  }
+
+  // We already checked for errors from pipelineErrors
+  if (traceRecord.isFinal) {
+    return RunStatus.SUCCESS;
+  }
+
+  return RunStatus.PENDING;
+}
+
 const EditorNodeLayout: React.FC<EditorNodeLayoutProps> = ({
   allBlocks,
   relevantBlocksForRootPipeline,
@@ -281,23 +320,15 @@ const EditorNodeLayout: React.FC<EditorNodeLayoutProps> = ({
       };
 
       if (block) {
-        const runStatus: RunStatus =
-          // If blockPipelineErrors is a string, it means the error is on the pipeline level
-          typeof pipelineErrors !== "string" &&
-          // eslint-disable-next-line security/detect-object-injection
-          Boolean(pipelineErrors?.[index])
-            ? RunStatus.ERROR
-            : errorTraceEntry?.blockInstanceId === blockConfig.instanceId
-            ? RunStatus.WARNING
-            : traceRecord?.skippedRun
-            ? RunStatus.SKIPPED
-            : traceRecord == null
-            ? RunStatus.NONE
-            : RunStatus.SUCCESS;
-
         contentProps = {
           icon: <BrickIcon brick={block} size="2x" inheritColor />,
-          runStatus,
+          runStatus: decideBrickStatus({
+            index,
+            blockConfig,
+            pipelineErrors,
+            traceRecord,
+            errorTraceEntry,
+          }),
           brickLabel: isNullOrBlank(blockConfig.label)
             ? block?.name
             : blockConfig.label,
@@ -479,9 +510,11 @@ const EditorNodeLayout: React.FC<EditorNodeLayoutProps> = ({
             );
           }
 
-          default:
+          default: {
             // Impossible code branch
-            return null;
+            // eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- dynamic check for never
+            throw new Error(`Unexpected type: ${type}`);
+          }
         }
       })}
     </ListGroup>

src/pageEditor/tabs/editTab/editorNodes/brickNode/BrickNodeContent.tsx

@@ -61,6 +61,9 @@ const BrickNodeContent: React.FC<BrickNodeContentProps> = ({
     case RunStatus.ERROR:
       badgeSource = "/img/fa-exclamation-circle-custom.svg";
       break;
+    case RunStatus.PENDING:
+      badgeSource = "/img/fa-circle-solid-loading-custom.svg";
+      break;
     default:
       badgeSource = null;
   }

src/runtime/pipelineTests/trace.test.ts

@@ -249,6 +249,7 @@ describe("Trace normal execution", () => {
       outputKey: undefined,
       output: { message: "hello" },
       skippedRun: false,
+      isFinal: true,
     };
 
     expect(traces.addEntry).toHaveBeenCalledTimes(1);
@@ -301,6 +302,7 @@ describe("Trace normal execution", () => {
       outputKey,
       output: { message: "hello" },
       skippedRun: false,
+      isFinal: true,
     };
 
     expect(traces.addExit).toHaveBeenCalledTimes(2);

src/runtime/reducePipeline.ts

@@ -506,6 +506,7 @@ export async function blockReducer(
     outputKey: blockConfig.outputKey,
     output: null,
     skippedRun: false,
+    isFinal: true,
   };
 
   if (
@@ -614,6 +615,7 @@ function throwBlockError(
     blockInstanceId: blockConfig.instanceId,
     error: serializeError(error),
     skippedRun: false,
+    isFinal: true,
   });
 
   if (blockConfig.onError?.alert) {

src/telemetry/trace.ts

@@ -95,6 +95,13 @@ export type TraceExitData = TraceRecordMeta &
      * If a condition was specified and not met, this is `true`, otherwise `false`.
      */
     skippedRun: boolean;
+
+    /**
+     * `true` if the brick was skipped or finished running. Introduced to avoid gotchas with effect bricks which
+     * produce a null/undefined result.
+     * @since 1.7.0
+     */
+    isFinal: boolean;
   };
 
 export type TraceRecord = TraceEntryData & Partial<TraceExitData>;