From 4f20f97317fed872ae5ed9914fbc93d4a0649b43 Mon Sep 17 00:00:00 2001
From: jasquat <2487833+jasquat@users.noreply.github.com>
Date: Thu, 7 Sep 2023 11:20:58 -0400
Subject: [PATCH] show an error message and render the remaining page if a bpmn
 diagram cannot load w/ burnettk (#458)

Co-authored-by: jasquat <jasquat@users.noreply.github.com>
---
 .../models/process_instance.py                |  2 +
 .../routes/process_instances_controller.py    | 14 +++--
 .../src/components/ErrorDisplay.tsx           | 14 ++++-
 .../src/components/Notification.tsx           | 49 ++++++++++++-----
 spiffworkflow-frontend/src/interfaces.ts      |  1 +
 .../src/routes/ProcessInstanceShow.tsx        | 55 +++++++++++++++----
 6 files changed, 104 insertions(+), 31 deletions(-)

diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py
index 7f7f3691a..bd5be1c06 100644
--- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py
@@ -98,6 +98,7 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
     last_milestone_bpmn_name: str = db.Column(db.String(255))
 
     bpmn_xml_file_contents: str | None = None
+    bpmn_xml_file_contents_retrieval_error: str | None = None
     process_model_with_diagram_identifier: str | None = None
 
     # full, none
@@ -109,6 +110,7 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
             "id": self.id,
             "bpmn_version_control_identifier": self.bpmn_version_control_identifier,
             "bpmn_version_control_type": self.bpmn_version_control_type,
+            "bpmn_xml_file_contents_retrieval_error": self.bpmn_xml_file_contents_retrieval_error,
             "bpmn_xml_file_contents": self.bpmn_xml_file_contents,
             "created_at_in_seconds": self.created_at_in_seconds,
             "end_in_seconds": self.end_in_seconds,
diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py
index 0ca0d96b3..cd96e9fc6 100644
--- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py
@@ -703,16 +703,20 @@ def _get_process_instance(
             name_of_file_with_diagram = process_model_with_diagram.primary_file_name
 
     if process_model_with_diagram and name_of_file_with_diagram:
+        bpmn_xml_file_contents = None
         if process_instance.bpmn_version_control_identifier == current_version_control_revision:
             bpmn_xml_file_contents = SpecFileService.get_data(
                 process_model_with_diagram, name_of_file_with_diagram
             ).decode("utf-8")
         else:
-            bpmn_xml_file_contents = GitService.get_instance_file_contents_for_revision(
-                process_model_with_diagram,
-                process_instance.bpmn_version_control_identifier,
-                file_name=name_of_file_with_diagram,
-            )
+            try:
+                bpmn_xml_file_contents = GitService.get_instance_file_contents_for_revision(
+                    process_model_with_diagram,
+                    process_instance.bpmn_version_control_identifier,
+                    file_name=name_of_file_with_diagram,
+                )
+            except GitCommandError as ex:
+                process_instance.bpmn_xml_file_contents_retrieval_error = str(ex)
         process_instance.bpmn_xml_file_contents = bpmn_xml_file_contents
 
     process_instance_as_dict = process_instance.serialized_with_metadata()
diff --git a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx
index 760094188..7a2d3acbf 100644
--- a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx
+++ b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx
@@ -7,6 +7,8 @@ import {
   TestCaseErrorDetails,
 } from '../interfaces';
 
+const defaultMessageClassName = 'failure-string';
+
 function errorDetailDisplay(
   errorObject: any,
   propertyName: string,
@@ -24,13 +26,21 @@ function errorDetailDisplay(
   return null;
 }
 
+export const errorForDisplayFromString = (errorMessage: string) => {
+  const errorForDisplay: ErrorForDisplay = {
+    message: errorMessage,
+    messageClassName: defaultMessageClassName,
+  };
+  return errorForDisplay;
+};
+
 export const errorForDisplayFromProcessInstanceErrorDetail = (
   processInstanceEvent: ProcessInstanceLogEntry,
   processInstanceErrorEventDetail: ProcessInstanceEventErrorDetail
 ) => {
   const errorForDisplay: ErrorForDisplay = {
     message: processInstanceErrorEventDetail.message,
-    messageClassName: 'failure-string',
+    messageClassName: defaultMessageClassName,
     task_name: processInstanceEvent.task_definition_name,
     task_id: processInstanceEvent.task_definition_identifier,
     line_number: processInstanceErrorEventDetail.task_line_number,
@@ -46,7 +56,7 @@ export const errorForDisplayFromTestCaseErrorDetails = (
 ) => {
   const errorForDisplay: ErrorForDisplay = {
     message: testCaseErrorDetails.error_messages.join('\n'),
-    messageClassName: 'failure-string',
+    messageClassName: defaultMessageClassName,
     task_name: testCaseErrorDetails.task_bpmn_name,
     task_id: testCaseErrorDetails.task_bpmn_identifier,
     line_number: testCaseErrorDetails.task_line_number,
diff --git a/spiffworkflow-frontend/src/components/Notification.tsx b/spiffworkflow-frontend/src/components/Notification.tsx
index 2a853ce2b..fc3c2f089 100644
--- a/spiffworkflow-frontend/src/components/Notification.tsx
+++ b/spiffworkflow-frontend/src/components/Notification.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useState } from 'react';
 import {
   Close,
   Checkmark,
@@ -11,8 +11,10 @@ import { Button } from '@carbon/react';
 type OwnProps = {
   title: string;
   children?: React.ReactNode;
-  onClose: Function;
+  onClose?: Function;
   type?: string;
+  hideCloseButton?: boolean;
+  allowTogglingFullMessage?: boolean;
 };
 
 export function Notification({
@@ -20,11 +22,17 @@ export function Notification({
   children,
   onClose,
   type = 'success',
+  hideCloseButton = false,
+  allowTogglingFullMessage = false,
 }: OwnProps) {
+  const [showMessage, setShowMessage] = useState<boolean>(
+    !allowTogglingFullMessage
+  );
   let iconComponent = <Checkmark className="notification-icon" />;
   if (type === 'error') {
     iconComponent = <Error className="notification-icon" />;
   }
+
   return (
     <div
       role="status"
@@ -34,19 +42,34 @@ export function Notification({
         <div className="cds--inline-notification__text-wrapper">
           {iconComponent}
           <div className="cds--inline-notification__title">{title}</div>
-          <div className="cds--inline-notification__subtitle">{children}</div>
+          {showMessage ? (
+            <div className="cds--inline-notification__subtitle">{children}</div>
+          ) : null}
         </div>
       </div>
-      <Button
-        data-qa="close-publish-notification"
-        renderIcon={Close}
-        iconDescription="Close Notification"
-        className="cds--inline-notification__close-button"
-        hasIconOnly
-        size="sm"
-        kind=""
-        onClick={onClose}
-      />
+      {hideCloseButton ? null : (
+        <Button
+          data-qa="close-publish-notification"
+          renderIcon={Close}
+          iconDescription="Close Notification"
+          className="cds--inline-notification__close-button"
+          hasIconOnly
+          size="sm"
+          kind=""
+          onClick={onClose}
+        />
+      )}
+      {allowTogglingFullMessage ? (
+        <Button
+          data-qa="close-publish-notification"
+          className="cds--inline-notification__close-button"
+          size="sm"
+          kind=""
+          onClick={() => setShowMessage(!showMessage)}
+        >
+          {showMessage ? 'Hide' : 'Details'}&nbsp;
+        </Button>
+      ) : null}
     </div>
   );
 }
diff --git a/spiffworkflow-frontend/src/interfaces.ts b/spiffworkflow-frontend/src/interfaces.ts
index 260de6f05..33ead303c 100644
--- a/spiffworkflow-frontend/src/interfaces.ts
+++ b/spiffworkflow-frontend/src/interfaces.ts
@@ -168,6 +168,7 @@ export interface ProcessInstance {
   end_in_seconds: number | null;
   process_initiator_username: string;
   bpmn_xml_file_contents?: string;
+  bpmn_xml_file_contents_retrieval_error?: string;
   created_at_in_seconds: number;
   updated_at_in_seconds: number;
   bpmn_version_control_identifier: string;
diff --git a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx b/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
index f2414df26..9eae75d54 100644
--- a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
+++ b/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
@@ -68,6 +68,11 @@ import ProcessInterstitial from '../components/ProcessInterstitial';
 import UserSearch from '../components/UserSearch';
 import ProcessInstanceLogList from '../components/ProcessInstanceLogList';
 import MessageInstanceList from '../components/MessageInstanceList';
+import {
+  childrenForErrorObject,
+  errorForDisplayFromString,
+} from '../components/ErrorDisplay';
+import { Notification } from '../components/Notification';
 
 type OwnProps = {
   variant: string;
@@ -1213,6 +1218,44 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
     );
   };
 
+  const diagramArea = (processModelId: string) => {
+    if (!processInstance) {
+      return null;
+    }
+
+    const detailsComponent = (
+      <>
+        {childrenForErrorObject(
+          errorForDisplayFromString(
+            processInstance.bpmn_xml_file_contents_retrieval_error || ''
+          )
+        )}
+      </>
+    );
+    return processInstance.bpmn_xml_file_contents_retrieval_error ? (
+      <Notification
+        title="Failed to load diagram"
+        type="error"
+        hideCloseButton
+        allowTogglingFullMessage
+      >
+        {detailsComponent}
+      </Notification>
+    ) : (
+      <>
+        <ReactDiagramEditor
+          processModelId={processModelId || ''}
+          diagramXML={processInstance.bpmn_xml_file_contents || ''}
+          fileName={processInstance.bpmn_xml_file_contents || ''}
+          tasks={tasks}
+          diagramType="readonly"
+          onElementClick={handleClickedDiagramTask}
+        />
+        <div id="diagram-container" />
+      </>
+    );
+  };
+
   if (processInstance && (tasks || tasksCallHadError) && permissionsLoaded) {
     const processModelId = unModifyProcessIdentifierForPathParam(
       params.process_model_id ? params.process_model_id : ''
@@ -1244,17 +1287,7 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
             <Tab disabled={!canViewMsgs}>Messages</Tab>
           </TabList>
           <TabPanels>
-            <TabPanel>
-              <ReactDiagramEditor
-                processModelId={processModelId || ''}
-                diagramXML={processInstance.bpmn_xml_file_contents || ''}
-                fileName={processInstance.bpmn_xml_file_contents || ''}
-                tasks={tasks}
-                diagramType="readonly"
-                onElementClick={handleClickedDiagramTask}
-              />
-              <div id="diagram-container" />
-            </TabPanel>
+            <TabPanel>{diagramArea(processModelId)}</TabPanel>
             <TabPanel>
               <ProcessInstanceLogList
                 variant={variant}