Merge pull request #146 from sartography/feature/task_data_api_refactor

Feature/task data api refactor

spiffworkflow-backend/src/spiffworkflow_backend/api.yml

@@ -1518,7 +1518,7 @@ paths:
                 items:
                   $ref: "#/components/schemas/Task"
 
-  /task-data/{modified_process_model_identifier}/{process_instance_id}:
+  /task-data/{modified_process_model_identifier}/{process_instance_id}/{spiff_step}:
     parameters:
       - name: modified_process_model_identifier
         in: path
@@ -1532,31 +1532,23 @@ paths:
         description: The unique id of an existing process instance.
         schema:
           type: integer
-      - name: all_tasks
-        in: query
-        required: false
-        description: If true, this wil return all tasks associated with the process instance and not just user tasks.
-        schema:
-          type: boolean
       - name: spiff_step
-        in: query
+        in: path
-        required: false
+        required: true
         description: If set will return the tasks as they were during a specific step of execution.
         schema:
           type: integer
     get:
+      operationId: spiffworkflow_backend.routes.tasks_controller.task_data_show
+      summary: Get task data for a single task in a spiff step.
       tags:
         - Process Instances
-      operationId: spiffworkflow_backend.routes.process_instances_controller.process_instance_task_list_with_task_data
-      summary: returns the list of all user tasks associated with process instance with the task data
       responses:
         "200":
           description: list of tasks
           content:
             application/json:
               schema:
-                type: array
-                items:
                 $ref: "#/components/schemas/Task"
 
   /task-data/{modified_process_model_identifier}/{process_instance_id}/{task_id}:
@@ -1579,6 +1571,12 @@ paths:
         description: The unique id of the task.
         schema:
           type: string
+      - name: spiff_step
+        in: query
+        required: false
+        description: If set will return the tasks as they were during a specific step of execution.
+        schema:
+          type: integer
     put:
       operationId: spiffworkflow_backend.routes.process_api_blueprint.task_data_update
       summary: Update the task data for requested instance and task

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

@@ -514,7 +514,6 @@ def process_instance_task_list_without_task_data_for_me(
         process_instance,
         all_tasks,
         spiff_step,
-        get_task_data=False,
     )
 
 
@@ -531,24 +530,6 @@ def process_instance_task_list_without_task_data(
         process_instance,
         all_tasks,
         spiff_step,
-        get_task_data=False,
-    )
-
-
-def process_instance_task_list_with_task_data(
-    modified_process_model_identifier: str,
-    process_instance_id: int,
-    all_tasks: bool = False,
-    spiff_step: int = 0,
-) -> flask.wrappers.Response:
-    """Process_instance_task_list_with_task_data."""
-    process_instance = _find_process_instance_by_id_or_raise(process_instance_id)
-    return process_instance_task_list(
-        modified_process_model_identifier,
-        process_instance,
-        all_tasks,
-        spiff_step,
-        get_task_data=True,
     )
 
 
