diff --git a/conftest.py b/conftest.py index 242f75a0..6e4703c2 100644 --- a/conftest.py +++ b/conftest.py @@ -6,6 +6,7 @@ import pytest from flask.app import Flask from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec @@ -56,6 +57,8 @@ def app() -> Flask: @pytest.fixture() def with_db_and_bpmn_file_cleanup() -> None: """Process_group_resource.""" + db.session.query(ActiveTaskUserModel).delete() + for model in SpiffworkflowBaseDBModel._all_subclasses(): db.session.query(model).delete() db.session.commit() diff --git a/migrations/env.py b/migrations/env.py index 630e381a..68feded2 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -1,3 +1,5 @@ +from __future__ import with_statement + import logging from logging.config import fileConfig diff --git a/migrations/versions/cf862b761896_.py b/migrations/versions/1e25676fed79_.py similarity index 94% rename from migrations/versions/cf862b761896_.py rename to migrations/versions/1e25676fed79_.py index c326afe5..dd2e3ccd 100644 --- a/migrations/versions/cf862b761896_.py +++ b/migrations/versions/1e25676fed79_.py @@ -1,8 +1,8 @@ """empty message -Revision ID: cf862b761896 +Revision ID: 1e25676fed79 Revises: -Create Date: 2022-10-20 11:23:58.758922 +Create Date: 2022-10-20 16:29:56.969687 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. -revision = 'cf862b761896' +revision = '1e25676fed79' down_revision = None branch_labels = None depends_on = None @@ -157,7 +157,8 @@ def upgrade(): op.create_table('active_task', sa.Column('id', sa.Integer(), nullable=False), sa.Column('process_instance_id', sa.Integer(), nullable=False), - sa.Column('assigned_principal_id', sa.Integer(), nullable=True), + sa.Column('actual_owner_id', sa.Integer(), nullable=True), + sa.Column('lane_assignment_id', sa.Integer(), nullable=True), sa.Column('form_file_name', sa.String(length=50), nullable=True), sa.Column('ui_form_file_name', sa.String(length=50), nullable=True), sa.Column('updated_at_in_seconds', sa.Integer(), nullable=True), @@ -169,7 +170,8 @@ def upgrade(): sa.Column('task_status', sa.String(length=50), nullable=True), sa.Column('process_model_display_name', sa.String(length=255), nullable=True), sa.Column('task_data', sa.Text(length=4294000000), nullable=True), - sa.ForeignKeyConstraint(['assigned_principal_id'], ['principal.id'], ), + sa.ForeignKeyConstraint(['actual_owner_id'], ['user.id'], ), + sa.ForeignKeyConstraint(['lane_assignment_id'], ['group.id'], ), sa.ForeignKeyConstraint(['process_instance_id'], ['process_instance.id'], ), sa.PrimaryKeyConstraint('id'), sa.UniqueConstraint('task_id', 'process_instance_id', name='active_task_unique') @@ -272,6 +274,17 @@ def upgrade(): sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), sa.PrimaryKeyConstraint('id') ) + op.create_table('active_task_user', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('active_task_id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['active_task_id'], ['active_task.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('active_task_id', 'user_id', name='active_task_user_unique') + ) + op.create_index(op.f('ix_active_task_user_active_task_id'), 'active_task_user', ['active_task_id'], unique=False) + op.create_index(op.f('ix_active_task_user_user_id'), 'active_task_user', ['user_id'], unique=False) op.create_table('data_store', sa.Column('id', sa.Integer(), nullable=False), sa.Column('updated_at_in_seconds', sa.Integer(), nullable=True), @@ -305,6 +318,9 @@ def downgrade(): op.drop_index(op.f('ix_message_correlation_message_instance_message_correlation_id'), table_name='message_correlation_message_instance') op.drop_table('message_correlation_message_instance') op.drop_table('data_store') + op.drop_index(op.f('ix_active_task_user_user_id'), table_name='active_task_user') + op.drop_index(op.f('ix_active_task_user_active_task_id'), table_name='active_task_user') + op.drop_table('active_task_user') op.drop_table('task_event') op.drop_table('spiff_logging') op.drop_table('permission_assignment') diff --git a/src/spiffworkflow_backend/config/development.py b/src/spiffworkflow_backend/config/development.py index 151868e5..a7152177 100644 --- a/src/spiffworkflow_backend/config/development.py +++ b/src/spiffworkflow_backend/config/development.py @@ -2,7 +2,7 @@ from os import environ SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME = environ.get( - "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME", default="staging.yml" + "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME", default="development.yml" ) SPIFFWORKFLOW_BACKEND_LOG_LEVEL = environ.get( diff --git a/src/spiffworkflow_backend/config/permissions/staging.yml b/src/spiffworkflow_backend/config/permissions/development.yml similarity index 100% rename from src/spiffworkflow_backend/config/permissions/staging.yml rename to src/spiffworkflow_backend/config/permissions/development.yml diff --git a/src/spiffworkflow_backend/config/permissions/testing.yml b/src/spiffworkflow_backend/config/permissions/testing.yml index 0d3f3806..f77dd3ee 100644 --- a/src/spiffworkflow_backend/config/permissions/testing.yml +++ b/src/spiffworkflow_backend/config/permissions/testing.yml @@ -2,8 +2,8 @@ groups: admin: users: [testadmin1, testadmin2] - finance: - users: [testuser1, testuser2] + Finance Team: + users: [testuser2] hr: users: [testuser2, testuser3, testuser4] @@ -16,13 +16,13 @@ permissions: uri: /* read-all: - groups: [finance, hr, admin] + groups: ["Finance Team", hr, admin] users: [] allowed_permissions: [read] uri: /* finance-admin: - groups: [finance] + groups: ["Finance Team"] users: [testuser4] allowed_permissions: [create, read, update, delete] uri: /v1.0/process-groups/finance/* diff --git a/src/spiffworkflow_backend/load_database_models.py b/src/spiffworkflow_backend/load_database_models.py index 33d32c1a..4427df06 100644 --- a/src/spiffworkflow_backend/load_database_models.py +++ b/src/spiffworkflow_backend/load_database_models.py @@ -15,8 +15,8 @@ from spiffworkflow_backend.models.user_group_assignment import ( UserGroupAssignmentModel, ) # noqa: F401 - from spiffworkflow_backend.models.active_task import ActiveTaskModel # noqa: F401 +from spiffworkflow_backend.models.active_task_user import (ActiveTaskUserModel) from spiffworkflow_backend.models.bpmn_process_id_lookup import ( BpmnProcessIdLookup, ) # noqa: F401 diff --git a/src/spiffworkflow_backend/models/active_task.py b/src/spiffworkflow_backend/models/active_task.py index f9daebe8..a8b898f0 100644 --- a/src/spiffworkflow_backend/models/active_task.py +++ b/src/spiffworkflow_backend/models/active_task.py @@ -3,16 +3,25 @@ from __future__ import annotations import json from dataclasses import dataclass +from typing import TYPE_CHECKING from flask_bpmn.models.db import db from flask_bpmn.models.db import SpiffworkflowBaseDBModel from sqlalchemy import ForeignKey from sqlalchemy.orm import relationship from sqlalchemy.orm import RelationshipProperty +from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.principal import PrincipalModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.task import Task +from spiffworkflow_backend.models.user import UserModel + + +if TYPE_CHECKING: + from spiffworkflow_backend.models.active_task_user import ( + ActiveTaskUserModel, + ) # noqa: F401 @dataclass @@ -26,14 +35,15 @@ class ActiveTaskModel(SpiffworkflowBaseDBModel): ), ) - assigned_principal: RelationshipProperty[PrincipalModel] = relationship( - PrincipalModel + actual_owner: RelationshipProperty[UserModel] = relationship( + UserModel ) id: int = db.Column(db.Integer, primary_key=True) process_instance_id: int = db.Column( ForeignKey(ProcessInstanceModel.id), nullable=False # type: ignore ) - assigned_principal_id: int = db.Column(ForeignKey(PrincipalModel.id)) + actual_owner_id: int = db.Column(ForeignKey(UserModel.id)) + lane_assignment_id: int | None = db.Column(ForeignKey(GroupModel.id)) form_file_name: str | None = db.Column(db.String(50)) ui_form_file_name: str | None = db.Column(db.String(50)) @@ -48,6 +58,14 @@ class ActiveTaskModel(SpiffworkflowBaseDBModel): process_model_display_name = db.Column(db.String(255)) task_data: str = db.Column(db.Text(4294000000)) + active_task_users = relationship("ActiveTaskUserModel", cascade="delete") + potential_owners = relationship( # type: ignore + "UserModel", + viewonly=True, + secondary="active_task_user", + overlaps="active_task_user,users", + ) + @classmethod def to_task(cls, task: ActiveTaskModel) -> Task: """To_task.""" diff --git a/src/spiffworkflow_backend/models/active_task_user.py b/src/spiffworkflow_backend/models/active_task_user.py new file mode 100644 index 00000000..92d97796 --- /dev/null +++ b/src/spiffworkflow_backend/models/active_task_user.py @@ -0,0 +1,31 @@ +from __future__ import annotations +from dataclasses import dataclass + +from flask_bpmn.models.db import db +from flask_bpmn.models.db import SpiffworkflowBaseDBModel +from sqlalchemy import ForeignKey + +from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.user import UserModel + + +@dataclass +class ActiveTaskUserModel(SpiffworkflowBaseDBModel): + + __tablename__ = "active_task_user" + + __table_args__ = ( + db.UniqueConstraint( + "active_task_id", + "user_id", + name="active_task_user_unique", + ), + ) + + id = db.Column(db.Integer, primary_key=True) + active_task_id = db.Column( + ForeignKey(ActiveTaskModel.id), nullable=False, index=True # type: ignore + ) + user_id = db.Column( + ForeignKey(UserModel.id), nullable=False, index=True + ) diff --git a/src/spiffworkflow_backend/models/user.py b/src/spiffworkflow_backend/models/user.py index 30d60a6f..65daaf39 100644 --- a/src/spiffworkflow_backend/models/user.py +++ b/src/spiffworkflow_backend/models/user.py @@ -1,4 +1,5 @@ """User.""" +from __future__ import annotations from typing import Any import jwt diff --git a/src/spiffworkflow_backend/routes/process_api_blueprint.py b/src/spiffworkflow_backend/routes/process_api_blueprint.py index aa9152f7..4f62bcd1 100644 --- a/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -886,7 +886,7 @@ def process_instance_report_show( def task_list_my_tasks(page: int = 1, per_page: int = 100) -> flask.wrappers.Response: """Task_list_my_tasks.""" principal = find_principal_or_raise() - + # TODO: use join table active_tasks = ( ActiveTaskModel.query.filter_by(assigned_principal_id=principal.id) .order_by(desc(ActiveTaskModel.id)) # type: ignore @@ -1089,6 +1089,7 @@ def task_submit( ProcessInstanceService.update_task_assignments(processor) + # TODO: update next_active_task_assigned_to_me = ActiveTaskModel.query.filter_by( assigned_principal_id=principal.id, process_instance_id=process_instance.id ).first() @@ -1258,10 +1259,12 @@ def find_active_task_by_id_or_raise( process_instance_id: int, task_id: str, principal_id: PrincipalModel ) -> ActiveTaskModel: """Find_active_task_by_id_or_raise.""" + + # TODO: update active_task_assigned_to_me = ActiveTaskModel.query.filter_by( process_instance_id=process_instance_id, task_id=task_id, - assigned_principal_id=principal_id, + # assigned_principal_id=principal_id, ).first() if active_task_assigned_to_me is None: message = ( diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index b9353686..278ea594 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -164,14 +164,16 @@ class AuthorizationService: ) if "users" in permission_config: for username in permission_config["users"]: - principal = ( - PrincipalModel.query.join(UserModel) - .filter(UserModel.username == username) - .first() - ) - cls.create_permission_for_principal( - principal, permission_target, allowed_permission - ) + user = UserModel.query.filter_by(username=username).first() + if user is not None: + principal = ( + PrincipalModel.query.join(UserModel) + .filter(UserModel.username == username) + .first() + ) + cls.create_permission_for_principal( + principal, permission_target, allowed_permission + ) @classmethod def create_permission_for_principal( diff --git a/src/spiffworkflow_backend/services/process_instance_processor.py b/src/spiffworkflow_backend/services/process_instance_processor.py index 38d4fe37..470eb729 100644 --- a/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/src/spiffworkflow_backend/services/process_instance_processor.py @@ -4,6 +4,7 @@ import decimal import json import logging import os +import re import time from datetime import datetime from typing import Any @@ -55,9 +56,11 @@ from SpiffWorkflow.task import TaskState from SpiffWorkflow.util.deep_merge import DeepMerge # type: ignore from spiffworkflow_backend.models.active_task import ActiveTaskModel +from spiffworkflow_backend.models.active_task_user import ActiveTaskUserModel from spiffworkflow_backend.models.bpmn_process_id_lookup import BpmnProcessIdLookup from spiffworkflow_backend.models.file import File from spiffworkflow_backend.models.file import FileType +from spiffworkflow_backend.models.group import GroupModel from spiffworkflow_backend.models.message_correlation import MessageCorrelationModel from spiffworkflow_backend.models.message_correlation_message_instance import ( MessageCorrelationMessageInstanceModel, @@ -108,6 +111,10 @@ class ProcessInstanceProcessorError(Exception): """ProcessInstanceProcessorError.""" +class NoPotentialOwnersForTaskError(Exception): + pass + + class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore """This is a custom script processor that can be easily injected into Spiff Workflow. @@ -511,28 +518,47 @@ class ProcessInstanceProcessor: if self.bpmn_process_instance.is_completed(): self.process_instance_model.end_in_seconds = round(time.time()) - db.session.add(self.process_instance_model) - ActiveTaskModel.query.filter_by( + active_tasks = ActiveTaskModel.query.filter_by( process_instance_id=self.process_instance_model.id - ).delete() + ).all() + if len(active_tasks) > 0: + for at in active_tasks: + db.session.delete(at) + + db.session.add(self.process_instance_model) + db.session.commit() ready_or_waiting_tasks = self.get_all_ready_or_waiting_tasks() for ready_or_waiting_task in ready_or_waiting_tasks: # filter out non-usertasks - if not self.bpmn_process_instance._is_engine_task( - ready_or_waiting_task.task_spec - ): + task_spec = ready_or_waiting_task.task_spec + if not self.bpmn_process_instance._is_engine_task(task_spec): user_id = ready_or_waiting_task.data["current_user"]["id"] - principal = PrincipalModel.query.filter_by(user_id=user_id).first() - if principal is None: - raise ( - ApiError( - error_code="principal_not_found", - message=f"Principal not found from user id: {user_id}", - status_code=400, - ) - ) + # principal = PrincipalModel.query.filter_by(user_id=user_id).first() + # if principal is None: + # raise ( + # ApiError( + # error_code="principal_not_found", + # message=f"Principal not found from user id: {user_id}", + # status_code=400, + # ) + # ) + # import pdb; pdb.set_trace() + task_lane = 'process_initiator' + if task_spec.lane is not None: + task_lane = task_spec.lane + + potential_owner_ids = [] + lane_assignment_id = None + if re.match(r"(process.?)initiator", task_lane, re.IGNORECASE): + potential_owner_ids = [self.process_instance_model.process_initiator_id] + else: + group_model = GroupModel.query.filter_by(identifier=task_lane).first() + if group_model is None: + raise (NoPotentialOwnersForTaskError(f"Could not find a group with name matching lane: {task_lane}")) + potential_owner_ids = [i.user_id for i in group_model.user_group_assignments] + lane_assignment_id = group_model.id extensions = ready_or_waiting_task.task_spec.extensions @@ -555,7 +581,6 @@ class ProcessInstanceProcessor: active_task = ActiveTaskModel( process_instance_id=self.process_instance_model.id, process_model_display_name=process_model_display_name, - assigned_principal_id=principal.id, form_file_name=form_file_name, ui_form_file_name=ui_form_file_name, task_id=str(ready_or_waiting_task.id), @@ -564,10 +589,15 @@ class ProcessInstanceProcessor: task_type=ready_or_waiting_task.task_spec.__class__.__name__, task_status=ready_or_waiting_task.get_state_name(), task_data=json.dumps(ready_or_waiting_task.data), + lane_assignment_id=lane_assignment_id, ) db.session.add(active_task) + db.session.commit() - db.session.commit() + for potential_owner_id in potential_owner_ids: + active_task_user = ActiveTaskUserModel(user_id=potential_owner_id,active_task_id=active_task.id) + db.session.add(active_task_user) + db.session.commit() @staticmethod def get_parser() -> MyCustomParser: diff --git a/src/spiffworkflow_backend/services/process_instance_service.py b/src/spiffworkflow_backend/services/process_instance_service.py index 2d6999fc..24b0eede 100644 --- a/src/spiffworkflow_backend/services/process_instance_service.py +++ b/src/spiffworkflow_backend/services/process_instance_service.py @@ -282,8 +282,7 @@ class ProcessInstanceService: ProcessInstanceService.log_task_action( user.id, processor, spiff_task, TaskAction.COMPLETE.value ) - processor.do_engine_steps() - processor.save() + processor.do_engine_steps(save=True) @staticmethod def log_task_action( diff --git a/tests/data/model_with_lanes/lanes.bpmn b/tests/data/model_with_lanes/lanes.bpmn new file mode 100644 index 00000000..3ee43501 --- /dev/null +++ b/tests/data/model_with_lanes/lanes.bpmn @@ -0,0 +1,103 @@ + + + + + + + + + StartEvent_1 + initator_one + Event_06f4e68 + initiator_two + + + finance_approval + + + + Flow_1tbyols + + + + + + This is initiator user? + + Flow_1tbyols + Flow_16ppta1 + + + + This is finance user? + + Flow_16ppta1 + Flow_1cfcauf + + + + Flow_0x92f7d + + + + + This is initiator again? + + Flow_1cfcauf + Flow_0x92f7d + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/spiffworkflow_backend/helpers/base_test.py b/tests/spiffworkflow_backend/helpers/base_test.py index c779c819..57b26617 100644 --- a/tests/spiffworkflow_backend/helpers/base_test.py +++ b/tests/spiffworkflow_backend/helpers/base_test.py @@ -207,10 +207,12 @@ class BaseTest: # return public_access_token def create_process_instance_from_process_model( - self, process_model: ProcessModelInfo, status: Optional[str] = "not_started" + self, process_model: ProcessModelInfo, status: Optional[str] = "not_started", user: Optional[UserModel] = None ) -> ProcessInstanceModel: """Create_process_instance_from_process_model.""" - user = self.find_or_create_user() + if user is None: + user = self.find_or_create_user() + current_time = round(time.time()) process_instance = ProcessInstanceModel( status=status, diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index 43949d60..d8f9ce4b 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -19,6 +19,11 @@ class TestAuthorizationService(BaseTest): raise_if_missing_user=True ) + def test_does_not_fail_if_user_not_created( + self, app: Flask, with_db_and_bpmn_file_cleanup: None + ) -> None: + AuthorizationService.import_permissions_from_yaml_file() + def test_can_import_permissions_from_yaml( self, app: Flask, with_db_and_bpmn_file_cleanup: None ) -> None: diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 009239f7..34594b6d 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -1,6 +1,11 @@ """Test_process_instance_processor.""" from flask.app import Flask +from spiffworkflow_backend.models.group import GroupModel +from spiffworkflow_backend.models.process_instance import ProcessInstanceModel, ProcessInstanceStatus +from spiffworkflow_backend.services.authorization_service import AuthorizationService +from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService from tests.spiffworkflow_backend.helpers.base_test import BaseTest +from tests.spiffworkflow_backend.helpers.test_data import load_test_spec from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, @@ -34,3 +39,57 @@ class TestProcessInstanceProcessor(BaseTest): result == "Chuck Norris doesn’t read books. He stares them down until he gets the information he wants." ) + + def test_sets_permission_correctly_on_active_task( + self, + app: Flask, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + testuser1 = self.find_or_create_user("testuser1") + testuser2 = self.find_or_create_user("testuser2") + assert testuser1.principal is not None + assert testuser2.principal is not None + AuthorizationService.import_permissions_from_yaml_file() + + finance_group = GroupModel.query.filter_by(identifier="Finance Team").first() + assert finance_group is not None + + process_model = load_test_spec(process_model_id="model_with_lanes", bpmn_file_name="lanes.bpmn") + process_instance = self.create_process_instance_from_process_model(process_model=process_model, user=testuser1) + processor = ProcessInstanceProcessor(process_instance) + processor.do_engine_steps(save=True) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + assert active_task.lane_assignment_id is None + assert len(active_task.potential_owners) == 1 + assert active_task.potential_owners[0] == testuser1 + + task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + ProcessInstanceService.complete_form_task( + processor, task, {}, testuser1 + ) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + assert active_task.lane_assignment_id == finance_group.id + assert len(active_task.potential_owners) == 1 + assert active_task.potential_owners[0] == testuser2 + + task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + ProcessInstanceService.complete_form_task( + processor, task, {}, testuser1 + ) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + assert active_task.lane_assignment_id is None + assert len(active_task.potential_owners) == 1 + assert active_task.potential_owners[0] == testuser1 + + task = processor.__class__.get_task_by_bpmn_identifier(active_task.task_name, processor.bpmn_process_instance) + ProcessInstanceService.complete_form_task( + processor, task, {}, testuser1 + ) + + assert process_instance.status == ProcessInstanceStatus.complete.value