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 <jasquat@users.noreply.github.com>

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:

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 ###

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
 

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))

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:

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,

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(

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,

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

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

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(