Merge pull request #229 from sartography/feature/interstitial_improvements

Feature/interstitial improvements

spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py

@@ -18,13 +18,13 @@ def setup_database_uri(app: Flask) -> None:
     if app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_URI") is None:
         database_name = f"spiffworkflow_backend_{app.config['ENV_IDENTIFIER']}"
         if app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_TYPE") == "sqlite":
-            app.config["SQLALCHEMY_DATABASE_URI"] = (
-                f"sqlite:///{app.instance_path}/db_{app.config['ENV_IDENTIFIER']}.sqlite3"
-            )
+            app.config[
+                "SQLALCHEMY_DATABASE_URI"
+            ] = f"sqlite:///{app.instance_path}/db_{app.config['ENV_IDENTIFIER']}.sqlite3"
         elif app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_TYPE") == "postgres":
-            app.config["SQLALCHEMY_DATABASE_URI"] = (
-                f"postgresql://spiffworkflow_backend:spiffworkflow_backend@localhost:5432/{database_name}"
-            )
+            app.config[
+                "SQLALCHEMY_DATABASE_URI"
+            ] = f"postgresql://spiffworkflow_backend:spiffworkflow_backend@localhost:5432/{database_name}"
         else:
             # use pswd to trick flake8 with hardcoded passwords
             db_pswd = app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_PASSWORD")

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

@@ -127,9 +127,9 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
     def serialized_with_metadata(self) -> dict[str, Any]:
         process_instance_attributes = self.serialized
         process_instance_attributes["process_metadata"] = self.process_metadata
-        process_instance_attributes["process_model_with_diagram_identifier"] = (
-            self.process_model_with_diagram_identifier
-        )
+        process_instance_attributes[
+            "process_model_with_diagram_identifier"
+        ] = self.process_model_with_diagram_identifier
         return process_instance_attributes
 
     @property

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

@@ -20,6 +20,7 @@ from flask import make_response
 from flask import stream_with_context
 from flask.wrappers import Response
 from jinja2 import TemplateSyntaxError
+from SpiffWorkflow.bpmn.workflow import BpmnWorkflow  # type: ignore
 from SpiffWorkflow.exceptions import WorkflowTaskException  # type: ignore
 from SpiffWorkflow.task import Task as SpiffTask  # type: ignore
 from SpiffWorkflow.task import TaskState
@@ -385,21 +386,26 @@ def _render_instructions_for_end_user(task_model: TaskModel, extensions: Optiona
 
 def _interstitial_stream(process_instance: ProcessInstanceModel) -> Generator[str, Optional[str], None]:
     processor = ProcessInstanceProcessor(process_instance)
-    reported_ids = []  # bit of an issue with end tasks showing as getting completed twice.
-    spiff_task = processor.next_task()
+    reported_ids = []  # A list of all the ids reported by this endpoint so far.
+
+    def get_reportable_tasks() -> Any:
+        return processor.bpmn_process_instance.get_tasks(
+            TaskState.WAITING | TaskState.STARTED | TaskState.READY | TaskState.ERROR
+        )
+
+    tasks = get_reportable_tasks()
+    while True:
+        for spiff_task in tasks:
             task_model = TaskModel.query.filter_by(guid=str(spiff_task.id)).first()
-    last_task = None
-    while last_task != spiff_task:
-        task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task())
             extensions = TaskService.get_extensions_from_task_model(task_model)
             instructions = _render_instructions_for_end_user(task_model, extensions)
             if instructions and spiff_task.id not in reported_ids:
-            reported_ids.append(spiff_task.id)
+                task = ProcessInstanceService.spiff_task_to_api_task(processor, spiff_task)
                 task.properties = extensions
                 yield f"data: {current_app.json.dumps(task)} \n\n"
-        last_task = spiff_task
+                reported_ids.append(spiff_task.id)
+            if spiff_task.state == TaskState.READY:
                 try:
-            processor.do_engine_steps(execution_strategy_name="one_at_a_time")
                     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:
