From a0966050474509dd50f23789dd7eb40348b5f942 Mon Sep 17 00:00:00 2001
From: jasquat <jasquat@users.noreply.github.com>
Date: Mon, 19 Dec 2022 11:54:22 -0500
Subject: [PATCH] do not allow editing task data for process instances that are
 not suspended and some code cleanup w/ burnettk

---
 spiffworkflow-backend/migrations/env.py       |   2 -
 .../models/process_instance.py                |   4 +
 .../routes/process_api_blueprint.py           |   8 ++
 .../services/git_service.py                   |   2 +-
 spiffworkflow-frontend/src/interfaces.ts      |   4 +
 .../src/routes/ProcessInstanceShow.tsx        | 122 +++++++++---------
 6 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/spiffworkflow-backend/migrations/env.py b/spiffworkflow-backend/migrations/env.py
index 68feded2..630e381a 100644
--- a/spiffworkflow-backend/migrations/env.py
+++ b/spiffworkflow-backend/migrations/env.py
@@ -1,5 +1,3 @@
-from __future__ import with_statement
-
 import logging
 from logging.config import fileConfig
 
diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py
index c89f457b..935eb209 100644
--- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py
@@ -26,6 +26,10 @@ class ProcessInstanceNotFoundError(Exception):
     """ProcessInstanceNotFoundError."""
 
 
+class ProcessInstanceTaskDataCannotBeUpdatedError(Exception):
+    """ProcessInstanceTaskDataCannotBeUpdatedError."""
+
+
 class NavigationItemSchema(Schema):
     """NavigationItemSchema."""
 
diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py
index 294a6524..f5a070b7 100644
--- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py
@@ -56,6 +56,9 @@ from spiffworkflow_backend.models.process_instance import ProcessInstanceApiSche
 from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
 from spiffworkflow_backend.models.process_instance import ProcessInstanceModelSchema
 from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus
+from spiffworkflow_backend.models.process_instance import (
+    ProcessInstanceTaskDataCannotBeUpdatedError,
+)
 from spiffworkflow_backend.models.process_instance_metadata import (
     ProcessInstanceMetadataModel,
 )
@@ -2110,6 +2113,11 @@ def update_task_data(
         ProcessInstanceModel.id == int(process_instance_id)
     ).first()
     if process_instance:
+        if process_instance.status != "suspended":
+            raise ProcessInstanceTaskDataCannotBeUpdatedError(
+                f"The process instance needs to be suspended to udpate the task-data. It is currently: {process_instance.status}"
+            )
+
         process_instance_bpmn_json_dict = json.loads(process_instance.bpmn_json)
         if "new_task_data" in body:
             new_task_data_str: str = body["new_task_data"]
diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py
index d94423a3..495603cf 100644
--- a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py
@@ -100,7 +100,7 @@ class GitService:
             branch_name_to_use,
             git_username,
             git_email,
-            current_app.config['GIT_USER_PASSWORD']
+            current_app.config["GIT_USER_PASSWORD"],
         ]
         return cls.run_shell_command_to_get_stdout(shell_command)
 
diff --git a/spiffworkflow-frontend/src/interfaces.ts b/spiffworkflow-frontend/src/interfaces.ts
index b0ab6208..7805249b 100644
--- a/spiffworkflow-frontend/src/interfaces.ts
+++ b/spiffworkflow-frontend/src/interfaces.ts
@@ -52,6 +52,10 @@ export interface ProcessInstance {
   id: number;
   process_model_identifier: string;
   process_model_display_name: string;
+  status: string;
+  start_in_seconds: number | null;
+  end_in_seconds: number | null;
+  bpmn_xml_file_contents?: string;
   spiff_step?: number;
 }
 
diff --git a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx b/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
index 88e5e3bb..119f7964 100644
--- a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
+++ b/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
@@ -183,29 +183,23 @@ export default function ProcessInstanceShow() {
     return taskIds;
   };
 
