Feature/draft data in join table (#355)

* added a new model to store task draft data in a join table

* cleaned up using the join table for draft table w/ burnettk

* created new single migration for changes w/ burnettk

* added hidden form which autosaves without validations w/ burnettk

* change close button name since it does indeed save on close now

---------

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

spiffworkflow-backend/migrations/versions/64adf34a98db_.py

@@ -0,0 +1,54 @@
+"""empty message
+
+Revision ID: 64adf34a98db
+Revises: 881cdb50a567
+Create Date: 2023-06-27 15:13:03.219908
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import mysql
+
+# revision identifiers, used by Alembic.
+revision = '64adf34a98db'
+down_revision = '881cdb50a567'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_table('task_draft_data',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('process_instance_id', sa.Integer(), nullable=False),
+    sa.Column('task_definition_id_path', sa.String(length=255), nullable=False),
+    sa.Column('saved_form_data_hash', sa.String(length=255), nullable=True),
+    sa.ForeignKeyConstraint(['process_instance_id'], ['process_instance.id'], ),
+    sa.PrimaryKeyConstraint('id'),
+    sa.UniqueConstraint('process_instance_id', 'task_definition_id_path', name='process_instance_task_definition_unique')
+    )
+    with op.batch_alter_table('task_draft_data', schema=None) as batch_op:
+        batch_op.create_index(batch_op.f('ix_task_draft_data_process_instance_id'), ['process_instance_id'], unique=False)
+        batch_op.create_index(batch_op.f('ix_task_draft_data_saved_form_data_hash'), ['saved_form_data_hash'], unique=False)
+        batch_op.create_index(batch_op.f('ix_task_draft_data_task_definition_id_path'), ['task_definition_id_path'], unique=False)
+
+    with op.batch_alter_table('task', schema=None) as batch_op:
+        batch_op.drop_index('ix_task_saved_form_data_hash')
+        batch_op.drop_column('saved_form_data_hash')
+
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table('task', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('saved_form_data_hash', mysql.VARCHAR(collation='utf8mb4_0900_as_cs', length=255), nullable=True))
+        batch_op.create_index('ix_task_saved_form_data_hash', ['saved_form_data_hash'], unique=False)
+
+    with op.batch_alter_table('task_draft_data', schema=None) as batch_op:
+        batch_op.drop_index(batch_op.f('ix_task_draft_data_task_definition_id_path'))
+        batch_op.drop_index(batch_op.f('ix_task_draft_data_saved_form_data_hash'))
+        batch_op.drop_index(batch_op.f('ix_task_draft_data_process_instance_id'))
+
+    op.drop_table('task_draft_data')
+    # ### end Alembic commands ###

spiffworkflow-backend/src/spiffworkflow_backend/load_database_models.py

@@ -82,5 +82,8 @@ from spiffworkflow_backend.models.process_model_cycle import (
 from spiffworkflow_backend.models.typeahead import (
     TypeaheadModel,
 )  # noqa: F401
+from spiffworkflow_backend.models.task_draft_data import (
+    TaskDraftDataModel,
+)  # noqa: F401
 
 add_listeners()

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

@@ -63,7 +63,6 @@ class TaskModel(SpiffworkflowBaseDBModel):
 
     json_data_hash: str = db.Column(db.String(255), nullable=False, index=True)
     python_env_data_hash: str = db.Column(db.String(255), nullable=False, index=True)
-    saved_form_data_hash: str | None = db.Column(db.String(255), nullable=True, index=True)
 
     start_in_seconds: float | None = db.Column(db.DECIMAL(17, 6))
     end_in_seconds: float | None = db.Column(db.DECIMAL(17, 6))
@@ -91,11 +90,6 @@ class TaskModel(SpiffworkflowBaseDBModel):
     def json_data(self) -> dict:
         return JsonDataModel.find_data_dict_by_hash(self.json_data_hash)
 
-    def get_saved_form_data(self) -> dict | None:
-        if self.saved_form_data_hash is not None:
-            return JsonDataModel.find_data_dict_by_hash(self.saved_form_data_hash)
-        return None
-
 
 class Task:
     HUMAN_TASK_TYPES = ["User Task", "Manual Task"]

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

@@ -0,0 +1,38 @@
+from __future__ import annotations
+
+from dataclasses import dataclass
+
+from sqlalchemy import ForeignKey
+from sqlalchemy import UniqueConstraint
+
+from spiffworkflow_backend.models.db import SpiffworkflowBaseDBModel
+from spiffworkflow_backend.models.db import db
+from spiffworkflow_backend.models.json_data import JsonDataModel  # noqa: F401
+from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
+
+
+@dataclass
+class TaskDraftDataModel(SpiffworkflowBaseDBModel):
+    __tablename__ = "task_draft_data"
+    __table_args__ = (
+        UniqueConstraint(
+            "process_instance_id",
+            "task_definition_id_path",
+            name="process_instance_task_definition_unique",
+        ),
+    )
+
+    id: int = db.Column(db.Integer, primary_key=True)
+    process_instance_id: int = db.Column(
+        ForeignKey(ProcessInstanceModel.id), nullable=False, index=True  # type: ignore
+    )
+
+    # a colon delimited path of bpmn_process_definition_ids for a given task
+    task_definition_id_path: str = db.Column(db.String(255), nullable=False, index=True)
+
+    saved_form_data_hash: str | None = db.Column(db.String(255), nullable=True, index=True)
+
+    def get_saved_form_data(self) -> dict | None:
+        if self.saved_form_data_hash is not None:
+            return JsonDataModel.find_data_dict_by_hash(self.saved_form_data_hash)
+        return None

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

@@ -47,6 +47,7 @@ from spiffworkflow_backend.routes.process_api_blueprint import _find_principal_o
 from spiffworkflow_backend.routes.process_api_blueprint import _find_process_instance_by_id_or_raise
 from spiffworkflow_backend.routes.process_api_blueprint import _get_process_model
 from spiffworkflow_backend.services.authorization_service import AuthorizationService
+from spiffworkflow_backend.services.authorization_service import HumanTaskAlreadyCompletedError
 from spiffworkflow_backend.services.authorization_service import HumanTaskNotFoundError
 from spiffworkflow_backend.services.authorization_service import UserDoesNotHaveAccessToTaskError
 from spiffworkflow_backend.services.file_system_service import FileSystemService
@@ -285,8 +286,14 @@ def task_show(process_instance_id: int, task_guid: str = "next") -> flask.wrappe
     except UserDoesNotHaveAccessToTaskError:
         can_complete = False
 
+    task_draft_data = TaskService.task_draft_data_from_task_model(task_model, create_if_not_exists=True)
+
+    saved_form_data = None
+    if task_draft_data is not None:
+        saved_form_data = task_draft_data.get_saved_form_data()
+
     task_model.data = task_model.get_data()
-    task_model.saved_form_data = task_model.get_saved_form_data()
+    task_model.saved_form_data = saved_form_data
     task_model.process_model_display_name = process_model.display_name
     task_model.process_model_identifier = process_model.id
     task_model.typename = task_definition.typename
@@ -480,15 +487,23 @@ def task_save_draft(
             ),
             status_code=400,
         )
-    AuthorizationService.assert_user_can_complete_task(process_instance.id, task_guid, principal.user)
+
+    try:
+        AuthorizationService.assert_user_can_complete_task(process_instance.id, task_guid, principal.user)
+    except HumanTaskAlreadyCompletedError:
+        return make_response(jsonify({"ok": True}), 200)
+
     task_model = _get_task_model_from_guid_or_raise(task_guid, process_instance_id)
-    json_data_dict = TaskService.update_task_data_on_task_model_and_return_dict_if_updated(
+    task_draft_data = TaskService.task_draft_data_from_task_model(task_model, create_if_not_exists=True)
-        task_model, body, "saved_form_data_hash"
+
-    )
+    if task_draft_data is not None:
-    if json_data_dict is not None:
+        json_data_dict = TaskService.update_task_data_on_task_model_and_return_dict_if_updated(
-        JsonDataModel.insert_or_update_json_data_dict(json_data_dict)
+            task_draft_data, body, "saved_form_data_hash"
-        db.session.add(task_model)
+        )
-        db.session.commit()
+        if json_data_dict is not None:
+            JsonDataModel.insert_or_update_json_data_dict(json_data_dict)
+            db.session.add(task_draft_data)
+            db.session.commit()
 
     return Response(
         json.dumps(

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

@@ -42,6 +42,10 @@ class HumanTaskNotFoundError(Exception):
     pass
 
 
+class HumanTaskAlreadyCompletedError(Exception):
+    pass
+
+
 class UserDoesNotHaveAccessToTaskError(Exception):
     pass
 
@@ -342,13 +346,18 @@ class AuthorizationService:
         human_task = HumanTaskModel.query.filter_by(
             task_id=task_guid,
             process_instance_id=process_instance_id,
-            completed=False,
         ).first()
         if human_task is None:
             raise HumanTaskNotFoundError(
                 f"Could find an human task with task guid '{task_guid}' for process instance '{process_instance_id}'"
             )
 
+        if human_task.completed:
+            raise HumanTaskAlreadyCompletedError(
+                f"Human task with task guid '{task_guid}' for process instance '{process_instance_id}' has already"
+                " been completed"
+            )
+
         if user not in human_task.potential_owners:
             raise UserDoesNotHaveAccessToTaskError(
                 f"User {user.username} does not have access to update"

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

@@ -26,6 +26,7 @@ from spiffworkflow_backend.models.spec_reference import SpecReferenceNotFoundErr
 from spiffworkflow_backend.models.task import TaskModel  # noqa: F401
 from spiffworkflow_backend.models.task import TaskNotFoundError
 from spiffworkflow_backend.models.task_definition import TaskDefinitionModel
+from spiffworkflow_backend.models.task_draft_data import TaskDraftDataModel
 from spiffworkflow_backend.services.process_instance_tmp_service import ProcessInstanceTmpService
 
 
@@ -572,7 +573,9 @@ class TaskService:
         return (bpmn_processes, task_models)
 
     @classmethod
-    def full_bpmn_process_path(cls, bpmn_process: BpmnProcessModel) -> list[str]:
+    def full_bpmn_process_path(
+        cls, bpmn_process: BpmnProcessModel, definition_column: str = "bpmn_identifier"
+    ) -> list[str]:
         """Returns a list of bpmn process identifiers pointing the given bpmn_process."""
         bpmn_process_identifiers: list[str] = []
         if bpmn_process.guid:
@@ -586,10 +589,27 @@ class TaskService:
                 _task_models_of_parent_bpmn_processes,
             ) = TaskService.task_models_of_parent_bpmn_processes(task_model)
             for parent_bpmn_process in parent_bpmn_processes:
-                bpmn_process_identifiers.append(parent_bpmn_process.bpmn_process_definition.bpmn_identifier)
+                bpmn_process_identifiers.append(
-        bpmn_process_identifiers.append(bpmn_process.bpmn_process_definition.bpmn_identifier)
+                    getattr(parent_bpmn_process.bpmn_process_definition, definition_column)
+                )
+        bpmn_process_identifiers.append(getattr(bpmn_process.bpmn_process_definition, definition_column))
         return bpmn_process_identifiers
 
+    @classmethod
+    def task_draft_data_from_task_model(
+        cls, task_model: TaskModel, create_if_not_exists: bool = False
+    ) -> TaskDraftDataModel | None:
+        full_bpmn_process_id_path = cls.full_bpmn_process_path(task_model.bpmn_process, "id")
+        task_definition_id_path = f"{':'.join(map(str,full_bpmn_process_id_path))}:{task_model.task_definition_id}"
+        task_draft_data: TaskDraftDataModel | None = TaskDraftDataModel.query.filter_by(
+            process_instance_id=task_model.process_instance_id, task_definition_id_path=task_definition_id_path
+        ).first()
+        if task_draft_data is None and create_if_not_exists:
+            task_draft_data = TaskDraftDataModel(
+                process_instance_id=task_model.process_instance_id, task_definition_id_path=task_definition_id_path
+            )
+        return task_draft_data
+
     @classmethod
     def bpmn_process_for_called_activity_or_top_level_process(cls, task_model: TaskModel) -> BpmnProcessModel:
         """Returns either the bpmn process for the call activity calling the process or the top level bpmn process.

spiffworkflow-frontend/src/index.css

@@ -606,3 +606,7 @@ hr {
 .primary-file-text-suffix {
   font-style: italic;
 }
+
+#hidden-form-for-autosave {
+  display: none;
+}

spiffworkflow-frontend/src/routes/TaskShow.tsx

@@ -34,6 +34,8 @@ export default function TaskShow() {
   const [disabled, setDisabled] = useState(false);
 
   const [taskData, setTaskData] = useState<any>(null);
+  const [autosaveOnFormChanges, setAutosaveOnFormChanges] =
+    useState<boolean>(true);
 
   const { addError, removeError } = useAPIError();
 
@@ -58,28 +60,6 @@ export default function TaskShow() {
       if (!result.can_complete) {
         navigateToInterstitial(result);
       }
-
-      /*  Disable call to load previous tasks -- do not display menu.
-      const url = `/v1.0/process-instances/for-me/${modifyProcessIdentifierForPathParam(
-        result.process_model_identifier
-      )}/${params.process_instance_id}/task-info`;
-      // if user is unauthorized to get process-instance task-info then don't do anything
-      // Checking like this so we can dynamically create the url with the correct process model
-      //  instead of passing the process model identifier in through the params
-      HttpService.makeCallToBackend({
-        path: url,
-        successCallback: (tasks: any) => {
-          setDisabled(false);
-          setUserTasks(tasks);
-        },
-        onUnauthorized: () => {
-          setDisabled(false);
-        },
-        failureCallback: (error: any) => {
-          addError(error);
-        },
-      });
-      */
     };
     HttpService.makeCallToBackend({
       path: `/tasks/${params.process_instance_id}/${params.task_id}`,
@@ -94,22 +74,38 @@ export default function TaskShow() {
   // in order to implement a "Save and close" button. That button no longer saves (since we have auto-save), but the crazy
   // frontend code to support that Save and close button is here, in case we need to reference that someday:
   //   https://github.com/sartography/spiff-arena/blob/182f56a1ad23ce780e8f5b0ed00efac3e6ad117b/spiffworkflow-frontend/src/routes/TaskShow.tsx#L329
-  const autoSaveTaskData = (formData: any) => {
+  const autoSaveTaskData = (formData: any, successCallback?: Function) => {
+    let successCallbackToUse = successCallback;
+    if (!successCallbackToUse) {
+      successCallbackToUse = doNothing;
+    }
     HttpService.makeCallToBackend({
       path: `/tasks/${params.process_instance_id}/${params.task_id}/save-draft`,
       postBody: formData,
       httpMethod: 'POST',
-      successCallback: doNothing,
+      successCallback: successCallbackToUse,
       failureCallback: addError,
     });
   };
 
+  const sendAutosaveEvent = (eventDetails?: any) => {
+    (document.getElementById('hidden-form-for-autosave') as any).dispatchEvent(
+      new CustomEvent('submit', {
+        cancelable: true,
+        bubbles: true,
+        detail: eventDetails,
+      })
+    );
+  };
+
   const addDebouncedTaskDataAutoSave = useDebouncedCallback(
-    (value: string) => {
+    () => {
-      autoSaveTaskData(value);
+      if (autosaveOnFormChanges) {
+        sendAutosaveEvent();
+      }
     },
     // delay in ms
-    1000
+    500
   );
 
   const processSubmitResult = (result: any) => {
@@ -127,12 +123,25 @@ export default function TaskShow() {
     }
   };
 
+  const handleAutosaveFormSubmit = (formObject: any, event: any) => {
+    const dataToSubmit = formObject?.formData;
+    let successCallback = null;
+    if (event.detail && 'successCallback' in event.detail) {
+      successCallback = event.detail.successCallback;
+    }
+    autoSaveTaskData(
+      recursivelyChangeNullAndUndefined(dataToSubmit, null),
+      successCallback
+    );
+  };
+
   const handleFormSubmit = (formObject: any, _event: any) => {
     if (disabled) {
       return;
     }
 
     const dataToSubmit = formObject?.formData;
+
     if (!dataToSubmit) {
       navigate(`/tasks`);
       return;
@@ -349,7 +358,9 @@ export default function TaskShow() {
   };
 
   const handleCloseButton = () => {
-    navigate(`/tasks`);
+    setAutosaveOnFormChanges(false);
+    const successCallback = () => navigate(`/tasks`);
+    sendAutosaveEvent({ successCallback });
   };
 
   const formElement = () => {
@@ -404,9 +415,9 @@ export default function TaskShow() {
             onClick={handleCloseButton}
             disabled={disabled}
             kind="secondary"
-            title="Save changes without submitting."
+            title="Save data as draft and close the form."
           >
-            Close
+            Save and Close
           </Button>
         );
       }
@@ -437,16 +448,19 @@ export default function TaskShow() {
 
     const widgets = { typeahead: TypeaheadWidget };
 
+    // we are using two forms here so we can have one that validates data and one that does not.
+    // this allows us to autosave form data without extra attributes and without validations
+    // but still requires validations when the user submits the form that they can edit.
     return (
       <Grid fullWidth condensed>
         <Column sm={4} md={5} lg={8}>
           <Form
-            id="our-very-own-form"
+            id="form-to-submit"
             disabled={disabled}
             formData={taskData}
             onChange={(obj: any) => {
               setTaskData(obj.formData);
-              addDebouncedTaskDataAutoSave(obj.formData);
+              addDebouncedTaskDataAutoSave();
             }}
             onSubmit={handleFormSubmit}
             schema={jsonSchema}
@@ -458,6 +472,17 @@ export default function TaskShow() {
           >
             {reactFragmentToHideSubmitButton}
           </Form>
+          <Form
+            id="hidden-form-for-autosave"
+            formData={taskData}
+            onSubmit={handleAutosaveFormSubmit}
+            schema={jsonSchema}
+            uiSchema={formUiSchema}
+            widgets={widgets}
+            validator={validator}
+            noValidate
+            omitExtraData
+          />
         </Column>
       </Grid>
     );