From 2128fad5bc8436f6a1f0279dc307286848b73f66 Mon Sep 17 00:00:00 2001
From: Dan <daniel.h.funk@gmail.com>
Date: Wed, 3 May 2023 17:08:22 -0400
Subject: [PATCH] Test and updates to assure that when a task has a boundary
 event, and you return to that event, and then progress one step, you don't
 get stuck with a task that can't ever be completed. Let SpiffWorkflow
 determine what tasks we need to update in the DB using the task_state_change
 date on the tasks.

---
 .../services/process_instance_processor.py    | 43 +++++----
 .../boundary_event_reset.bpmn                 | 51 +++++++++++
 .../boundary_event_reset/sub_with_timer.bpmn  | 89 +++++++++++++++++++
 .../unit/test_process_instance_processor.py   | 47 ++++++++++
 4 files changed, 212 insertions(+), 18 deletions(-)
 create mode 100644 spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn
 create mode 100644 spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn

diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py
index 2ff469b3..4da0724f 100644
--- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py
@@ -1115,59 +1115,54 @@ class ProcessInstanceProcessor:
 
     def manual_complete_task(self, task_id: str, execute: bool) -> None:
         """Mark the task complete optionally executing it."""
-        spiff_tasks_updated = {}
         start_in_seconds = time.time()
         spiff_task = self.bpmn_process_instance.get_task_from_id(UUID(task_id))
         event_type = ProcessInstanceEventType.task_skipped.value
+        start_time = time.time()
+
         if execute:
             current_app.logger.info(
                 f"Manually executing Task {spiff_task.task_spec.name} of process"
                 f" instance {self.process_instance_model.id}"
             )
-            # Executing a subworkflow manually will restart its subprocess and allow stepping through it
+            # Executing a sub-workflow manually will restart its subprocess and allow stepping through it
             if isinstance(spiff_task.task_spec, SubWorkflowTask):
                 subprocess = self.bpmn_process_instance.get_subprocess(spiff_task)
                 # We have to get to the actual start event
-                for task in self.bpmn_process_instance.get_tasks(workflow=subprocess):
-                    task.complete()
-                    spiff_tasks_updated[task.id] = task
-                    if isinstance(task.task_spec, StartEvent):
+                for spiff_task in self.bpmn_process_instance.get_tasks(workflow=subprocess):
+                    spiff_task.run()
+                    if isinstance(spiff_task.task_spec, StartEvent):
                         break
             else:
-                spiff_task.complete()
-                spiff_tasks_updated[spiff_task.id] = spiff_task
-                for child in spiff_task.children:
-                    spiff_tasks_updated[child.id] = child
+                spiff_task.run()
             event_type = ProcessInstanceEventType.task_executed_manually.value
         else:
             spiff_logger = logging.getLogger("spiff")
             spiff_logger.info(f"Skipped task {spiff_task.task_spec.name}", extra=spiff_task.log_info())
-            spiff_task._set_state(TaskState.COMPLETED)
-            for child in spiff_task.children:
-                child.task_spec._update(child)
-                spiff_tasks_updated[child.id] = child
+            spiff_task.complete()
             spiff_task.workflow.last_task = spiff_task
-            spiff_tasks_updated[spiff_task.id] = spiff_task
-
         end_in_seconds = time.time()
 
         if isinstance(spiff_task.task_spec, EndEvent):
             for task in self.bpmn_process_instance.get_tasks(TaskState.DEFINITE_MASK, workflow=spiff_task.workflow):
                 task.complete()
-                spiff_tasks_updated[task.id] = task
 
         # A subworkflow task will become ready when its workflow is complete.  Engine steps would normally
         # then complete it, but we have to do it ourselves here.
         for task in self.bpmn_process_instance.get_tasks(TaskState.READY):
             if isinstance(task.task_spec, SubWorkflowTask):
                 task.complete()
-                spiff_tasks_updated[task.id] = task
 
         task_service = TaskService(
             process_instance=self.process_instance_model,
             serializer=self._serializer,
             bpmn_definition_to_task_definitions_mappings=self.bpmn_definition_to_task_definitions_mappings,
         )
