Merge remote-tracking branch 'origin/main' into feature/call-activity-references

.github/workflows/backend_tests.yml

@@ -273,10 +273,10 @@ jobs:
           nox --force-color --session=coverage -- xml
 
       - name: Upload coverage report
-        uses: codecov/codecov-action@v3.1.0
+        uses: codecov/codecov-action@v3.1.3
 
       - name: SonarCloud Scan
-        uses: sonarsource/sonarcloud-github-action@master
+        uses: sonarsource/sonarcloud-github-action@v1.8
         # thought about just skipping dependabot
         # if: ${{ github.actor != 'dependabot[bot]' }}
         # but figured all pull requests seems better, since none of them will have access to sonarcloud.

.github/workflows/docker_image_for_main_builds.yml

@@ -65,7 +65,7 @@ jobs:
 
       - name: Write app version info
         working-directory: spiffworkflow-frontend
-        run: echo $DOCKER_METADATA_OUTPUT_JSON | jq '.labels' > version_info.json
+        run: echo "$DOCKER_METADATA_OUTPUT_JSON" | jq '.labels' > version_info.json
       - name: Build and push Frontend Docker image
         uses: docker/build-push-action@v4.0.0
         with:
@@ -109,7 +109,7 @@ jobs:
 
       - name: Write app version info
         working-directory: spiffworkflow-backend
-        run: echo $DOCKER_METADATA_OUTPUT_JSON | jq '.labels' > version_info.json
+        run: echo "$DOCKER_METADATA_OUTPUT_JSON" | jq '.labels' > version_info.json
       - name: Build and push Backend Docker image
         uses: docker/build-push-action@v4.0.0
         with:

spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py

@@ -49,6 +49,7 @@ class HumanTaskModel(SpiffworkflowBaseDBModel):
 
     # task_id came first which is why it's a string and task_model_id is the int and foreignkey
     task_model_id: int = db.Column(ForeignKey(TaskModel.id), nullable=True, index=True)  # type: ignore
+    task_model = relationship(TaskModel)
     task_id: str = db.Column(db.String(50))
     task_name: str = db.Column(db.String(255))
     task_title: str = db.Column(db.String(50))

spiffworkflow-backend/src/spiffworkflow_backend/models/task.py

@@ -3,6 +3,7 @@ import enum
 from dataclasses import dataclass
 from typing import Any
 from typing import Optional
+from typing import TYPE_CHECKING
 from typing import Union
 
 import marshmallow
@@ -18,6 +19,11 @@ from spiffworkflow_backend.models.db import SpiffworkflowBaseDBModel
 from spiffworkflow_backend.models.json_data import JsonDataModel
 from spiffworkflow_backend.models.task_definition import TaskDefinitionModel
 
+if TYPE_CHECKING:
+    from spiffworkflow_backend.models.human_task_user import (  # noqa: F401
+        HumanTaskModel,
+    )
+
 
 class TaskNotFoundError(Exception):
     pass
@@ -52,6 +58,7 @@ class TaskModel(SpiffworkflowBaseDBModel):
     guid: str = db.Column(db.String(36), nullable=False, unique=True)
     bpmn_process_id: int = db.Column(ForeignKey(BpmnProcessModel.id), nullable=False, index=True)  # type: ignore
     bpmn_process = relationship(BpmnProcessModel, back_populates="tasks")
+    human_tasks = relationship("HumanTaskModel", back_populates="task_model", cascade="delete")
     process_instance_id: int = db.Column(ForeignKey("process_instance.id"), nullable=False, index=True)
 
     # find this by looking up the "workflow_name" and "task_spec" from the properties_json
@@ -110,6 +117,7 @@ class Task:
         event_definition: Union[dict[str, Any], None] = None,
         call_activity_process_identifier: Optional[str] = None,
         calling_subprocess_task_id: Optional[str] = None,
+        error_message: Optional[str] = None,
     ):
         """__init__."""
         self.id = id
@@ -147,6 +155,7 @@ class Task:
         self.properties = properties  # Arbitrary extension properties from BPMN editor.
         if self.properties is None:
             self.properties = {}
+        self.error_message = error_message
 
     @property
     def serialized(self) -> dict[str, Any]:
@@ -183,6 +192,7 @@ class Task:
             "event_definition": self.event_definition,
             "call_activity_process_identifier": self.call_activity_process_identifier,
             "calling_subprocess_task_id": self.calling_subprocess_task_id,
+            "error_message": self.error_message,
         }
 
     @classmethod

spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py