@@ -407,6 +413,7 @@ def _interstitial_stream(process_instance: ProcessInstanceModel) -> Generator[st
                         "engine_steps_error", "Failed complete an automated task.", exp=wfe
                     )
                     yield f"data: {current_app.json.dumps(api_error)} \n\n"
+                    return
                 except Exception as e:
                     api_error = ApiError(
                         error_code="engine_steps_error",
@@ -414,16 +421,32 @@ def _interstitial_stream(process_instance: ProcessInstanceModel) -> Generator[st
                         status_code=400,
                     )
                     yield f"data: {current_app.json.dumps(api_error)} \n\n"
+                    return
+        processor.bpmn_process_instance.refresh_waiting_tasks()
+        ready_engine_task_count = get_ready_engine_step_count(processor.bpmn_process_instance)
+        tasks = get_reportable_tasks()
+        if ready_engine_task_count == 0:
+            break  # No more tasks to report
 
+    task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task())
+    if task.id not in reported_ids:
+        yield f"data: {current_app.json.dumps(task)} \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.
         spiff_task = processor.next_task()
         task_model = TaskModel.query.filter_by(guid=str(spiff_task.id)).first()
 
-    # 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"
+
+def get_ready_engine_step_count(bpmn_process_instance: BpmnWorkflow) -> int:
+    return len(
+        list(
+            [
+                t
+                for t in bpmn_process_instance.get_tasks(TaskState.READY)
+                if bpmn_process_instance._is_engine_task(t.task_spec)
+            ]
+        )
+    )
 
 
 def _dequeued_interstitial_stream(process_instance_id: int) -> Generator[str, Optional[str], None]:

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

@@ -423,9 +423,9 @@ class ProcessInstanceProcessor:
         tld.process_instance_id = process_instance_model.id
 
         # we want this to be the fully qualified path to the process model including all group subcomponents
-        current_app.config["THREAD_LOCAL_DATA"].process_model_identifier = (
-            f"{process_instance_model.process_model_identifier}"
-        )
+        current_app.config[
+            "THREAD_LOCAL_DATA"
+        ].process_model_identifier = f"{process_instance_model.process_model_identifier}"
 
         self.process_instance_model = process_instance_model
         self.process_model_service = ProcessModelService()
@@ -585,9 +585,9 @@ class ProcessInstanceProcessor:
                 bpmn_subprocess_definition.bpmn_identifier
             ] = bpmn_process_definition_dict
             spiff_bpmn_process_dict["subprocess_specs"][bpmn_subprocess_definition.bpmn_identifier]["task_specs"] = {}