+
+        spiff_tasks_updated = {}
+        for task in self.bpmn_process_instance.get_tasks():
+            if task.last_state_change > start_time:
+                spiff_tasks_updated[task.id] = task
         for updated_spiff_task in spiff_tasks_updated.values():
             (
                 bpmn_process,
@@ -1216,6 +1211,13 @@ class ProcessInstanceProcessor:
             raise TaskNotFoundError(
                 f"Cannot find a task with guid '{to_task_guid}' for process instance '{process_instance.id}'"
             )
+        # If this task model has a parent boundary event, reset to that point instead, so we can reset all the boundary timers, etc...
+        parent_id = to_task_model.properties_json.get('parent','')
+        parent = TaskModel.query.filter_by(guid=parent_id).first()
+        is_boundary_parent = False
+        if parent and parent.task_definition.typename == '_BoundaryEventParent':
+            to_task_model = parent
+            is_boundary_parent = True  # Will need to complete this task at the end so we are on the correct process.
 
         # NOTE: run ALL queries before making changes to ensure we get everything before anything changes
         parent_bpmn_processes, task_models_of_parent_bpmn_processes = TaskService.task_models_of_parent_bpmn_processes(
@@ -1320,6 +1322,11 @@ class ProcessInstanceProcessor:
         db.session.commit()
 
         processor = ProcessInstanceProcessor(process_instance)
+
+        # If this as a boundary event parent, run it, so we get back to an active task.
+        if is_boundary_parent:
+            processor.do_engine_steps(execution_strategy_name='one_at_a_time')
+
         processor.save()
         processor.suspend()
 
diff --git a/spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn b/spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn
new file mode 100644
index 00000000..01ce0c5a
--- /dev/null
+++ b/spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn
@@ -0,0 +1,51 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="Definitions_96f6665" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="3.0.0-dev">
+  <bpmn:process id="Process_Admin_Tools_Test" name="AdminToolsTest" isExecutable="true">
+    <bpmn:startEvent id="Event_17e2qgy">
+      <bpmn:outgoing>Flow_1ist4rn</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:sequenceFlow id="Flow_1ist4rn" sourceRef="Event_17e2qgy" targetRef="Activity_039a4i7" />
+    <bpmn:endEvent id="Event_1qodpuj">
+      <bpmn:incoming>Flow_1xbry1g</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="Flow_0vzi07z" sourceRef="Activity_039a4i7" targetRef="Activity_0sqxs4d" />
+    <bpmn:callActivity id="Activity_039a4i7" calledElement="Process_With_Timer">
+      <bpmn:incoming>Flow_1ist4rn</bpmn:incoming>
+      <bpmn:outgoing>Flow_0vzi07z</bpmn:outgoing>
+    </bpmn:callActivity>
+    <bpmn:sequenceFlow id="Flow_1xbry1g" sourceRef="Activity_0sqxs4d" targetRef="Event_1qodpuj" />
+    <bpmn:manualTask id="Activity_0sqxs4d" name="Final">
+      <bpmn:incoming>Flow_0vzi07z</bpmn:incoming>
+      <bpmn:outgoing>Flow_1xbry1g</bpmn:outgoing>
+    </bpmn:manualTask>
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_Admin_Tools_Test">
+      <bpmndi:BPMNShape id="Event_17e2qgy_di" bpmnElement="Event_17e2qgy">
+        <dc:Bounds x="352" y="152" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_02srymo_di" bpmnElement="Activity_039a4i7">
+        <dc:Bounds x="440" y="130" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_1qodpuj_di" bpmnElement="Event_1qodpuj">
+        <dc:Bounds x="742" y="152" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_1r3vbnd_di" bpmnElement="Activity_0sqxs4d">
+        <dc:Bounds x="600" y="130" width="100" height="80" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="Flow_1ist4rn_di" bpmnElement="Flow_1ist4rn">
+        <di:waypoint x="388" y="170" />
+        <di:waypoint x="440" y="170" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_0vzi07z_di" bpmnElement="Flow_0vzi07z">
+        <di:waypoint x="540" y="170" />
+        <di:waypoint x="600" y="170" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_1xbry1g_di" bpmnElement="Flow_1xbry1g">
+        <di:waypoint x="700" y="170" />
+        <di:waypoint x="742" y="170" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>
diff --git a/spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn b/spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn
new file mode 100644
index 00000000..adffceda
--- /dev/null
+++ b/spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn
@@ -0,0 +1,89 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="Definitions_96f6665" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="3.0.0-dev">
+  <bpmn:process id="Process_With_Timer" name="Process With Timer" isExecutable="true">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>Flow_1e5apvr</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:sequenceFlow id="Flow_1e5apvr" sourceRef="StartEvent_1" targetRef="manual_task_1" />
+    <bpmn:sequenceFlow id="Flow_0vtgres" sourceRef="manual_task_1" targetRef="Activity_2" />
+    <bpmn:endEvent id="Event_1pgaya7">
+      <bpmn:incoming>Flow_110vf76</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="Flow_110vf76" sourceRef="Activity_2" targetRef="Event_1pgaya7" />
+    <bpmn:boundaryEvent id="Timer_Event_Horror" name="Timer_Event_Horror" attachedToRef="manual_task_1">
+      <bpmn:outgoing>Flow_1hy0t7d</bpmn:outgoing>
+      <bpmn:timerEventDefinition id="TimerEventDefinition_1jkwn61">
+        <bpmn:timeDuration xsi:type="bpmn:tFormalExpression">'P14D'</bpmn:timeDuration>
+      </bpmn:timerEventDefinition>
+    </bpmn:boundaryEvent>
+    <bpmn:sequenceFlow id="Flow_1hy0t7d" sourceRef="Timer_Event_Horror" targetRef="Activity_3" />
+    <bpmn:endEvent id="Event_10frcbe">
+      <bpmn:incoming>Flow_1xbdri7</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="Flow_1xbdri7" sourceRef="Activity_3" targetRef="Event_10frcbe" />
+    <bpmn:manualTask id="manual_task_1" name="Manual Task #1">
+      <bpmn:incoming>Flow_1e5apvr</bpmn:incoming>
+      <bpmn:outgoing>Flow_0vtgres</bpmn:outgoing>
+    </bpmn:manualTask>
+    <bpmn:manualTask id="Activity_3" name="#3">
+      <bpmn:incoming>Flow_1hy0t7d</bpmn:incoming>
+      <bpmn:outgoing>Flow_1xbdri7</bpmn:outgoing>
+    </bpmn:manualTask>
+    <bpmn:scriptTask id="Activity_2" name="#2">
+      <bpmn:incoming>Flow_0vtgres</bpmn:incoming>
+      <bpmn:outgoing>Flow_110vf76</bpmn:outgoing>
+      <bpmn:script>y='1000'</bpmn:script>
+    </bpmn:scriptTask>
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_With_Timer">
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="179" y="159" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_1pgaya7_di" bpmnElement="Event_1pgaya7">
+        <dc:Bounds x="592" y="159" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_10frcbe_di" bpmnElement="Event_10frcbe">
+        <dc:Bounds x="592" y="282" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_1cl74h9_di" bpmnElement="manual_task_1">
+        <dc:Bounds x="270" y="137" width="100" height="80" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_0yn987b_di" bpmnElement="Activity_3">
+        <dc:Bounds x="430" y="260" width="100" height="80" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_19c5vp3_di" bpmnElement="Activity_2">
+        <dc:Bounds x="430" y="137" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_1gfn4de_di" bpmnElement="Timer_Event_Horror">
+        <dc:Bounds x="302" y="199" width="36" height="36" />
+        <bpmndi:BPMNLabel>
+          <dc:Bounds x="277" y="242" width="87" height="27" />
+        </bpmndi:BPMNLabel>
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="Flow_1e5apvr_di" bpmnElement="Flow_1e5apvr">
+        <di:waypoint x="215" y="177" />
+        <di:waypoint x="270" y="177" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_0vtgres_di" bpmnElement="Flow_0vtgres">
+        <di:waypoint x="370" y="177" />
+        <di:waypoint x="430" y="177" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_110vf76_di" bpmnElement="Flow_110vf76">
+        <di:waypoint x="530" y="177" />
+        <di:waypoint x="592" y="177" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_1hy0t7d_di" bpmnElement="Flow_1hy0t7d">
+        <di:waypoint x="320" y="235" />
+        <di:waypoint x="320" y="300" />
+        <di:waypoint x="430" y="300" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_1xbdri7_di" bpmnElement="Flow_1xbdri7">
+        <di:waypoint x="530" y="300" />
+        <di:waypoint x="592" y="300" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>
diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py
index bfda1eb3..2495a6b5 100644
--- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py
+++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py
@@ -434,6 +434,53 @@ class TestProcessInstanceProcessor(BaseTest):
 
         assert process_instance.status == "complete"
 
+    def test_properly_resets_process_on_tasks_with_boundary_events(
+            self,
+            app: Flask,
+            client: FlaskClient,
+            with_db_and_bpmn_file_cleanup: None,
+            with_super_admin_user: UserModel,
+    ) -> None:
+        self.create_process_group_with_api(client, with_super_admin_user, "test_group", "test_group")
+        process_model = load_test_spec(
+            process_model_id="test_group/boundary_event_reset",
+            process_model_source_directory="boundary_event_reset",
+        )
+        process_instance = self.create_process_instance_from_process_model(
+            process_model=process_model, user=with_super_admin_user
+        )
+        processor = ProcessInstanceProcessor(process_instance)
+        processor.do_engine_steps(save=True)
+        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, {}, with_super_admin_user, human_task_one)
+        assert (
+                len(process_instance.active_human_tasks) == 1
+        ), "expected 1 active human tasks after 2nd one is completed"
+        assert process_instance.active_human_tasks[0].task_title == 'Final'
+
+        # Reset the process back to the task within the call activity that contains a timer_boundary event.
+        reset_to_spiff_task = processor.__class__.get_task_by_bpmn_identifier(
+            'manual_task_1', processor.bpmn_process_instance
+        )
+        processor.suspend()
+        processor = ProcessInstanceProcessor(process_instance)
+        ProcessInstanceProcessor.reset_process(process_instance, str(reset_to_spiff_task.id))
+        human_task_one = process_instance.active_human_tasks[0]
+        assert human_task_one.task_title == 'Manual Task #1'
+        processor = ProcessInstanceProcessor(process_instance)
+        processor.manual_complete_task(str(spiff_manual_task.id), execute=True)
+        processor = ProcessInstanceProcessor(process_instance)
+        processor.resume()
+        processor.do_engine_steps(save=True)
+        process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
+
+        assert (len(process_instance.active_human_tasks) == 1)
+        assert process_instance.active_human_tasks[0].task_title == 'Final', \
+            "once we reset, resume, and complete the task, we should be back to the Final step again, and not" \
+            "stuck waiting for the call activity to complete (which was happening in a bug I'm fixing right now)"
+
     def test_properly_saves_tasks_when_running(
         self,
         app: Flask,