@@ -405,14 +405,29 @@ def _interstitial_stream(process_instance_id: int) -> Generator[str, Optional[st
             reported_ids.append(spiff_task.id)
             yield f"data: {current_app.json.dumps(task)} \n\n"
         last_task = spiff_task
-        processor.do_engine_steps(execution_strategy_name="run_until_user_message")
+        try:
             processor.do_engine_steps(execution_strategy_name="one_at_a_time")
-        spiff_task = processor.next_task()
+            processor.do_engine_steps(execution_strategy_name="run_until_user_message")
+            processor.save()  # Fixme - maybe find a way not to do this on every loop?
+        except WorkflowTaskException as wfe:
+            api_error = ApiError.from_workflow_exception(
+                "engine_steps_error", "Failed complete an automated task.", exp=wfe
+            )
+            yield f"data: {current_app.json.dumps(api_error)} \n\n"
+        except Exception as e:
+            api_error = ApiError(
+                error_code="engine_steps_error",
+                message=f"Failed complete an automated task. Error was: {str(e)}",
+                status_code=400,
+            )
+            yield f"data: {current_app.json.dumps(api_error)} \n\n"
+
         # Note, this has to be done in case someone leaves the page,
         # which can otherwise cancel this function and leave completed tasks un-registered.
-        processor.save()  # Fixme - maybe find a way not to do this on every loop?
-    if len(reported_ids) == 0:
+        spiff_task = processor.next_task()
+
     # Always provide some response, in the event no instructions were provided.
+    if len(reported_ids) == 0:
         task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task())
         yield f"data: {current_app.json.dumps(task)} \n\n"
 

spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py

@@ -1741,8 +1741,8 @@ class ProcessInstanceProcessor:
     def next_task(self) -> SpiffTask:
         """Returns the next task that should be completed even if there are parallel tasks and multiple options are available.
 
-        If the process_instance is complete
-        it will return the final end task.
+        If the process_instance is complete it will return the final end task.
+        If the process_instance is in an error state it will return the task that is erroring.
         """
         # If the whole blessed mess is done, return the end_event task in the tree
         # This was failing in the case of a call activity where we have an intermediate EndEvent
@@ -1769,8 +1769,12 @@ class ProcessInstanceProcessor:
             waiting_tasks = self.bpmn_process_instance.get_tasks(TaskState.WAITING)
             if len(waiting_tasks) > 0:
                 return waiting_tasks[0]
-            else:
-                return  # We have not tasks to return.
+
+            # If there are no ready tasks, and not waiting tasks, return the latest error.
+            error_task = None
+            for task in SpiffTask.Iterator(self.bpmn_process_instance.task_tree, TaskState.ERROR):
+                error_task = task
+            return error_task
 
         # Get a list of all completed user tasks (Non engine tasks)
         completed_user_tasks = self.completed_user_tasks()

spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py

@@ -462,6 +462,12 @@ class ProcessInstanceService:
 
         serialized_task_spec = processor.serialize_task_spec(spiff_task.task_spec)
 
+        # Grab the last error message.
+        error_message = None
+        for event in processor.process_instance_model.process_instance_events:
+            for detail in event.error_details:
+                error_message = detail.message
+
         task = Task(
             spiff_task.id,
             spiff_task.task_spec.name,
@@ -479,6 +485,7 @@ class ProcessInstanceService:
             event_definition=serialized_task_spec.get("event_definition"),
             call_activity_process_identifier=call_activity_process_identifier,
             calling_subprocess_task_id=calling_subprocess_task_id,
+            error_message=error_message,
         )
 
         return task

spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py

@@ -634,7 +634,7 @@ class TaskService:
                 isinstance(exception, ApiError) and exception.error_code == "task_error"
             ):
                 task_line_number = exception.line_number
-                task_line_contents = exception.error_line
+                task_line_contents = exception.error_line[0:255]
                 task_trace = exception.task_trace
                 task_offset = exception.offset
 

spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py

@@ -311,6 +311,7 @@ class TestProcessInstanceProcessor(BaseTest):
         ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one)
         assert process_instance.status == "complete"
 
+    # this test has been failing intermittently for some time on windows, perhaps ever since it was first added
     def test_properly_resets_process_to_given_task_with_call_activity(
         self,
         app: Flask,
@@ -350,12 +351,14 @@ class TestProcessInstanceProcessor(BaseTest):
         ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one)
 
         processor.suspend()