-            bpmn_subprocess_definition_bpmn_identifiers[bpmn_subprocess_definition.id] = (
-                bpmn_subprocess_definition.bpmn_identifier
-            )
+            bpmn_subprocess_definition_bpmn_identifiers[
+                bpmn_subprocess_definition.id
+            ] = bpmn_subprocess_definition.bpmn_identifier
 
         task_definitions = TaskDefinitionModel.query.filter(
             TaskDefinitionModel.bpmn_process_definition_id.in_(  # type: ignore

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

@@ -220,15 +220,16 @@ class TaskService:
         if task_model.state == "COMPLETED":
             event_type = ProcessInstanceEventType.task_completed.value
             timestamp = task_model.end_in_seconds or task_model.start_in_seconds or time.time()
-            process_instance_event, _process_instance_error_detail = (
-                ProcessInstanceTmpService.add_event_to_process_instance(
+            (
+                process_instance_event,
+                _process_instance_error_detail,
+            ) = ProcessInstanceTmpService.add_event_to_process_instance(
                 self.process_instance,
                 event_type,
                 task_guid=task_model.guid,
                 timestamp=timestamp,
                 add_to_db_session=False,
             )
-            )
             self.process_instance_events[task_model.guid] = process_instance_event
 
         self.update_bpmn_process(spiff_task.workflow, bpmn_process)
@@ -454,7 +455,6 @@ class TaskService:
                     spiff_task,
                     self.bpmn_definition_to_task_definitions_mappings,
                 )
-
             self.update_task_model(task_model, spiff_task)
             self.task_models[task_model.guid] = task_model
 

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

@@ -299,28 +299,35 @@ class RunUntilServiceTaskExecutionStrategy(ExecutionStrategy):
 
 
 class RunUntilUserTaskOrMessageExecutionStrategy(ExecutionStrategy):
-    """When you want to run tasks until you hit something to report to the end user."""
+    """When you want to run tasks until you hit something to report to the end user.
 
-    def get_engine_steps(self, bpmn_process_instance: BpmnWorkflow) -> list[SpiffTask]:
-        return list(
-            [
-                t
-                for t in bpmn_process_instance.get_tasks(TaskState.READY)
-                if t.task_spec.spec_type not in ["User Task", "Manual Task"]
-                and not (
-                    hasattr(t.task_spec, "extensions") and t.task_spec.extensions.get("instructionsForEndUser", None)
-                )
-            ]
-        )
+    Note that this will run at least one engine step if possible,
+    but will stop if it hits instructions after the first task.
+    """
 
     def spiff_run(self, bpmn_process_instance: BpmnWorkflow, exit_at: None = None) -> None:
-        engine_steps = self.get_engine_steps(bpmn_process_instance)
-        while engine_steps:
+        bpmn_process_instance.refresh_waiting_tasks()
+        engine_steps = self.get_ready_engine_steps(bpmn_process_instance)
+        if len(engine_steps) > 0:
+            self.delegate.will_complete_task(engine_steps[0])
+            engine_steps[0].run()
+            self.delegate.did_complete_task(engine_steps[0])
+
+        should_continue = True
+        bpmn_process_instance.refresh_waiting_tasks()
+        engine_steps = self.get_ready_engine_steps(bpmn_process_instance)
+        while engine_steps and should_continue:
             for task in engine_steps:
+                if hasattr(task.task_spec, "extensions") and task.task_spec.extensions.get(
+                    "instructionsForEndUser", None
+                ):
+                    should_continue = False
+                    break
                 self.delegate.will_complete_task(task)
                 task.run()
                 self.delegate.did_complete_task(task)
-            engine_steps = self.get_engine_steps(bpmn_process_instance)
+            bpmn_process_instance.refresh_waiting_tasks()
+            engine_steps = self.get_ready_engine_steps(bpmn_process_instance)
         self.delegate.after_engine_steps(bpmn_process_instance)
 
 

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

@@ -348,8 +348,13 @@ class TestProcessInstanceProcessor(BaseTest):
         assert len(process_instance.human_tasks) == 1
 
         spiff_manual_task = processor.bpmn_process_instance.get_task_from_id(UUID(human_task_one.task_id))
+        assert len(process_instance.active_human_tasks) == 1, "expected 1 active human task"
+
         ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one)
         assert len(process_instance.human_tasks) == 2, "expected 2 human tasks after first one is completed"
+        assert (
+            len(process_instance.active_human_tasks) == 1
+        ), "expected 1 active human tasks after 1st one is completed"
 
         # unnecessary lookup just in case on windows
         process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
@@ -358,8 +363,8 @@ class TestProcessInstanceProcessor(BaseTest):
         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)
         assert (
-            len(process_instance.active_human_tasks) == 0
-        ), "expected 0 active human tasks after 2nd one is completed"
+            len(process_instance.active_human_tasks) == 1
+        ), "expected 1 active human tasks after 2nd one is completed, as we have looped back around."
 
         processor.suspend()
 
@@ -372,7 +377,7 @@ class TestProcessInstanceProcessor(BaseTest):
         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
-        assert len(process_instance.human_tasks) == 2, "expected 2 human tasks before reset"
+        assert len(process_instance.human_tasks) == 3, "expected 3 human tasks before reset"
         ProcessInstanceProcessor.reset_process(process_instance, task_model_to_reset_to.guid)
         assert len(process_instance.human_tasks) == 2, "still expected 2 human tasks after reset"
 