-  const currentSpiffStep = (processInstanceToUse: any) => {
-    if (typeof params.spiff_step === 'undefined') {
-      return processInstanceToUse.spiff_step;
+  const currentSpiffStep = () => {
+    if (processInstance && typeof params.spiff_step === 'undefined') {
+      return processInstance.spiff_step || 0;
     }
 
     return Number(params.spiff_step);
   };
 
-  const showingFirstSpiffStep = (processInstanceToUse: any) => {
-    return currentSpiffStep(processInstanceToUse) === 1;
+  const showingFirstSpiffStep = () => {
+    return currentSpiffStep() === 1;
   };
 
-  const showingLastSpiffStep = (processInstanceToUse: any) => {
-    return (
-      currentSpiffStep(processInstanceToUse) === processInstanceToUse.spiff_step
-    );
+  const showingLastSpiffStep = () => {
+    return processInstance && currentSpiffStep() === processInstance.spiff_step;
   };
 
-  const spiffStepLink = (
-    processInstanceToUse: any,
-    label: any,
-    distance: number
-  ) => {
+  const spiffStepLink = (label: any, distance: number) => {
     const processIdentifier = searchParams.get('process_identifier');
     let queryParams = '';
     if (processIdentifier) {
@@ -217,32 +211,35 @@ export default function ProcessInstanceShow() {
         data-qa="process-instance-step-link"
         to={`/admin/process-instances/${params.process_model_id}/${
           params.process_instance_id
-        }/${currentSpiffStep(processInstanceToUse) + distance}${queryParams}`}
+        }/${currentSpiffStep() + distance}${queryParams}`}
       >
         {label}
       </Link>
     );
   };
 
-  const previousStepLink = (processInstanceToUse: any) => {
-    if (showingFirstSpiffStep(processInstanceToUse)) {
+  const previousStepLink = () => {
+    if (showingFirstSpiffStep()) {
       return null;
     }
 
-    return spiffStepLink(processInstanceToUse, <CaretLeft />, -1);
+    return spiffStepLink(<CaretLeft />, -1);
   };
 
-  const nextStepLink = (processInstanceToUse: any) => {
-    if (showingLastSpiffStep(processInstanceToUse)) {
+  const nextStepLink = () => {
+    if (showingLastSpiffStep()) {
       return null;
     }
 
-    return spiffStepLink(processInstanceToUse, <CaretRight />, 1);
+    return spiffStepLink(<CaretRight />, 1);
   };
 
-  const getInfoTag = (processInstanceToUse: any) => {
+  const getInfoTag = () => {
+    if (!processInstance) {
+      return null;
+    }
     const currentEndDate = convertSecondsToFormattedDateTime(
-      processInstanceToUse.end_in_seconds
+      processInstance.end_in_seconds || 0
     );
     let currentEndDateTag;
     if (currentEndDate) {
@@ -253,7 +250,7 @@ export default function ProcessInstanceShow() {
           </Column>
           <Column sm={3} md={3} lg={3} className="grid-date">
             {convertSecondsToFormattedDateTime(
-              processInstanceToUse.end_in_seconds
+              processInstance.end_in_seconds || 0
             ) || 'N/A'}
           </Column>
         </Grid>
@@ -261,13 +258,13 @@ export default function ProcessInstanceShow() {
     }
 
     let statusIcon = <InProgress />;
-    if (processInstanceToUse.status === 'suspended') {
+    if (processInstance.status === 'suspended') {
       statusIcon = <PauseOutline />;
-    } else if (processInstanceToUse.status === 'complete') {
+    } else if (processInstance.status === 'complete') {
       statusIcon = <Checkmark />;
-    } else if (processInstanceToUse.status === 'terminated') {
+    } else if (processInstance.status === 'terminated') {
       statusIcon = <StopOutline />;
-    } else if (processInstanceToUse.status === 'error') {
+    } else if (processInstance.status === 'error') {
       statusIcon = <Warning />;
     }
 
@@ -279,7 +276,7 @@ export default function ProcessInstanceShow() {
           </Column>
           <Column sm={3} md={3} lg={3} className="grid-date">
             {convertSecondsToFormattedDateTime(
-              processInstanceToUse.start_in_seconds
+              processInstance.start_in_seconds || 0
             )}
           </Column>
         </Grid>
@@ -290,7 +287,7 @@ export default function ProcessInstanceShow() {
           </Column>
           <Column sm={3} md={3} lg={3}>
             <Tag type="gray" size="sm" className="span-tag">
-              {processInstanceToUse.status} {statusIcon}
+              {processInstance.status} {statusIcon}
             </Tag>
           </Column>
         </Grid>
@@ -333,11 +330,10 @@ export default function ProcessInstanceShow() {
     );
   };
 
-  const terminateButton = (processInstanceToUse: any) => {
+  const terminateButton = () => {
     if (
-      ['complete', 'terminated', 'error'].indexOf(
-        processInstanceToUse.status
-      ) === -1
+      processInstance &&
+      !['complete', 'terminated', 'error'].includes(processInstance.status)
     ) {
       return (
         <ButtonWithConfirmation
@@ -345,7 +341,7 @@ export default function ProcessInstanceShow() {
           renderIcon={StopOutline}
           iconDescription="Terminate"
           hasIconOnly
-          description={`Terminate Process Instance: ${processInstanceToUse.id}`}
+          description={`Terminate Process Instance: ${processInstance.id}`}
           onConfirmation={terminateProcessInstance}
           confirmButtonLabel="Terminate"
         />
@@ -354,11 +350,12 @@ export default function ProcessInstanceShow() {
     return <div />;
   };
 
-  const suspendButton = (processInstanceToUse: any) => {
+  const suspendButton = () => {
     if (
-      ['complete', 'terminated', 'error', 'suspended'].indexOf(
-        processInstanceToUse.status
-      ) === -1
+      processInstance &&
+      !['complete', 'terminated', 'error', 'suspended'].includes(
+        processInstance.status
+      )
     ) {
       return (
         <Button
@@ -374,8 +371,8 @@ export default function ProcessInstanceShow() {
     return <div />;
   };
 
-  const resumeButton = (processInstanceToUse: any) => {
-    if (processInstanceToUse.status === 'suspended') {
+  const resumeButton = () => {
+    if (processInstance && processInstance.status === 'suspended') {
       return (
         <Button
           onClick={resumeProcessInstance}
@@ -450,9 +447,11 @@ export default function ProcessInstanceShow() {
 
   const canEditTaskData = (task: any) => {
     return (
+      processInstance &&
       ability.can('PUT', targetUris.processInstanceTaskListDataPath) &&
       task.state === 'READY' &&
-      showingLastSpiffStep(processInstance as any)
+      processInstance.status === 'suspended' &&
+      showingLastSpiffStep()
     );
   };
 
@@ -475,7 +474,7 @@ export default function ProcessInstanceShow() {
   };
 
   const saveTaskDataFailure = (result: any) => {
-    setErrorMessage({ message: result.toString() });
+    setErrorMessage({ message: result.message });
   };
 
   const saveTaskData = () => {
@@ -591,35 +590,37 @@ export default function ProcessInstanceShow() {
     return null;
   };
 
-  const stepsElement = (processInstanceToUse: any) => {
+  const stepsElement = () => {
+    if (!processInstance) {
+      return null;
+    }
     return (
       <Grid condensed fullWidth>
         <Column sm={3} md={3} lg={3}>
           <Stack orientation="horizontal" gap={3} className="smaller-text">
-            {previousStepLink(processInstanceToUse)}
-            Step {currentSpiffStep(processInstanceToUse)} of{' '}
-            {processInstanceToUse.spiff_step}
-            {nextStepLink(processInstanceToUse)}
+            {previousStepLink()}
+            Step {currentSpiffStep()} of {processInstance.spiff_step}
+            {nextStepLink()}
           </Stack>
         </Column>
       </Grid>
     );
   };
 
-  const buttonIcons = (processInstanceToUse: any) => {
+  const buttonIcons = () => {
     const elements = [];
     if (
       ability.can('POST', `${targetUris.processInstanceActionPath}/terminate`)
     ) {
-      elements.push(terminateButton(processInstanceToUse));
+      elements.push(terminateButton());
     }
     if (
       ability.can('POST', `${targetUris.processInstanceActionPath}/suspend`)
     ) {
-      elements.push(suspendButton(processInstanceToUse));
+      elements.push(suspendButton());
     }
     if (ability.can('POST', `${targetUris.processInstanceActionPath}/resume`)) {
-      elements.push(resumeButton(processInstanceToUse));
+      elements.push(resumeButton());
     }
     if (ability.can('DELETE', targetUris.processInstanceActionPath)) {
       elements.push(
@@ -629,7 +630,7 @@ export default function ProcessInstanceShow() {
           renderIcon={TrashCan}
           iconDescription="Delete"
           hasIconOnly
-          description={`Delete Process Instance: ${processInstanceToUse.id}`}
+          description={`Delete Process Instance: ${processInstance}`}
           onConfirmation={deleteProcessInstance}
           confirmButtonLabel="Delete"
         />
@@ -639,7 +640,6 @@ export default function ProcessInstanceShow() {
   };
 
   if (processInstance && (tasks || tasksCallHadError)) {
-    const processInstanceToUse = processInstance as any;
     const taskIds = getTaskIds();
     const processModelId = unModifyProcessIdentifierForPathParam(
       params.process_model_id ? params.process_model_id : ''
@@ -655,26 +655,26 @@ export default function ProcessInstanceShow() {
               entityType: 'process-model-id',
               linkLastItem: true,
             },
-            [`Process Instance Id: ${processInstanceToUse.id}`],
+            [`Process Instance Id: ${processInstance.id}`],
           ]}
         />
         <Stack orientation="horizontal" gap={1}>
           <h1 className="with-icons">
-            Process Instance Id: {processInstanceToUse.id}
+            Process Instance Id: {processInstance.id}
           </h1>
-          {buttonIcons(processInstanceToUse)}
+          {buttonIcons()}
         </Stack>
         <br />
         <br />
-        {getInfoTag(processInstanceToUse)}
+        {getInfoTag()}
         <br />
         {taskDataDisplayArea()}
-        {stepsElement(processInstanceToUse)}
+        {stepsElement()}
         <br />
         <ReactDiagramEditor
           processModelId={processModelId || ''}
-          diagramXML={processInstanceToUse.bpmn_xml_file_contents || ''}
-          fileName={processInstanceToUse.bpmn_xml_file_contents || ''}
+          diagramXML={processInstance.bpmn_xml_file_contents || ''}
+          fileName={processInstance.bpmn_xml_file_contents || ''}
           readyOrWaitingProcessInstanceTasks={taskIds.readyOrWaiting}
           completedProcessInstanceTasks={taskIds.completed}
           diagramType="readonly"