From 3fce735d4fb400a0a6f8f5fb352ef0e085e50044 Mon Sep 17 00:00:00 2001 From: jasquat <2487833+jasquat@users.noreply.github.com> Date: Tue, 19 Dec 2023 11:59:29 -0500 Subject: [PATCH] Feature/task table drop (#823) * removed id from task and still working on getting the migration working w/ burnettk * fixed migration to work on postgres and sqlite as well w/ burnettk * fixed tests w/ burnettk --------- Co-authored-by: jasquat --- .../migrations/versions/0c7428378d6e_.py | 4 +- .../migrations/versions/3191627ae224_.py | 62 +++++++++++++++++++ .../src/spiffworkflow_backend/models/db.py | 5 ++ .../models/human_task.py | 4 +- .../src/spiffworkflow_backend/models/task.py | 11 +++- .../routes/process_instances_controller.py | 7 ++- .../routes/tasks_controller.py | 5 +- .../services/process_instance_processor.py | 2 +- .../unit/test_process_instance_migrator.py | 4 +- .../unit/test_process_instance_processor.py | 1 - .../unit/test_task_service.py | 6 +- 11 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 spiffworkflow-backend/migrations/versions/3191627ae224_.py diff --git a/spiffworkflow-backend/migrations/versions/0c7428378d6e_.py b/spiffworkflow-backend/migrations/versions/0c7428378d6e_.py index cddb4345..9fb330bf 100644 --- a/spiffworkflow-backend/migrations/versions/0c7428378d6e_.py +++ b/spiffworkflow-backend/migrations/versions/0c7428378d6e_.py @@ -413,7 +413,7 @@ def upgrade(): sa.ForeignKeyConstraint(['process_instance_id'], ['process_instance.id'], ), sa.ForeignKeyConstraint(['task_definition_id'], ['task_definition.id'], ), sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('guid') + sa.UniqueConstraint('guid', name='guid') ) with op.batch_alter_table('task', schema=None) as batch_op: batch_op.create_index(batch_op.f('ix_task_bpmn_process_id'), ['bpmn_process_id'], unique=False) @@ -446,7 +446,7 @@ def upgrade(): sa.ForeignKeyConstraint(['completed_by_user_id'], ['user.id'], ), sa.ForeignKeyConstraint(['lane_assignment_id'], ['group.id'], ), sa.ForeignKeyConstraint(['process_instance_id'], ['process_instance.id'], ), - sa.ForeignKeyConstraint(['task_model_id'], ['task.id'], ), + sa.ForeignKeyConstraint(['task_model_id'], ['task.id'], name='human_task_ibfk_5'), sa.PrimaryKeyConstraint('id') ) with op.batch_alter_table('human_task', schema=None) as batch_op: diff --git a/spiffworkflow-backend/migrations/versions/3191627ae224_.py b/spiffworkflow-backend/migrations/versions/3191627ae224_.py new file mode 100644 index 00000000..52102cfb --- /dev/null +++ b/spiffworkflow-backend/migrations/versions/3191627ae224_.py @@ -0,0 +1,62 @@ +"""empty message + +Revision ID: 3191627ae224 +Revises: 441dca328887 +Create Date: 2023-12-18 17:08:53.142318 + +""" +# import os +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import mysql +from spiffworkflow_backend.models.db import db, dialect_name + +# revision identifiers, used by Alembic. +revision = '3191627ae224' +down_revision = '441dca328887' +branch_labels = None +depends_on = None + +def is_mysql() -> bool: + return dialect_name() == 'mysql' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + # we did in fact heavily adjust this one. be careful if auto-generating. + with op.batch_alter_table('task', schema=None) as batch_op: + if is_mysql(): + batch_op.drop_index('guid') + batch_op.create_index(batch_op.f('ix_task_guid'), ['guid'], unique=False) + + with op.batch_alter_table('human_task', schema=None) as batch_op: + batch_op.add_column(sa.Column('task_guid', sa.String(length=36), nullable=True)) + batch_op.drop_constraint('human_task_ibfk_5', type_='foreignkey') + batch_op.drop_index('ix_human_task_task_model_id') + batch_op.create_index(batch_op.f('ix_human_task_task_guid'), ['task_guid'], unique=False) + batch_op.create_foreign_key('human_task_ibfk_task_guid', 'task', ['task_guid'], ['guid']) + batch_op.drop_column('task_model_id') + + with op.batch_alter_table('task', schema=None) as batch_op: + batch_op.drop_column('id') + batch_op.create_primary_key('guid_pk', ['guid']) + + # ### 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('id', mysql.INTEGER(), autoincrement=True, nullable=False)) + batch_op.drop_index(batch_op.f('ix_task_guid')) + batch_op.create_index('guid', ['guid'], unique=False) + + with op.batch_alter_table('human_task', schema=None) as batch_op: + batch_op.add_column(sa.Column('task_model_id', mysql.INTEGER(), autoincrement=False, nullable=True)) + batch_op.drop_constraint(None, type_='foreignkey') + batch_op.create_foreign_key('human_task_ibfk_5', 'task', ['task_model_id'], ['id']) + batch_op.drop_index(batch_op.f('ix_human_task_task_guid')) + batch_op.create_index('ix_human_task_task_model_id', ['task_model_id'], unique=False) + batch_op.drop_column('task_guid') + + # ### end Alembic commands ### diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/db.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/db.py index 12e0ae6d..75bcbf5a 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/db.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/db.py @@ -3,6 +3,7 @@ from __future__ import annotations import enum import time from typing import Any +from typing import cast from flask_migrate import Migrate # type: ignore from flask_sqlalchemy import SQLAlchemy @@ -19,6 +20,10 @@ migrate = Migrate() # 2) database migration code picks them up when migrations are automatically generated +def dialect_name() -> str: + return cast(str, db.engine.dialect.name) + + class SpiffworkflowBaseDBModel(db.Model): # type: ignore __abstract__ = True diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py index dadac51f..fee793c8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py @@ -39,9 +39,9 @@ class HumanTaskModel(SpiffworkflowBaseDBModel): updated_at_in_seconds: int = db.Column(db.Integer) created_at_in_seconds: int = db.Column(db.Integer) - # task_id came first which is why it's a string and task_model_id is the int and foreignkey - task_model_id: int = db.Column(ForeignKey(TaskModel.id), nullable=True, index=True) # type: ignore + task_guid: str = db.Column(ForeignKey(TaskModel.guid), nullable=True, index=True) task_model = relationship(TaskModel) + task_id: str = db.Column(db.String(50)) task_name: str = db.Column(db.String(255)) task_title: str = db.Column(db.String(50)) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py index 2b32dac8..186b9bde 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py @@ -49,8 +49,7 @@ class MultiInstanceType(enum.Enum): class TaskModel(SpiffworkflowBaseDBModel): __tablename__ = "task" __allow_unmapped__ = True - id: int = db.Column(db.Integer, primary_key=True) - guid: str = db.Column(db.String(36), nullable=False, unique=True) + guid: str = db.Column(db.String(36), nullable=False, index=True, primary_key=True, unique=True) bpmn_process_id: int = db.Column(ForeignKey(BpmnProcessModel.id), nullable=False, index=True) # type: ignore bpmn_process = relationship(BpmnProcessModel, back_populates="tasks") human_tasks = relationship("HumanTaskModel", back_populates="task_model", cascade="delete") @@ -99,6 +98,14 @@ class TaskModel(SpiffworkflowBaseDBModel): task_model: TaskModel = self.__class__.query.filter_by(guid=self.properties_json["parent"]).first() return task_model + @classmethod + def sort_by_last_state_changed(cls, task_models: list[TaskModel]) -> list[TaskModel]: + def sort_function(task: TaskModel) -> float: + state_change: float = task.properties_json["last_state_change"] + return state_change + + return sorted(task_models, key=sort_function) + # this will redirect to login if the task does not allow guest access. # so if you already completed the task, and you are not signed in, you will get sent to a login page. def allows_guest(self, process_instance_id: int) -> bool: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py index bb95b720..6793a4dc 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -367,6 +367,10 @@ def _process_instance_task_list( to_task_guid: str | None = None, most_recent_tasks_only: bool = False, ) -> flask.wrappers.Response: + """This is only used on the Process Instance Show page on the frontend. + + This is how we know what the state of each task is and how to color things. + """ bpmn_process_ids = [] if bpmn_process_guid: bpmn_process = BpmnProcessModel.query.filter_by(guid=bpmn_process_guid).first() @@ -429,8 +433,7 @@ def _process_instance_task_list( direct_parent_bpmn_process_definition_alias = aliased(BpmnProcessDefinitionModel) task_model_query = ( - task_model_query.order_by(TaskModel.id.desc()) # type: ignore - .join(TaskDefinitionModel, TaskDefinitionModel.id == TaskModel.task_definition_id) + task_model_query.join(TaskDefinitionModel, TaskDefinitionModel.id == TaskModel.task_definition_id) # type: ignore .join(bpmn_process_alias, bpmn_process_alias.id == TaskModel.bpmn_process_id) .outerjoin( direct_parent_bpmn_process_alias, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index 06307842..cbe07c97 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py @@ -320,7 +320,6 @@ def task_instance_list( task_model = _get_task_model_from_guid_or_raise(task_guid, process_instance_id) task_model_instances = ( TaskModel.query.filter_by(task_definition_id=task_model.task_definition.id, bpmn_process_id=task_model.bpmn_process_id) - .order_by(TaskModel.id.desc()) # type: ignore .join(TaskDefinitionModel, TaskDefinitionModel.id == TaskModel.task_definition_id) .add_columns( TaskDefinitionModel.bpmn_identifier, @@ -335,7 +334,9 @@ def task_instance_list( TaskModel.properties_json, ) ).all() - return make_response(jsonify(task_model_instances), 200) + + sorted_task_models = TaskModel.sort_by_last_state_changed(task_model_instances) + return make_response(jsonify(sorted_task_models), 200) def manual_complete_task( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py index f3d96edb..6310bf32 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -1094,7 +1094,7 @@ class ProcessInstanceProcessor: bpmn_process_identifier=bpmn_process_identifier, form_file_name=form_file_name, ui_form_file_name=ui_form_file_name, - task_model_id=task_model.id, + task_guid=task_model.guid, task_id=task_guid, task_name=ready_or_waiting_task.task_spec.bpmn_id, task_title=ready_or_waiting_task.task_spec.bpmn_name, diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_migrator.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_migrator.py index 07f2a77d..46d15976 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_migrator.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_migrator.py @@ -61,9 +61,9 @@ class TestProcessInstanceMigrator(BaseTest): task_model.properties_json = new_properties_json db.session.add(task_model) db.session.commit() - task_model = TaskModel.query.filter_by(id=task_model.id).first() + task_model = TaskModel.query.filter_by(guid=task_model.guid).first() assert task_model.properties_json["last_state_change"] is None VersionOneThree().run() - task_model = TaskModel.query.filter_by(id=task_model.id).first() + task_model = TaskModel.query.filter_by(guid=task_model.guid).first() assert task_model.properties_json["last_state_change"] is not None diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index d833c6fc..e974627c 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -323,7 +323,6 @@ class TestProcessInstanceProcessor(BaseTest): all_task_models_matching_top_level_subprocess_script = ( TaskModel.query.join(TaskDefinitionModel) .filter(TaskDefinitionModel.bpmn_identifier == "top_level_subprocess_script") - .order_by(TaskModel.id.desc()) # type: ignore .all() ) assert len(all_task_models_matching_top_level_subprocess_script) == 1 diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_task_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_task_service.py index d69656c7..f083662b 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_task_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_task_service.py @@ -88,7 +88,6 @@ class TestTaskService(BaseTest): task_model_level_2b = ( TaskModel.query.join(TaskDefinitionModel) .filter(TaskDefinitionModel.bpmn_identifier == "level_2b_subprocess_script_task") - .order_by(TaskModel.id) .first() ) assert task_model_level_2b is not None @@ -101,10 +100,7 @@ class TestTaskService(BaseTest): assert task_models[0].task_definition.bpmn_identifier == "level2b_second_call" task_model_level_3 = ( - TaskModel.query.join(TaskDefinitionModel) - .filter(TaskDefinitionModel.bpmn_identifier == "level_3_script_task") - .order_by(TaskModel.id) - .first() + TaskModel.query.join(TaskDefinitionModel).filter(TaskDefinitionModel.bpmn_identifier == "level_3_script_task").first() ) assert task_model_level_3 is not None (bpmn_processes, task_models) = TaskService.task_models_of_parent_bpmn_processes(