@@ -380,7 +385,7 @@ class TestProcessInstanceProcessor(BaseTest):
         db.session.expire_all()
         assert (
             len(process_instance.human_tasks) == 2
-        ), "still expected 2 human tasks after reset and session expire_all"
+        ), "still expected 3 human tasks after reset and session expire_all"
 
         process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
         processor = ProcessInstanceProcessor(process_instance)

spiffworkflow-frontend/package-lock.json

@@ -38,7 +38,7 @@
         "bootstrap": "^5.2.0",
         "bpmn-js": "^9.3.2",
         "bpmn-js-properties-panel": "^1.10.0",
-        "bpmn-js-spiffworkflow": "sartography/bpmn-js-spiffworkflow#main",
+        "bpmn-js-spiffworkflow": "github:sartography/bpmn-js-spiffworkflow#main",
         "cookie": "^0.5.0",
         "craco": "^0.0.3",
         "cypress-slow-down": "^1.2.1",
@@ -8343,7 +8343,7 @@
     },
     "node_modules/bpmn-js-spiffworkflow": {
       "version": "0.0.8",
-      "resolved": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#5f2cb3d50b021dffce61fd6b2929ef5620dcdb2e",
+      "resolved": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#313969da1067fce0a51b152626a609a122697693",
       "license": "MIT",
       "dependencies": {
         "inherits": "^2.0.4",
@@ -38215,7 +38215,7 @@
       }
     },
     "bpmn-js-spiffworkflow": {
-      "version": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#5f2cb3d50b021dffce61fd6b2929ef5620dcdb2e",
+      "version": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#313969da1067fce0a51b152626a609a122697693",
       "from": "bpmn-js-spiffworkflow@sartography/bpmn-js-spiffworkflow#main",
       "requires": {
         "inherits": "^2.0.4",

spiffworkflow-frontend/package.json

@@ -33,7 +33,7 @@
     "bootstrap": "^5.2.0",
     "bpmn-js": "^9.3.2",
     "bpmn-js-properties-panel": "^1.10.0",
-    "bpmn-js-spiffworkflow": "sartography/bpmn-js-spiffworkflow#main",
+    "bpmn-js-spiffworkflow": "github:sartography/bpmn-js-spiffworkflow#main",
     "cookie": "^0.5.0",
     "craco": "^0.0.3",
     "cypress-slow-down": "^1.2.1",

spiffworkflow-frontend/src/components/InstructionsForEndUser.tsx

@@ -2,12 +2,19 @@ import React from 'react';
 // @ts-ignore
 import MDEditor from '@uiw/react-md-editor';
 
-export default function InstructionsForEndUser({ task }: any) {
+type OwnProps = {
+  task: any;
+  defaultMessage?: string;
+};
+
+export default function InstructionsForEndUser({
+  task,
+  defaultMessage = '',
+}: OwnProps) {
   if (!task) {
     return null;
   }
-  let instructions =
-    'There is no additional instructions or information for this task.';
+  let instructions = defaultMessage;
   let { properties } = task;
   if (!properties) {
     properties = task.extensions;

spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx

@@ -1408,6 +1408,7 @@ export default function ProcessInstanceListTable({
     );
   };
 
+  // eslint-disable-next-line sonarjs/cognitive-complexity
   const buildTable = () => {
     const headerLabels: Record<string, string> = {
       id: 'Id',
@@ -1449,11 +1450,11 @@ export default function ProcessInstanceListTable({
         buttonElement = (
           <Button
             kind={
-              hasAccessToCompleteTask && row.task_id ? 'secondary' : 'tertiary'
+              hasAccessToCompleteTask && row.task_id ? 'secondary' : 'ghost'
             }
             href={interstitialUrl}
           >
-            Go
+            {hasAccessToCompleteTask && row.task_id ? 'Go' : 'View'}
           </Button>
         );
         currentRow.push(<td>{buttonElement}</td>);

spiffworkflow-frontend/src/index.css

@@ -29,6 +29,10 @@
   border-color: none;
 }
 
+.cds--loading__stroke {
+  stroke: gray;
+}
+
 /* make this a little less prominent so the actual human beings completing tasks stand out */
 .system-user-log-entry {
   color: #B0B0B0;
@@ -424,3 +428,24 @@ svg.notification-icon {
   margin-bottom: 1em;
   font-weight: bold;
 }
+
+
+.user_instructions_0 {
+  filter: opacity(1);
+}
+
+.user_instructions_1 {
+  filter: opacity(90%);
+}
+
+.user_instructions_2 {
+  filter: opacity(70%);
+}
+
+.user_instructions_3 {
+  filter: opacity(50%);
+}
+
+.user_instructions_4 {
+  filter: opacity(30%);
+}

spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx

@@ -2,7 +2,7 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import { useNavigate, useParams } from 'react-router-dom';
 import { fetchEventSource } from '@microsoft/fetch-event-source';
 // @ts-ignore
-import { Loading, Grid, Column } from '@carbon/react';
+import { Loading, Grid, Column, Button } from '@carbon/react';
 import { BACKEND_BASE_URL } from '../config';
 import { getBasicHeaders } from '../services/HttpService';
 
@@ -35,7 +35,7 @@ export default function ProcessInterstitial() {
           if ('error_code' in retValue) {
             addError(retValue);
           } else {
-            setData((prevData) => [...prevData, retValue]);
+            setData((prevData) => [retValue, ...prevData]);
             setLastTask(retValue);
           }
         },
@@ -100,6 +100,23 @@ export default function ProcessInterstitial() {
     }
   };
 
+  const getReturnHomeButton = (index: number) => {
+    if (
+      index === 0 &&
+      ['WAITING', 'ERROR', 'LOCKED', 'COMPLETED'].includes(getStatus())
+    )
+      return (
+        <>
+          <br />
+          <br />
+          <Button kind="secondary" onClick={() => navigate(`/tasks`)}>
+            Return to Home
+          </Button>
+        </>
+      );
+    return '';
+  };
+
   function capitalize(str: string): string {
     if (str && str.length > 0) {
       return str.charAt(0).toUpperCase() + str.slice(1).toLowerCase();
@@ -109,7 +126,15 @@ export default function ProcessInterstitial() {
 
   const userMessage = (myTask: ProcessInstanceTask) => {
     if (!myTask.can_complete && userTasks.includes(myTask.type)) {
-      return <div>This next task must be completed by a different person.</div>;
+      return (
+        <>
+          <h4 className="heading-compact-01">Waiting on Someone Else</h4>
+          <p>
+            This next task is assigned to a different person or team. There is
+            no action for you take at this time.
+          </p>
+        </>
+      );
     }
     if (shouldRedirect(myTask)) {
       return <div>Redirecting you to the next task now ...</div>;
@@ -120,7 +145,10 @@ export default function ProcessInterstitial() {
 
     return (
       <div>
-        <InstructionsForEndUser task={myTask} />
+        <InstructionsForEndUser
+          task={myTask}
+          defaultMessage="There are no additional instructions or information for this task."
+        />
       </div>
     );
   };
@@ -147,7 +175,7 @@ export default function ProcessInterstitial() {
             ],
           ]}
         />
-        <div style={{ display: 'flex', alignItems: 'center' }}>
+        <div style={{ display: 'flex', alignItems: 'center', gap: '10px' }}>
           {getStatusImage()}
           <div>
             <h1 style={{ marginBottom: '0em' }}>
@@ -159,13 +187,19 @@ export default function ProcessInterstitial() {
         </div>
         <br />
         <br />
-        {data.map((d) => (
+        {data.map((d, index) => (
           <Grid fullWidth style={{ marginBottom: '1em' }}>
-            <Column md={2} lg={4} sm={2}>
-              Task: <em>{d.title}</em>
-            </Column>
             <Column md={6} lg={6} sm={4}>
+              <div
+                className={
+                  index < 4
+                    ? `user_instructions_${index}`
+                    : `user_instructions_4`
+                }
+              >
                 {userMessage(d)}
+              </div>
+              {getReturnHomeButton(index)}
             </Column>
           </Grid>
         ))}