From 15b294710751398555a843b4bda26facf4fecb96 Mon Sep 17 00:00:00 2001 From: jasquat <2487833+jasquat@users.noreply.github.com> Date: Wed, 28 Jun 2023 12:53:39 -0400 Subject: [PATCH] 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 --- .../migrations/versions/64adf34a98db_.py | 54 +++++++++++ .../load_database_models.py | 3 + .../src/spiffworkflow_backend/models/task.py | 6 -- .../models/task_draft_data.py | 38 ++++++++ .../routes/tasks_controller.py | 33 +++++-- .../services/authorization_service.py | 11 ++- .../services/task_service.py | 26 +++++- spiffworkflow-frontend/src/index.css | 4 + .../src/routes/TaskShow.tsx | 89 ++++++++++++------- 9 files changed, 213 insertions(+), 51 deletions(-) create mode 100644 spiffworkflow-backend/migrations/versions/64adf34a98db_.py create mode 100644 spiffworkflow-backend/src/spiffworkflow_backend/models/task_draft_data.py diff --git a/spiffworkflow-backend/migrations/versions/64adf34a98db_.py b/spiffworkflow-backend/migrations/versions/64adf34a98db_.py new file mode 100644 index 000000000..49ee592d2 --- /dev/null +++ b/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 ### diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/load_database_models.py b/spiffworkflow-backend/src/spiffworkflow_backend/load_database_models.py index 0553b1c2f..2e301fd17 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/load_database_models.py +++ b/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() diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py index 8e9d6359f..2cf6d80e5 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py +++ b/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"] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/task_draft_data.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/task_draft_data.py new file mode 100644 index 000000000..8499779ee --- /dev/null +++ b/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 diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index 2427c10b5..4635e1a72 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/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_model, body, "saved_form_data_hash" - ) - if json_data_dict is not None: - JsonDataModel.insert_or_update_json_data_dict(json_data_dict) - db.session.add(task_model) - db.session.commit() + task_draft_data = TaskService.task_draft_data_from_task_model(task_model, create_if_not_exists=True) + + if task_draft_data is not None: + json_data_dict = TaskService.update_task_data_on_task_model_and_return_dict_if_updated( + task_draft_data, body, "saved_form_data_hash" + ) + 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( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index eb0c70280..ace0324fa 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/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" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index c670635a5..82f90913f 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/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.bpmn_process_definition.bpmn_identifier) + bpmn_process_identifiers.append( + 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. diff --git a/spiffworkflow-frontend/src/index.css b/spiffworkflow-frontend/src/index.css index bb28112b8..d6096c630 100644 --- a/spiffworkflow-frontend/src/index.css +++ b/spiffworkflow-frontend/src/index.css @@ -606,3 +606,7 @@ hr { .primary-file-text-suffix { font-style: italic; } + +#hidden-form-for-autosave { + display: none; +} diff --git a/spiffworkflow-frontend/src/routes/TaskShow.tsx b/spiffworkflow-frontend/src/routes/TaskShow.tsx index 83cf919d5..8ea176826 100644 --- a/spiffworkflow-frontend/src/routes/TaskShow.tsx +++ b/spiffworkflow-frontend/src/routes/TaskShow.tsx @@ -34,6 +34,8 @@ export default function TaskShow() { const [disabled, setDisabled] = useState(false); const [taskData, setTaskData] = useState(null); + const [autosaveOnFormChanges, setAutosaveOnFormChanges] = + useState(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 ); } @@ -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 (
{ setTaskData(obj.formData); - addDebouncedTaskDataAutoSave(obj.formData); + addDebouncedTaskDataAutoSave(); }} onSubmit={handleFormSubmit} schema={jsonSchema} @@ -458,6 +472,17 @@ export default function TaskShow() { > {reactFragmentToHideSubmitButton}
+
);