-        task_model_to_reset_to = (
+        all_task_models_matching_top_level_subprocess_script = (
             TaskModel.query.join(TaskDefinitionModel)
             .filter(TaskDefinitionModel.bpmn_identifier == "top_level_subprocess_script")
             .order_by(TaskModel.id.desc())  # type: ignore
-            .first()
+            .all()
         )
+        assert len(all_task_models_matching_top_level_subprocess_script) == 1
+        task_model_to_reset_to = all_task_models_matching_top_level_subprocess_script[0]
         assert task_model_to_reset_to is not None
         ProcessInstanceProcessor.reset_process(process_instance, task_model_to_reset_to.guid)
 
@@ -364,7 +367,7 @@ class TestProcessInstanceProcessor(BaseTest):
         process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
         processor = ProcessInstanceProcessor(process_instance)
 
-        # make sure we reset to the task we expected
+        # make sure we did actually reset to the task we expected
         ready_or_waiting_tasks = processor.get_all_ready_or_waiting_tasks()
         top_level_subprocess_script_spiff_task = next(
             task for task in ready_or_waiting_tasks if task.task_spec.name == "top_level_subprocess_script"
@@ -373,7 +376,11 @@ class TestProcessInstanceProcessor(BaseTest):
         processor.resume()
         processor.do_engine_steps(save=True, execution_strategy_name="greedy")
 
+        # reload again, just in case, since the assertion where it says there should be 1 active_human_task
+        # is failing intermittently on windows, so just debugging.
+        process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
         assert len(process_instance.active_human_tasks) == 1
+
         human_task_one = process_instance.active_human_tasks[0]
         spiff_manual_task = processor.bpmn_process_instance.get_task_from_id(UUID(human_task_one.task_id))
         ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one)

spiffworkflow-frontend/src/components/ErrorDisplay.tsx

@@ -109,6 +109,7 @@ export default function ErrorDisplay() {
 
   if (errorObject) {
     const title = 'Error:';
+    window.scrollTo(0, 0); // Scroll back to the top of the page
 
     errorTag = (
       <Notification title={title} onClose={() => removeError()} type="error">

spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx

@@ -1190,7 +1190,7 @@ export default function ProcessInstanceListTable({
                         return null;
                       }}
                       placeholder="Start typing username"
-                      titleText="Process Initiator"
+                      titleText="Started By"
                       selectedItem={processInitiatorSelection}
                     />
                   );
@@ -1199,7 +1199,7 @@ export default function ProcessInstanceListTable({
                   <TextInput
                     id="process-instance-initiator-search"
                     placeholder="Enter username"
-                    labelText="Process Initiator"
+                    labelText="Started By"
                     invalid={processInitiatorNotFoundErrorText !== ''}
                     invalidText={processInitiatorNotFoundErrorText}
                     onChange={(event: any) => {
@@ -1365,68 +1365,30 @@ export default function ProcessInstanceListTable({
     const formatter =
       reportColumnFormatters[column.accessor] ?? defaultFormatter;
     const value = row[column.accessor];
-    const modifiedModelId = modifyProcessIdentifierForPathParam(
-      row.process_model_identifier
-    );
-    const navigateToProcessInstance = () => {
-      navigate(`${processInstanceShowPathPrefix}/${modifiedModelId}/${row.id}`);
-    };
-    const navigateToProcessModel = () => {
-      navigate(`/admin/process-models/${modifiedModelId}`);
-    };
 
     if (column.accessor === 'status') {
       return (
-        // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
-        <td
-          onClick={navigateToProcessInstance}
-          onKeyDown={navigateToProcessInstance}
-          data-qa={`process-instance-status-${value}`}
-        >
+        <td data-qa={`process-instance-status-${value}`}>
           {formatter(row, value)}
         </td>
       );
     }
     if (column.accessor === 'process_model_display_name') {
-      const pmStyle = { background: 'rgba(0, 0, 0, .02)' };
-      return (
-        // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
-        <td
-          style={pmStyle}
-          onClick={navigateToProcessModel}
-          onKeyDown={navigateToProcessModel}
-        >
-          {formatter(row, value)}
-        </td>
-      );
+      return <td> {formatter(row, value)} </td>;
     }
     if (column.accessor === 'waiting_for') {
-      return (
-        // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
-        <td
-          onClick={navigateToProcessInstance}
-          onKeyDown={navigateToProcessInstance}
-        >
-          {getWaitingForTableCellComponent(row)}
-        </td>
-      );
+      return <td> {getWaitingForTableCellComponent(row)} </td>;
     }
     if (column.accessor === 'updated_at_in_seconds') {
       return (
         <TableCellWithTimeAgoInWords
           timeInSeconds={row.updated_at_in_seconds}
-          onClick={navigateToProcessInstance}
-          onKeyDown={navigateToProcessInstance}
         />
       );
     }
     return (
       // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
-      <td
-        data-qa={`process-instance-show-link-${column.accessor}`}
-        onKeyDown={navigateToProcessInstance}
-        onClick={navigateToProcessInstance}
-      >
+      <td data-qa={`process-instance-show-link-${column.accessor}`}>
         {formatter(row, value)}
       </td>
     );
@@ -1484,9 +1446,22 @@ export default function ProcessInstanceListTable({
       }
 
       const rowStyle = { cursor: 'pointer' };
-
+      const modifiedModelId = modifyProcessIdentifierForPathParam(
+        row.process_model_identifier
+      );
+      const navigateToProcessInstance = () => {
+        navigate(
+          `${processInstanceShowPathPrefix}/${modifiedModelId}/${row.id}`
+        );
+      };
       return (
-        <tr style={rowStyle} key={row.id}>
+        // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
+        <tr
+          style={rowStyle}
+          key={row.id}
+          onClick={navigateToProcessInstance}
+          onKeyDown={navigateToProcessInstance}
+        >
           {currentRow}
         </tr>
       );

spiffworkflow-frontend/src/components/ProcessModelSearch.tsx

@@ -15,7 +15,7 @@ export default function ProcessModelSearch({
   processModels,
   selectedItem,
   onChange,
-  titleText = 'Process model',
+  titleText = 'Process',
 }: OwnProps) {
   const getParentGroupsDisplayName = (processModel: ProcessModel) => {
     if (processModel.parent_groups) {

spiffworkflow-frontend/src/interfaces.ts

@@ -81,6 +81,7 @@ export interface ProcessInstanceTask {
 
   potential_owner_usernames?: string;
   assigned_user_group_identifier?: string;
+  error_message?: string;
 }
 
 export interface ProcessReference {

spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx

@@ -10,6 +10,7 @@ import { getBasicHeaders } from '../services/HttpService';
 import InstructionsForEndUser from '../components/InstructionsForEndUser';
 import ProcessBreadcrumb from '../components/ProcessBreadcrumb';
 import { ProcessInstanceTask } from '../interfaces';
+import useAPIError from '../hooks/UseApiError';
 
 export default function ProcessInterstitial() {
   const [data, setData] = useState<any[]>([]);
@@ -20,6 +21,7 @@ export default function ProcessInterstitial() {
   const userTasks = useMemo(() => {
     return ['User Task', 'Manual Task'];
   }, []);
+  const { addError } = useAPIError();
 
   useEffect(() => {
     fetchEventSource(
@@ -27,9 +29,13 @@ export default function ProcessInterstitial() {
       {
         headers: getBasicHeaders(),
         onmessage(ev) {
-          const task = JSON.parse(ev.data);
-          setData((prevData) => [...prevData, task]);
-          setLastTask(task);
+          const retValue = JSON.parse(ev.data);
+          if ('error_code' in retValue) {
+            addError(retValue);
+          } else {
+            setData((prevData) => [...prevData, retValue]);
+            setLastTask(retValue);
+          }
         },
         onclose() {
           setState('CLOSED');
@@ -85,6 +91,8 @@ export default function ProcessInterstitial() {
         return <img src="/interstitial/waiting.png" alt="Waiting ...." />;
       case 'COMPLETED':
         return <img src="/interstitial/completed.png" alt="Completed" />;
+      case 'ERROR':
+        return <img src="/interstitial/errored.png" alt="Errored" />;
       default:
         return getStatus();
     }
@@ -104,6 +112,10 @@ export default function ProcessInterstitial() {
     if (shouldRedirect(myTask)) {
       return <div>Redirecting you to the next task now ...</div>;
     }
+    if (myTask.error_message) {
+      return <div>{myTask.error_message}</div>;
+    }
+
     return (
       <div>
         <InstructionsForEndUser task={myTask} />
@@ -147,7 +159,7 @@ export default function ProcessInterstitial() {
             <Column md={2} lg={4} sm={2}>
               Task: <em>{d.title}</em>
             </Column>
-            <Column md={6} lg={8} sm={4}>
+            <Column md={6} lg={6} sm={4}>
               {userMessage(d)}
             </Column>
           </Grid>

spiffworkflow-frontend/src/routes/TaskShow.tsx

@@ -117,10 +117,10 @@ export default function TaskShow() {
     const processResult = (result: ProcessInstanceTask) => {
       setTask(result);
       setDisabled(false);
-
       if (!result.can_complete) {
         navigateToInterstitial(result);
       }
+      window.scrollTo(0, 0); // Scroll back to the top of the page
 
       /*  Disable call to load previous tasks -- do not display menu.
       const url = `/v1.0/process-instances/for-me/${modifyProcessIdentifierForPathParam(