Merge pull request #146 from sartography/feature/task_data_api_refactor

Feature/task data api refactor
2023-02-23 17:10:47 -05:00 · 2023-02-23 17:10:47 -05:00 · 9dc34b3cb7
parent 1f3b5129fe 4f7a82f8d1
commit 9dc34b3cb7
6 changed files with 114 additions and 68 deletions
--- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml
+++ b/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,32 +1532,24 @@ 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
-        required: false
+        in: path
+        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"
+                $ref: "#/components/schemas/Task"

  /task-data/{modified_process_model_identifier}/{process_instance_id}/{task_id}:
    parameters:
@ -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
--- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py
+++ b/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)
--- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py
+++ b/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 = {}
--- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py
+++ b/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")
-        assert end["data"]["result"] == {"message": "message 1"}
+        end_task = next(task for task in response.json if task["type"] == "End Event")
+        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,
--- a/spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx
+++ b/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`,
--- a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx
+++ b/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)) {
-        taskPath = `${targetUris.processInstanceTaskListDataPath}${taskParams}`;
-      } else if (ability.can('GET', taskListPath)) {
+      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) => {
-    if (task == null) {
+  const processTaskResult = (result: ProcessInstanceTask) => {
+    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
-    setTaskToDisplay(taskToDisplayCopy);
+    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>
+      </>
    );
  };