feature/handle-null-bpmn-process-on-pi (#827)

* do not error out and allow process instances to recover if the bpmn_process is null but the definition is set w/ burnettk

* fixed another flakey test w/ burnettk

---------

Co-authored-by: jasquat <jasquat@users.noreply.github.com>

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

@@ -105,6 +105,18 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
 
     actions: dict | None = None
 
+    def spiffworkflow_fully_initialized(self) -> bool:
+        """We have created process definitions and processes.
+
+        Up until Dec 2023, we thought it was not possible for bpmn_process_definition_id to be populated and for
+        bpmn_process_id to be null. It is still not expected in normal operation, but if something really awful
+        happens while saving tasks to the database (we observed a case where the background processor was running
+        old code and thought it had a task.id column that actually didn't exist), it is possible for bpmn_process_id
+        to be null. In those cases, we basically treat things as if it is a fresh instance in terms of how we
+        generate the serialization to give to spiff lib.
+        """
+        return self.bpmn_process_definition_id is not None and self.bpmn_process_id is not None
+
     def serialized(self) -> dict[str, Any]:
         """Return object data in serializeable format."""
         return {

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

@@ -463,7 +463,7 @@ class ProcessInstanceProcessor:
         self.bpmn_definition_to_task_definitions_mappings: dict = {}
 
         subprocesses: IdToBpmnProcessSpecMapping | None = None
-        if process_instance_model.bpmn_process_definition_id is None:
+        if not process_instance_model.spiffworkflow_fully_initialized():
             (
                 bpmn_process_spec,
                 subprocesses,
@@ -780,7 +780,7 @@ class ProcessInstanceProcessor:
     ) -> tuple[BpmnWorkflow, dict, dict]:
         full_bpmn_process_dict = {}
         bpmn_definition_to_task_definitions_mappings: dict = {}
-        if process_instance_model.bpmn_process_definition_id is not None:
+        if process_instance_model.spiffworkflow_fully_initialized():
             # turn off logging to avoid duplicated spiff logs
             spiff_logger = logging.getLogger("spiff")
             original_spiff_logger_log_level = spiff_logger.level
@@ -981,7 +981,7 @@ class ProcessInstanceProcessor:
 
         Expects the calling method to commit it.
         """
-        if self.process_instance_model.bpmn_process_definition_id is not None:
+        if self.process_instance_model.spiffworkflow_fully_initialized():
             return None
 
         bpmn_dict = self.serialize()

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

@@ -6,6 +6,7 @@ from spiffworkflow_backend.data_migrations.process_instance_migrator import Proc
 from spiffworkflow_backend.data_migrations.version_1_3 import VersionOneThree
 from spiffworkflow_backend.models.db import db
 from spiffworkflow_backend.models.task import TaskModel  # noqa: F401
+from spiffworkflow_backend.models.task_definition import TaskDefinitionModel
 from spiffworkflow_backend.services.process_instance_processor import ProcessInstanceProcessor
 
 from tests.spiffworkflow_backend.helpers.base_test import BaseTest
@@ -51,7 +52,12 @@ class TestProcessInstanceMigrator(BaseTest):
         processor = ProcessInstanceProcessor(process_instance)
         processor.do_engine_steps(save=True)
 
-        task_model = TaskModel.query.filter_by(process_instance_id=process_instance.id).all()[-1]
+        task_model = (
+            TaskModel.query.filter_by(process_instance_id=process_instance.id)
+            .join(TaskDefinitionModel)
+            .filter(TaskDefinitionModel.bpmn_identifier == "finance_approval")
+            .first()
+        )
         assert task_model is not None
         assert task_model.parent_task_model() is not None
         assert task_model.parent_task_model().properties_json["last_state_change"] is not None