@@ -557,7 +538,6 @@ def process_instance_task_list(
     process_instance: ProcessInstanceModel,
     all_tasks: bool = False,
     spiff_step: int = 0,
-    get_task_data: bool = False,
 ) -> flask.wrappers.Response:
     """Process_instance_task_list."""
     step_detail_query = db.session.query(SpiffStepDetailsModel).filter(
@@ -576,31 +556,15 @@ def process_instance_task_list(
 
     steps_by_id = {step_detail.task_id: step_detail for step_detail in step_details}
 
-    # FIXME: never evaluate task data in this call and instead create a new api getter
-    # that will return the task data for a given step only. We think processing this
-    # data is what is causing long load times on the processInstanceShowPage.
     subprocess_state_overrides = {}
     for step_detail in step_details:
         if step_detail.task_id in tasks:
-            task_data = (
-                step_detail.task_json["task_data"] | step_detail.task_json["python_env"]
-            )
-            if task_data is None:
-                task_data = {}
-            tasks[step_detail.task_id]["data"] = task_data
             tasks[step_detail.task_id]["state"] = Task.task_state_name_to_int(
                 step_detail.task_state
             )
         else:
             for subprocess_id, subprocess_info in subprocesses.items():
                 if step_detail.task_id in subprocess_info["tasks"]:
-                    task_data = (
-                        step_detail.task_json["task_data"]
-                        | step_detail.task_json["python_env"]
-                    )
-                    if task_data is None:
-                        task_data = {}
-                    subprocess_info["tasks"][step_detail.task_id]["data"] = task_data
                     subprocess_info["tasks"][step_detail.task_id]["state"] = (
                         Task.task_state_name_to_int(step_detail.task_state)
                     )
@@ -657,8 +621,6 @@ def process_instance_task_list(
             calling_subprocess_task_id=calling_subprocess_task_id,
             task_spiff_step=task_spiff_step,
         )
-        if get_task_data:
-            task.data = spiff_task.data
         tasks.append(task)
 
     return make_response(jsonify(tasks), 200)

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

@@ -36,6 +36,7 @@ from spiffworkflow_backend.models.human_task_user import HumanTaskUserModel
 from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
 from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus
 from spiffworkflow_backend.models.process_model import ProcessModelInfo
+from spiffworkflow_backend.models.spiff_step_details import SpiffStepDetailsModel
 from spiffworkflow_backend.models.task import Task
 from spiffworkflow_backend.models.user import UserModel
 from spiffworkflow_backend.routes.process_api_blueprint import (
@@ -171,6 +172,46 @@ def task_list_for_my_groups(
     )
 
 
+def task_data_show(
+    modified_process_model_identifier: str,
+    process_instance_id: int,
+    spiff_step: int = 0,
+) -> flask.wrappers.Response:
+    process_instance = _find_process_instance_by_id_or_raise(process_instance_id)
+    step_detail = (
+        db.session.query(SpiffStepDetailsModel)
+        .filter(
+            SpiffStepDetailsModel.process_instance_id == process_instance.id,
+            SpiffStepDetailsModel.spiff_step == spiff_step,
+        )
+        .first()
+    )
+
+    if step_detail is None:
+        raise ApiError(
+            error_code="spiff_step_for_proces_instance_not_found",
+            message=(
+                "The given spiff step for the given process instance could not be"
+                " found."
+            ),
+            status_code=400,
+        )
+
+    processor = ProcessInstanceProcessor(process_instance)
+    spiff_task = processor.__class__.get_task_by_bpmn_identifier(
+        step_detail.bpmn_task_identifier, processor.bpmn_process_instance
+    )
+    task_data = step_detail.task_json["task_data"] | step_detail.task_json["python_env"]
+    task = ProcessInstanceService.spiff_task_to_api_task(
+        processor,
+        spiff_task,
+        task_spiff_step=spiff_step,
+    )
+    task.data = task_data
+
+    return make_response(jsonify(task), 200)
+
+
 def _munge_form_ui_schema_based_on_hidden_fields_in_task_data(task: Task) -> None:
     if task.form_ui_schema is None:
         task.form_ui_schema = {}

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

@@ -2766,8 +2766,14 @@ class TestProcessApi(BaseTest):
             headers=self.logged_in_headers(with_super_admin_user),
         )
         assert response.status_code == 200
-        end = next(task for task in response.json if task["type"] == "End Event")
+        end_task = next(task for task in response.json if task["type"] == "End Event")
-        assert end["data"]["result"] == {"message": "message 1"}
+        response = client.get(
+            f"/v1.0/task-data/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}/{end_task['task_spiff_step']}",
+            headers=self.logged_in_headers(with_super_admin_user),
+        )
+        assert response.status_code == 200
+        task = response.json
+        assert task["data"]["result"] == {"message": "message 1"}
 
     def test_manual_complete_task(
         self,

spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx

@@ -17,7 +17,7 @@ export const useUriListForPermissions = () => {
       processInstanceResumePath: `/v1.0/process-instance-resume/${params.process_model_id}/${params.process_instance_id}`,
       processInstanceSuspendPath: `/v1.0/process-instance-suspend/${params.process_model_id}/${params.process_instance_id}`,
       processInstanceResetPath: `/v1.0/process-instance-reset/${params.process_model_id}/${params.process_instance_id}`,
-      processInstanceTaskListDataPath: `/v1.0/task-data/${params.process_model_id}/${params.process_instance_id}`,
+      processInstanceTaskDataPath: `/v1.0/task-data/${params.process_model_id}/${params.process_instance_id}`,
       processInstanceSendEventPath: `/v1.0/send-event/${params.process_model_id}/${params.process_instance_id}`,
       processInstanceCompleteTaskPath: `/v1.0/complete-task/${params.process_model_id}/${params.process_instance_id}`,
       processInstanceTaskListPath: `/v1.0/process-instances/${params.process_model_id}/${params.process_instance_id}/task-info`,

spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx

@@ -27,6 +27,7 @@ import {
   Modal,
   Dropdown,
   Stack,
+  Loading,
   // @ts-ignore
 } from '@carbon/react';
 import { Can } from '@casl/react';
@@ -65,8 +66,12 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
     useState<ProcessInstance | null>(null);
   const [tasks, setTasks] = useState<ProcessInstanceTask[] | null>(null);
   const [tasksCallHadError, setTasksCallHadError] = useState<boolean>(false);
-  const [taskToDisplay, setTaskToDisplay] = useState<object | null>(null);
+  const [taskToDisplay, setTaskToDisplay] =
+    useState<ProcessInstanceTask | null>(null);
   const [taskDataToDisplay, setTaskDataToDisplay] = useState<string>('');
+  const [showTaskDataLoading, setShowTaskDataLoading] =
+    useState<boolean>(false);
+
   const [processDataToDisplay, setProcessDataToDisplay] =
     useState<ProcessData | null>(null);
   const [editingTaskData, setEditingTaskData] = useState<boolean>(false);
@@ -99,7 +104,7 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
     [targetUris.messageInstanceListPath]: ['GET'],
     [targetUris.processInstanceActionPath]: ['DELETE'],
     [targetUris.processInstanceLogListPath]: ['GET'],
-    [targetUris.processInstanceTaskListDataPath]: ['GET', 'PUT'],
+    [targetUris.processInstanceTaskDataPath]: ['GET', 'PUT'],
     [targetUris.processInstanceSendEventPath]: ['POST'],
     [targetUris.processInstanceCompleteTaskPath]: ['POST'],
     [targetUris.processModelShowPath]: ['PUT'],
@@ -145,9 +150,7 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
         taskParams = `${taskParams}&spiff_step=${params.spiff_step}`;
       }
       let taskPath = '';
-      if (ability.can('GET', targetUris.processInstanceTaskListDataPath)) {
+      if (ability.can('GET', taskListPath)) {
-        taskPath = `${targetUris.processInstanceTaskListDataPath}${taskParams}`;
-      } else if (ability.can('GET', taskListPath)) {
         taskPath = `${taskListPath}${taskParams}`;
       }
       if (taskPath) {
@@ -557,11 +560,33 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
     return <div />;
   };
 
-  const initializeTaskDataToDisplay = (task: any) => {
+  const processTaskResult = (result: ProcessInstanceTask) => {
-    if (task == null) {
+    if (result == null) {
       setTaskDataToDisplay('');
     } else {
-      setTaskDataToDisplay(JSON.stringify(task.data, null, 2));
+      setTaskDataToDisplay(JSON.stringify(result.data, null, 2));
+    }
+    setShowTaskDataLoading(false);
+  };
+
+  const initializeTaskDataToDisplay = (task: ProcessInstanceTask | null) => {
+    if (
+      task &&
+      task.state === 'COMPLETED' &&
+      ability.can('GET', targetUris.processInstanceTaskDataPath)
+    ) {
+      setShowTaskDataLoading(true);
+      HttpService.makeCallToBackend({
+        path: `${targetUris.processInstanceTaskDataPath}/${task.task_spiff_step}`,
+        httpMethod: 'GET',
+        successCallback: processTaskResult,
+        failureCallback: (error: any) => {
+          setTaskDataToDisplay(`ERROR: ${error.message}`);
+          setShowTaskDataLoading(false);
+        },
+      });
+    } else {
+      setTaskDataToDisplay('');
     }
   };
 
@@ -668,7 +693,7 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
   const canEditTaskData = (task: any) => {
     return (
       processInstance &&
-      ability.can('PUT', targetUris.processInstanceTaskListDataPath) &&
+      ability.can('PUT', targetUris.processInstanceTaskDataPath) &&
       isCurrentTask(task) &&
       processInstance.status === 'suspended' &&
       showingLastSpiffStep()
@@ -742,8 +767,13 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
   const saveTaskDataResult = (_: any) => {
     setEditingTaskData(false);
     const dataObject = taskDataStringToObject(taskDataToDisplay);
-    const taskToDisplayCopy = { ...taskToDisplay, data: dataObject }; // spread operator
+    if (taskToDisplay) {
+      const taskToDisplayCopy: ProcessInstanceTask = {
+        ...taskToDisplay,
+        data: dataObject,
+      }; // spread operator
       setTaskToDisplay(taskToDisplayCopy);
+    }
     refreshPage();
   };
 
@@ -757,7 +787,7 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
     // taskToUse is copy of taskToDisplay, with taskDataToDisplay in data attribute
     const taskToUse: any = { ...taskToDisplay, data: taskDataToDisplay };
     HttpService.makeCallToBackend({
-      path: `${targetUris.processInstanceTaskListDataPath}/${taskToUse.id}`,
+      path: `${targetUris.processInstanceTaskDataPath}/${taskToUse.id}`,
       httpMethod: 'PUT',
       successCallback: saveTaskDataResult,
       failureCallback: addError,
@@ -901,6 +931,10 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
   };
 
   const taskDataContainer = () => {
+    let taskDataClassName = '';
+    if (taskDataToDisplay.startsWith('ERROR:')) {
+      taskDataClassName = 'failure-string';
+    }
     return editingTaskData ? (
       <Editor
         height={600}
@@ -910,7 +944,12 @@ export default function ProcessInstanceShow({ variant }: OwnProps) {
         onChange={(value) => setTaskDataToDisplay(value || '')}
       />
     ) : (
-      <pre>{taskDataToDisplay}</pre>
+      <>
+        {showTaskDataLoading ? (
+          <Loading className="some-class" withOverlay={false} small />
+        ) : null}
+        <pre className={taskDataClassName}>{taskDataToDisplay}</pre>
+      </>
     );
   };