Bug/predicted tasks dropped (#484)

* when removing predicted tasks for data storage, don't alter the in-memory version of the workflow.

* run_pyl

* Bump gitPython for security vulnerability.

poetry.lock

@@ -299,19 +299,22 @@ smmap = ">=3.0.1,<6"
 
 [[package]]
 name = "gitpython"
-version = "3.1.31"
+version = "3.1.36"
 description = "GitPython is a Python library used to interact with Git repositories"
 category = "dev"
 optional = false
 python-versions = ">=3.7"
 files = [
-    {file = "GitPython-3.1.31-py3-none-any.whl", hash = "sha256:f04893614f6aa713a60cbbe1e6a97403ef633103cdd0ef5eb6efe0deb98dbe8d"},
-    {file = "GitPython-3.1.31.tar.gz", hash = "sha256:8ce3bcf69adfdf7c7d503e78fd3b1c492af782d58893b650adb2ac8912ddd573"},
+    {file = "GitPython-3.1.36-py3-none-any.whl", hash = "sha256:8d22b5cfefd17c79914226982bb7851d6ade47545b1735a9d010a2a4c26d8388"},
+    {file = "GitPython-3.1.36.tar.gz", hash = "sha256:4bb0c2a6995e85064140d31a33289aa5dce80133a23d36fcd372d716c54d3ebf"},
 ]
 
 [package.dependencies]
 gitdb = ">=4.0.1,<5"
 
+[package.extras]
+test = ["black", "coverage[toml]", "ddt (>=1.1.1,!=1.4.3)", "mypy", "pre-commit", "pytest", "pytest-cov", "pytest-sugar", "virtualenv"]
+
 [[package]]
 name = "identify"
 version = "2.5.22"

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

@@ -259,7 +259,18 @@ class TaskService:
         This will NOT update start_in_seconds or end_in_seconds.
         It also returns the relating json_data object so they can be imported later.
         """
+
         new_properties_json = self.serializer.task_to_dict(spiff_task)
+
+        # Only save links to children that are definite - we don't currently store predicted
+        # children in the database.  They are filtered out in other places in the code, so we
+        # must remove references to them here.
+        modified_children = []
+        for child in spiff_task.children:
+            if not child._has_state(TaskState.PREDICTED_MASK):
+                modified_children.append(str(child.id))
+        new_properties_json["children"] = modified_children
+
         if new_properties_json["task_spec"] == "Start":
             new_properties_json["parent"] = None
         spiff_task_data = new_properties_json.pop("data")

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

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import concurrent.futures
-import copy
 import time
 from abc import abstractmethod
 from collections.abc import Callable
@@ -242,6 +241,28 @@ class TaskModelSavingDelegate(EngineStepDelegate):
             self.secondary_engine_step_delegate.did_complete_task(spiff_task)
 
     def save(self, bpmn_process_instance: BpmnWorkflow, _commit: bool = True) -> None:
+        # NOTE: process-all-tasks: All tests pass with this but it's less efficient and would be nice to replace
+        # excludes COMPLETED. the others were required to get PP1 to go to completion.
+        # process FUTURE tasks because Boundary events are not processed otherwise.
+        #
+        # ANOTHER NOTE: at one point we attempted to be smarter about what tasks we considered for persistence,
+        # but it didn't quite work in all cases, so we deleted it. you can find it in commit
+        # 1ead87b4b496525df8cc0e27836c3e987d593dc0 if you are curious.
+        for waiting_spiff_task in bpmn_process_instance.get_tasks(
+            TaskState.WAITING
+            | TaskState.CANCELLED
+            | TaskState.READY
+            | TaskState.MAYBE
+            | TaskState.LIKELY
+            | TaskState.FUTURE
+            | TaskState.STARTED
+            | TaskState.ERROR
+        ):
+            # these will be removed from the parent and then ignored
+            if waiting_spiff_task._has_state(TaskState.PREDICTED_MASK):
+                continue
+            self.task_service.update_task_model_with_spiff_task(waiting_spiff_task)
+
         self.task_service.save_objects_to_database()
 
         if self.secondary_engine_step_delegate:
@@ -249,35 +270,7 @@ class TaskModelSavingDelegate(EngineStepDelegate):
         db.session.commit()
 
     def after_engine_steps(self, bpmn_process_instance: BpmnWorkflow) -> None:
-        if self._should_update_task_model():
-            # NOTE: process-all-tasks: All tests pass with this but it's less efficient and would be nice to replace
-            # excludes COMPLETED. the others were required to get PP1 to go to completion.
-            # process FUTURE tasks because Boundary events are not processed otherwise.
-            #
-            # ANOTHER NOTE: at one point we attempted to be smarter about what tasks we considered for persistence,
-            # but it didn't quite work in all cases, so we deleted it. you can find it in commit
-            # 1ead87b4b496525df8cc0e27836c3e987d593dc0 if you are curious.
-            for waiting_spiff_task in bpmn_process_instance.get_tasks(
-                TaskState.WAITING
-                | TaskState.CANCELLED
-                | TaskState.READY
-                | TaskState.MAYBE
-                | TaskState.LIKELY
-                | TaskState.FUTURE
-                | TaskState.STARTED
-                | TaskState.ERROR
-            ):
-                # these will be removed from the parent and then ignored
-                if waiting_spiff_task._has_state(TaskState.PREDICTED_MASK):
-                    continue
-
-                # removing elements from an array causes the loop to exit so deep copy the array first
-                waiting_children = copy.copy(waiting_spiff_task.children)
-                for waiting_child in waiting_children:
-                    if waiting_child._has_state(TaskState.PREDICTED_MASK):
-                        waiting_spiff_task._children.remove(waiting_child.id)
-
-                self.task_service.update_task_model_with_spiff_task(waiting_spiff_task)
+        pass
 
     def on_exception(self, bpmn_process_instance: BpmnWorkflow) -> None:
         self.after_engine_steps(bpmn_process_instance)