Merge pull request #89 from sartography/feature/refactor_error_messages_in_frontend

Feature/refactor error messages in frontend

spiffworkflow-frontend/src/App.tsx

@@ -14,7 +14,7 @@ import { ErrorForDisplay } from './interfaces';
 
 import { AbilityContext } from './contexts/Can';
 import UserService from './services/UserService';
-import { Notification } from './components/Notification';
+import ErrorDisplay from './components/ErrorDisplay';
 
 export default function App() {
   const [errorObject, setErrorObject] = useState<ErrorForDisplay | null>(null);
@@ -31,52 +31,6 @@ export default function App() {
 
   const ability = defineAbility(() => {});
 
-  let errorTag = null;
-  if (errorObject) {
-    let sentryLinkTag = null;
-    if (errorObject.sentry_link) {
-      sentryLinkTag = (
-        <span>
-          {
-            ': Find details about this error here (it may take a moment to become available): '
-          }
-          <a href={errorObject.sentry_link} target="_blank" rel="noreferrer">
-            {errorObject.sentry_link}
-          </a>
-        </span>
-      );
-    }
-
-    let message = <div>{errorObject.message}</div>;
-    let title = 'Error:';
-    if ('task_name' in errorObject && errorObject.task_name) {
-      title = `Error in python script:`;
-      message = (
-        <>
-          <br />
-          <div>
-            Task: {errorObject.task_name} ({errorObject.task_id})
-          </div>
-          <div>File name: {errorObject.file_name}</div>
-          <div>Line number in script task: {errorObject.line_number}</div>
-          <br />
-          <div>{errorObject.message}</div>
-        </>
-      );
-    }
-
-    errorTag = (
-      <Notification
-        title={title}
-        onClose={() => setErrorObject(null)}
-        type="error"
-      >
-        {message}
-        {sentryLinkTag}
-      </Notification>
-    );
-  }
-
   return (
     <div className="cds--white">
       {/* @ts-ignore */}
@@ -85,7 +39,7 @@ export default function App() {
           <BrowserRouter>
             <NavigationBar />
             <Content>
-              {errorTag}
+              <ErrorDisplay />
               <ErrorBoundary>
                 <Routes>
                   <Route path="/*" element={<HomePageRoutes />} />

spiffworkflow-frontend/src/components/ErrorDisplay.tsx

@@ -0,0 +1,55 @@
+import { useContext } from 'react';
+import ErrorContext from '../contexts/ErrorContext';
+import { Notification } from './Notification';
+
+export default function ErrorDisplay() {
+  const [errorObject, setErrorObject] = (useContext as any)(ErrorContext);
+
+  let errorTag = null;
+  if (errorObject) {
+    let sentryLinkTag = null;
+    if (errorObject.sentry_link) {
+      sentryLinkTag = (
+        <span>
+          {
+            ': Find details about this error here (it may take a moment to become available): '
+          }
+          <a href={errorObject.sentry_link} target="_blank" rel="noreferrer">
+            {errorObject.sentry_link}
+          </a>
+        </span>
+      );
+    }
+
+    let message = <div>{errorObject.message}</div>;
+    let title = 'Error:';
+    if ('task_name' in errorObject && errorObject.task_name) {
+      title = 'Error in python script:';
+      message = (
+        <>
+          <br />
+          <div>
+            Task: {errorObject.task_name} ({errorObject.task_id})
+          </div>
+          <div>File name: {errorObject.file_name}</div>
+          <div>Line number in script task: {errorObject.line_number}</div>
+          <br />
+          <div>{errorObject.message}</div>
+        </>
+      );
+    }
+
+    errorTag = (
+      <Notification
+        title={title}
+        onClose={() => setErrorObject(null)}
+        type="error"
+      >
+        {message}
+        {sentryLinkTag}
+      </Notification>
+    );
+  }
+
+  return errorTag;
+}

spiffworkflow-frontend/src/routes/ProcessModelShow.tsx

@@ -232,6 +232,15 @@ export default function ProcessModelShow() {
     isPrimaryBpmnFile: boolean
   ) => {
     const elements = [];
+
+    // So there is a bug in here. Since we use a react context for error messages, and since
+    // its provider wraps the entire app, child components will re-render when there is an
+    // error displayed. This is normally fine, but it interacts badly with the casl ability.can
+    // functionality. We have observed that permissionsLoaded is never set to false. So when
+    // you run a process and it fails, for example, process model show will re-render, the ability
+    // will be cleared out and it will start fetching permissions from the server, but this
+    // component still thinks permissionsLoaded is telling the truth (it says true, but it's actually false).
+    // The only bad effect that we know of is that the Edit icon becomes an eye icon even for admins.
     let icon = View;
     let actionWord = 'View';
     if (ability.can('PUT', targetUris.processModelFileCreatePath)) {
@@ -327,11 +336,7 @@ export default function ProcessModelShow() {
       let fileLink = null;
       const fileUrl = profileModelFileEditUrl(processModelFile);
       if (fileUrl) {
-        if (ability.can('GET', targetUris.processModelFileCreatePath)) {
-          fileLink = <Link to={fileUrl}>{processModelFile.name}</Link>;
-        } else {
-          fileLink = <span>{processModelFile.name}</span>;
-        }
+        fileLink = <Link to={fileUrl}>{processModelFile.name}</Link>;
       }
       constructedTag = (
         <TableRow key={processModelFile.name}>

spiffworkflow-frontend/tsconfig.json

@@ -1,6 +1,5 @@
 {
   "compilerOptions": {
-    "baseUrl": ".",
     "esModuleInterop": true,
     "forceConsistentCasingInFileNames": true,
     "jsx": "react-jsx",