From d7e8533061a995595f520f31f7d4b56440078f5b Mon Sep 17 00:00:00 2001 From: burnettk Date: Tue, 25 Oct 2022 17:38:59 -0400 Subject: [PATCH] Squashed 'spiffworkflow-backend/' changes from ab9b5f36..7f1776b5 7f1776b5 Merge pull request #161 from sartography/feature/potential_owners_from_task_data d19477e3 test removing an approver from a list while user lane_owners dict w/ burnettk 2f333204 resolved merge conflicts w/ burnettk 6b9c248b Merge remote-tracking branch 'origin/main' into feature/potential_owners_from_task_data 24a96ccd WIP: deleting user after approval w/ burnettk 8659f212 pyl now passes w/ burnettk e9ef5bfe Merge remote-tracking branch 'origin/main' into feature/potential_owners_from_task_data 56f8d734 added some support to get the potential task owners from task data w/ burnettk git-subtree-dir: spiffworkflow-backend git-subtree-split: 7f1776b5b7f11ae6c02ef435200b0ba87219d7d2 --- src/spiffworkflow_backend/scripts/get_user.py | 26 +++ .../services/process_instance_processor.py | 98 +++++++--- .../lanes_with_owner_dict.bpmn | 175 ++++++++++++++++++ .../unit/test_authorization_service.py | 2 + .../unit/test_process_instance_processor.py | 106 +++++++++++ 5 files changed, 379 insertions(+), 28 deletions(-) create mode 100644 src/spiffworkflow_backend/scripts/get_user.py create mode 100644 tests/data/model_with_lanes/lanes_with_owner_dict.bpmn diff --git a/src/spiffworkflow_backend/scripts/get_user.py b/src/spiffworkflow_backend/scripts/get_user.py new file mode 100644 index 00000000..b362c88d --- /dev/null +++ b/src/spiffworkflow_backend/scripts/get_user.py @@ -0,0 +1,26 @@ +"""Get_env.""" +from typing import Any +from flask import g +from typing import Optional + +from SpiffWorkflow.task import Task as SpiffTask # type: ignore + +from spiffworkflow_backend.scripts.script import Script + + +class GetUser(Script): + """GetUser.""" + + def get_description(self) -> str: + """Get_description.""" + return """Return the current user.""" + + def run( + self, + task: Optional[SpiffTask], + environment_identifier: str, + *_args: Any, + **kwargs: Any + ) -> Any: + """Run.""" + return g.user.username diff --git a/src/spiffworkflow_backend/services/process_instance_processor.py b/src/spiffworkflow_backend/services/process_instance_processor.py index 70c38dba..a2fe0d44 100644 --- a/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/src/spiffworkflow_backend/services/process_instance_processor.py @@ -14,6 +14,7 @@ from typing import List from typing import NewType from typing import Optional from typing import Tuple +from typing import TypedDict from typing import Union from flask import current_app @@ -75,6 +76,7 @@ from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.models.task_event import TaskAction from spiffworkflow_backend.models.task_event import TaskEventModel +from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user import UserModelSchema from spiffworkflow_backend.scripts.script import Script from spiffworkflow_backend.services.file_system_service import FileSystemService @@ -106,6 +108,13 @@ DEFAULT_GLOBALS.update(safe_globals) DEFAULT_GLOBALS["__builtins__"]["__import__"] = _import +class PotentialOwnerIdList(TypedDict): + """PotentialOwnerIdList.""" + + potential_owner_ids: list[int] + lane_assignment_id: Optional[int] + + class ProcessInstanceProcessorError(Exception): """ProcessInstanceProcessorError.""" @@ -114,6 +123,10 @@ class NoPotentialOwnersForTaskError(Exception): """NoPotentialOwnersForTaskError.""" +class PotentialOwnerUserNotFoundError(Exception): + """PotentialOwnerUserNotFoundError.""" + + class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore """This is a custom script processor that can be easily injected into Spiff Workflow. @@ -499,6 +512,58 @@ class ProcessInstanceProcessor: self.save() + def raise_if_no_potential_owners( + self, potential_owner_ids: list[int], message: str + ) -> None: + """Raise_if_no_potential_owners.""" + if not potential_owner_ids: + raise (NoPotentialOwnersForTaskError(message)) + + def get_potential_owner_ids_from_task( + self, task: SpiffTask + ) -> PotentialOwnerIdList: + """Get_potential_owner_ids_from_task.""" + task_spec = task.task_spec + task_lane = "process_initiator" + if task_spec.lane is not None and task_spec.lane != "": + 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] + elif "lane_owners" in task.data and task_lane in task.data["lane_owners"]: + for username in task.data["lane_owners"][task_lane]: + lane_owner_user = UserModel.query.filter_by(username=username).first() + if lane_owner_user is not None: + potential_owner_ids.append(lane_owner_user.id) + self.raise_if_no_potential_owners( + potential_owner_ids, + f"No users found in task data lane owner list for lane: {task_lane}. " + f"The user list used: {task.data['lane_owners'][task_lane]}", + ) + 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 + self.raise_if_no_potential_owners( + potential_owner_ids, + f"Could not find any users in group to assign to lane: {task_lane}", + ) + + return { + "potential_owner_ids": potential_owner_ids, + "lane_assignment_id": lane_assignment_id, + } + def save(self) -> None: """Saves the current state of this processor to the database.""" self.process_instance_model.bpmn_json = self.serialize() @@ -532,32 +597,9 @@ class ProcessInstanceProcessor: # filter out non-usertasks task_spec = ready_or_waiting_task.task_spec if not self.bpmn_process_instance._is_engine_task(task_spec): - ready_or_waiting_task.data["current_user"]["id"] - task_lane = "process_initiator" - if task_spec.lane is not None and task_spec.lane != "": - 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 - + potential_owner_hash = self.get_potential_owner_ids_from_task( + ready_or_waiting_task + ) extensions = ready_or_waiting_task.task_spec.extensions form_file_name = None @@ -586,12 +628,12 @@ class ProcessInstanceProcessor: task_title=ready_or_waiting_task.task_spec.description, task_type=ready_or_waiting_task.task_spec.__class__.__name__, task_status=ready_or_waiting_task.get_state_name(), - lane_assignment_id=lane_assignment_id, + lane_assignment_id=potential_owner_hash["lane_assignment_id"], ) db.session.add(active_task) db.session.commit() - for potential_owner_id in potential_owner_ids: + for potential_owner_id in potential_owner_hash["potential_owner_ids"]: active_task_user = ActiveTaskUserModel( user_id=potential_owner_id, active_task_id=active_task.id ) diff --git a/tests/data/model_with_lanes/lanes_with_owner_dict.bpmn b/tests/data/model_with_lanes/lanes_with_owner_dict.bpmn new file mode 100644 index 00000000..2b89acb0 --- /dev/null +++ b/tests/data/model_with_lanes/lanes_with_owner_dict.bpmn @@ -0,0 +1,175 @@ + + + + + + + + + StartEvent_1 + initator_one + initiator_two + Activity_0wq6mdd + + + finance_approval_one + finance_approval_two + Activity_1s1855p + + + Event_0nsh6vv + bigwig_approval + + + + Flow_1tbyols + + + + + This is initiator user? + + Flow_1tbyols + Flow_0xyca1b + + + + + + + This is initiator again? + + Flow_1aluose + Flow_0jh05kw + + + Flow_04sc2wb + + + Flow_0jh05kw + Flow_04sc2wb + + + + Flow_0xyca1b + Flow_13ejjwk + lane_owners = { + "Finance Team": ['testuser3', 'testuser4'], + "Bigwig": ['testadmin1'], + "Process Initiator": ['testadmin1'] +} + + + + + This is finance user one? + + + + Flow_13ejjwk + Flow_0bgkfue + + + + This is finance user two? {{approver}} + + Flow_1ivhu7x + Flow_1aluose + + + + + Flow_0bgkfue + Flow_1ivhu7x + approver = get_user() +lane_owners["Finance Team"].remove(approver) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index fe54686d..ff2ac9bc 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -94,6 +94,8 @@ class TestAuthorizationService(BaseTest): """Test_user_can_be_added_to_active_task_on_first_login.""" initiator_user = self.find_or_create_user("initiator_user") assert initiator_user.principal is not None + # to ensure there is a user that can be assigned to the task + self.find_or_create_user("testuser1") AuthorizationService.import_permissions_from_yaml_file() process_model = load_test_spec( diff --git a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index b7dffc4b..543b99c0 100644 --- a/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -1,5 +1,6 @@ """Test_process_instance_processor.""" import pytest +from flask import g from flask.app import Flask from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec @@ -118,3 +119,108 @@ class TestProcessInstanceProcessor(BaseTest): ) assert process_instance.status == ProcessInstanceStatus.complete.value + + def test_sets_permission_correctly_on_active_task_when_using_dict( + self, + app: Flask, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + """Test_sets_permission_correctly_on_active_task_when_using_dict.""" + initiator_user = self.find_or_create_user("initiator_user") + finance_user_three = self.find_or_create_user("testuser3") + finance_user_four = self.find_or_create_user("testuser4") + testadmin1 = self.find_or_create_user("testadmin1") + assert initiator_user.principal is not None + assert finance_user_three.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_with_owner_dict.bpmn", + ) + process_instance = self.create_process_instance_from_process_model( + process_model=process_model, user=initiator_user + ) + 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] == initiator_user + + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + with pytest.raises(UserDoesNotHaveAccessToTaskError): + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, finance_user_three + ) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) + + 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) == 2 + assert active_task.potential_owners == [finance_user_three, finance_user_four] + + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + with pytest.raises(UserDoesNotHaveAccessToTaskError): + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) + + g.user = finance_user_three + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, finance_user_three + ) + 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] == finance_user_four + + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + with pytest.raises(UserDoesNotHaveAccessToTaskError): + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) + + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, finance_user_four + ) + 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] == initiator_user + + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) + + assert len(process_instance.active_tasks) == 1 + active_task = process_instance.active_tasks[0] + spiff_task = processor.__class__.get_task_by_bpmn_identifier( + active_task.task_name, processor.bpmn_process_instance + ) + with pytest.raises(UserDoesNotHaveAccessToTaskError): + ProcessInstanceService.complete_form_task( + processor, spiff_task, {}, initiator_user + ) + ProcessInstanceService.complete_form_task(processor, spiff_task, {}, testadmin1) + + assert process_instance.status == ProcessInstanceStatus.complete.value