Multiinstance task data fix (#1497)

* Revert "Fix process data get subprocess (#1493)"

This reverts commit 84feef321d.

* added process model so we can test fixing loading task data with mi w/ burnettk

* added test and a potential fix w/ burnettk

* pyl is passing

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

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* put back api yml description

---------

Co-authored-by: jasquat <jasquat@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

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

@@ -3,6 +3,7 @@ import os
 import uuid
 from typing import Any
 from typing import TypedDict
+from uuid import UUID
 
 import flask.wrappers
 import sentry_sdk
@@ -29,11 +30,9 @@ from spiffworkflow_backend.exceptions.error import HumanTaskAlreadyCompletedErro
 from spiffworkflow_backend.exceptions.error import HumanTaskNotFoundError
 from spiffworkflow_backend.exceptions.error import UserDoesNotHaveAccessToTaskError
 from spiffworkflow_backend.exceptions.process_entity_not_found_error import ProcessEntityNotFoundError
-from spiffworkflow_backend.models.bpmn_process import BpmnProcessModel
 from spiffworkflow_backend.models.db import db
 from spiffworkflow_backend.models.human_task import HumanTaskModel
 from spiffworkflow_backend.models.human_task_user import HumanTaskUserModel
-from spiffworkflow_backend.models.json_data import JsonDataModel
 from spiffworkflow_backend.models.principal import PrincipalModel
 from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
 from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus
@@ -136,35 +135,43 @@ def _process_data_fetcher(
     bpmn_process_guid: str | None = None,
     process_identifier: str | None = None,
 ) -> flask.wrappers.Response:
+    if process_identifier and bpmn_process_guid is None:
+        raise ApiError(
+            error_code="missing_required_parameter",
+            message="process_identifier was given but bpmn_process_guid was not. Both must be provided if either is required.",
+            status_code=404,
+        )
+    if process_identifier is None and bpmn_process_guid:
+        raise ApiError(
+            error_code="missing_required_parameter",
+            message="bpmn_process_guid was given but process_identifier was not. Both must be provided if either is required.",
+            status_code=404,
+        )
+
     process_instance = _find_process_instance_by_id_or_raise(process_instance_id)
-    if bpmn_process_guid is not None:
+    processor = ProcessInstanceProcessor(process_instance)
-        bpmn_process = BpmnProcessModel.query.filter_by(guid=bpmn_process_guid).first()
-    else:
-        bpmn_process = process_instance.bpmn_process
-    if bpmn_process is None:
-        raise ApiError(
-            error_code="bpmn_process_not_found",
-            message=f"Cannot find a bpmn process with guid '{bpmn_process_guid}' for process instance {process_instance.id}",
-            status_code=404,
-        )
 
-    bpmn_process_data = JsonDataModel.find_data_dict_by_hash(bpmn_process.json_data_hash)
+    bpmn_process_instance = processor.bpmn_process_instance
-    if bpmn_process_data is None:
+    bpmn_process_data = processor.get_data()
-        raise ApiError(
+    if process_identifier and bpmn_process_instance.spec.name != process_identifier:
-            error_code="bpmn_process_data_not_found",
+        bpmn_process_instance = processor.bpmn_process_instance.subprocesses.get(UUID(bpmn_process_guid))
-            message=f"Cannot find a bpmn process data with guid '{bpmn_process_guid}' for process instance {process_instance.id}",
+        if bpmn_process_instance is None:
-            status_code=404,
+            raise ApiError(
-        )
+                error_code="bpmn_process_not_found",
+                message=f"Cannot find a bpmn process with guid '{bpmn_process_guid}' for process instance {process_instance.id}",
+                status_code=404,
+            )
+        bpmn_process_data = bpmn_process_instance.data
 
-    data_objects = bpmn_process_data["data_objects"]
+    data_objects = bpmn_process_instance.spec.data_objects
     data_object = data_objects.get(process_data_identifier)
 
     if data_object is None:
         raise ApiError(
             error_code="data_object_not_found",
             message=(
-                f"Cannot find a data object with identifier '{process_data_identifier}' for bpmn process"
+                f"Cannot find a data object with identifier '{process_data_identifier}' for bpmn process '{process_identifier}'"
-                f" '{bpmn_process.bpmn_process_definition.bpmn_identifier}' in process instance {process_instance.id}"
+                f" in process instance {process_instance.id}"
             ),
             status_code=404,
         )

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

@@ -693,19 +693,28 @@ class ProcessInstanceProcessor:
             states_to_exclude_from_rehydration = ["COMPLETED", "ERROR"]
 
         task_list_by_hash = {t.guid: t for t in tasks}
-        parent_task_guids = []
+        task_guids_to_add = set()
         for task in tasks:
+            parent_guid = task.parent_guid()
             if task.state not in states_to_exclude_from_rehydration:
                 json_data_hashes.add(task.json_data_hash)
+                task_guids_to_add.add(task.guid)
 
                 # load parent task data to avoid certain issues that can arise from parallel branches
-                parent_guid = task.parent_guid()
                 if (
                     parent_guid in task_list_by_hash
                     and task_list_by_hash[parent_guid].state in states_to_exclude_from_rehydration
                 ):
                     json_data_hashes.add(task_list_by_hash[parent_guid].json_data_hash)
-                    parent_task_guids.append(parent_guid)
+                    task_guids_to_add.add(parent_guid)
+            elif (
+                parent_guid in task_list_by_hash
+                and "instance_map" in (task_list_by_hash[parent_guid].runtime_info or {})
+                and task_list_by_hash[parent_guid] not in states_to_exclude_from_rehydration
+            ):
+                # make sure we add task data for multi-instance tasks as well
+                json_data_hashes.add(task.json_data_hash)
+                task_guids_to_add.add(task.guid)
 
         json_data_records = JsonDataModel.query.filter(JsonDataModel.hash.in_(json_data_hashes)).all()  # type: ignore
         json_data_mappings = {}
@@ -718,7 +727,7 @@ class ProcessInstanceProcessor:
                 tasks_dict = spiff_bpmn_process_dict["subprocesses"][bpmn_subprocess_guid]["tasks"]
             tasks_dict[task.guid] = task.properties_json
             task_data = {}
-            if task.state not in states_to_exclude_from_rehydration or task.guid in parent_task_guids:
+            if task.guid in task_guids_to_add:
                 task_data = json_data_mappings[task.json_data_hash]
             tasks_dict[task.guid]["data"] = task_data
 

spiffworkflow-backend/tests/data/multiinstance_manual_task/multiinstance-manual-task.bpmn

@@ -0,0 +1,63 @@
+<?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:spiffworkflow="http://spiffworkflow.org/bpmn/schema/1.0/core" 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_multiinstance_manual_task_eayacuw" isExecutable="true">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>Flow_17db3yp</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:sequenceFlow id="Flow_17db3yp" sourceRef="StartEvent_1" targetRef="Activity_0wg6zvw" />
+    <bpmn:endEvent id="EndEvent_1">
+      <bpmn:incoming>Flow_12pkbxb</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="Flow_12pkbxb" sourceRef="manual_task" targetRef="EndEvent_1" />
+    <bpmn:manualTask id="manual_task" name="Manual Task">
+      <bpmn:extensionElements>
+        <spiffworkflow:instructionsForEndUser>{{ the_input_var }}</spiffworkflow:instructionsForEndUser>
+        <spiffworkflow:preScript />
+        <spiffworkflow:postScript>the_output_var = the_input_var</spiffworkflow:postScript>
+      </bpmn:extensionElements>
+      <bpmn:incoming>Flow_01p4bbz</bpmn:incoming>
+      <bpmn:outgoing>Flow_12pkbxb</bpmn:outgoing>
+      <bpmn:multiInstanceLoopCharacteristics spiffworkflow:scriptsOnInstances="true">
+        <bpmn:loopDataInputRef>the_input</bpmn:loopDataInputRef>
+        <bpmn:loopDataOutputRef>z</bpmn:loopDataOutputRef>
+        <bpmn:inputDataItem id="the_input_var" name="the_input_var" />
+        <bpmn:outputDataItem id="the_output_var" name="the_output_var" />
+      </bpmn:multiInstanceLoopCharacteristics>
+    </bpmn:manualTask>
+    <bpmn:sequenceFlow id="Flow_01p4bbz" sourceRef="Activity_0wg6zvw" targetRef="manual_task" />
+    <bpmn:scriptTask id="Activity_0wg6zvw">
+      <bpmn:incoming>Flow_17db3yp</bpmn:incoming>
+      <bpmn:outgoing>Flow_01p4bbz</bpmn:outgoing>
+      <bpmn:script>the_input = ['a', 'b', 'c']</bpmn:script>
+    </bpmn:scriptTask>
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_test_multistance_script_task_eayacuw">
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="82" y="159" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Event_14za570_di" bpmnElement="EndEvent_1">
+        <dc:Bounds x="432" y="159" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_0zqotmb_di" bpmnElement="manual_task">
+        <dc:Bounds x="270" y="137" width="100" height="80" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_05mv918_di" bpmnElement="Activity_0wg6zvw">
+        <dc:Bounds x="140" y="137" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="Flow_17db3yp_di" bpmnElement="Flow_17db3yp">
+        <di:waypoint x="118" y="177" />
+        <di:waypoint x="140" y="177" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_12pkbxb_di" bpmnElement="Flow_12pkbxb">
+        <di:waypoint x="370" y="177" />
+        <di:waypoint x="432" y="177" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="Flow_01p4bbz_di" bpmnElement="Flow_01p4bbz">
+        <di:waypoint x="240" y="177" />
+        <di:waypoint x="270" y="177" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>

spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py

@@ -5,7 +5,6 @@ import os
 import time
 from hashlib import sha256
 from typing import Any
-from unittest.mock import patch
 
 import flask
 import pytest
@@ -13,8 +12,6 @@ from flask.app import Flask
 from flask.testing import FlaskClient
 from SpiffWorkflow.util.task import TaskState  # type: ignore
 from spiffworkflow_backend.exceptions.process_entity_not_found_error import ProcessEntityNotFoundError
-from spiffworkflow_backend.models.bpmn_process import BpmnProcessModel
-from spiffworkflow_backend.models.bpmn_process_definition import BpmnProcessDefinitionModel
 from spiffworkflow_backend.models.db import db
 from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
 from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus
@@ -3326,55 +3323,6 @@ class TestProcessApi(BaseTest):
         assert response.json is not None
         assert response.json["process_data_value"] == "hey"
 
-    def test_process_data_show_with_sub_process(
-        self,
-        app: Flask,
-        client: FlaskClient,
-        with_db_and_bpmn_file_cleanup: None,
-        with_super_admin_user: UserModel,
-    ) -> None:
-        process_model = load_test_spec(
-            "test_group/with-service-task-call-activity-sub-process",
-            process_model_source_directory="with-service-task-call-activity-sub-process",
-        )
-        process_instance_one = self.create_process_instance_from_process_model(process_model)
-        processor = ProcessInstanceProcessor(process_instance_one)
-        connector_response = {
-            "body": '{"ok": true}',
-            "mimetype": "application/json",
-            "http_status": 200,
-            "operator_identifier": "http/GetRequestV2",
-        }
-        with patch("requests.post") as mock_post:
-            mock_post.return_value.status_code = 200
-            mock_post.return_value.ok = True
-            mock_post.return_value.text = json.dumps(connector_response)
-            processor.do_engine_steps(save=True)
-        self.complete_next_manual_task(processor, execution_mode="synchronous")
-        self.complete_next_manual_task(processor, execution_mode="synchronous", data={"firstName": "Chuck"})
-        assert process_instance_one.status == "complete"
-
-        process_identifier = "call_activity_sub_process"
-        bpmn_processes = (
-            BpmnProcessModel.query.join(
-                BpmnProcessDefinitionModel, BpmnProcessDefinitionModel.id == BpmnProcessModel.bpmn_process_definition_id
-            )
-            .filter(BpmnProcessDefinitionModel.bpmn_identifier == process_identifier)
-            .all()
-        )
-        assert len(bpmn_processes) == 1
-        bpmn_process = bpmn_processes[0]
-
-        response = client.get(
-            f"/v1.0/process-data/default/{self.modify_process_identifier_for_path_param(process_model.id)}/sub_level_data_object_three/"
-            f"{process_instance_one.id}?process_identifier={process_identifier}&bpmn_process_guid={bpmn_process.guid}",
-            headers=self.logged_in_headers(with_super_admin_user),
-        )
-
-        assert response.status_code == 200
-        assert response.json is not None
-        assert response.json["process_data_value"] == "d"
-
     def _setup_testing_instance(
         self,
         client: FlaskClient,

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

@@ -1018,3 +1018,38 @@ class TestProcessInstanceProcessor(BaseTest):
         assert human_task_one.task_guid != str(non_manual_spiff_task.id)
         with pytest.raises(TaskMismatchError):
             processor.complete_task(non_manual_spiff_task, human_task_one, user=process_instance.process_initiator)
+
+    def test_can_run_multiinstance_tasks_with_human_task(
+        self,
+        app: Flask,
+        client: FlaskClient,
+        with_db_and_bpmn_file_cleanup: None,
+    ) -> None:
+        process_model = load_test_spec(
+            process_model_id="group/multiinstance_manual_task",
+            process_model_source_directory="multiinstance_manual_task",
+        )
+        process_instance = self.create_process_instance_from_process_model(process_model=process_model)
+        processor = ProcessInstanceProcessor(process_instance)
+        processor.do_engine_steps(save=True)
+
+        process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
+        processor = ProcessInstanceProcessor(process_instance)
+        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))
+        processor.complete_task(spiff_manual_task, human_task_one, user=process_instance.process_initiator)
+
+        process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
+        processor = ProcessInstanceProcessor(process_instance)
+        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))
+        processor.complete_task(spiff_manual_task, human_task_one, user=process_instance.process_initiator)
+
+        process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first()
+        processor = ProcessInstanceProcessor(process_instance)
+        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))
+        processor.complete_task(spiff_manual_task, human_task_one, user=process_instance.process_initiator)
+
+        processor.do_engine_steps(save=True)
+        assert process_instance.status == "complete"