From be14b9c05ff5462206efa420b35199fb797aabf1 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 19 Apr 2023 13:52:29 -0400 Subject: [PATCH] Add a test for the interstitial endpoint. Assure all "GO" buttons lead to the interstial page, and display differently depending on if there is anything you can actually do. --- .../spiffworkflow_backend/config/default.py | 2 +- .../routes/tasks_controller.py | 123 +++++++++--------- .../tests/data/interstitial/interstitial.bpmn | 123 ++++++++++++++++++ .../integration/test_process_api.py | 86 ++++++++++++ spiffworkflow-frontend/package-lock.json | 4 +- .../components/ProcessInstanceListTable.tsx | 39 +++--- .../src/routes/ProcessInterstitial.tsx | 28 ++-- 7 files changed, 307 insertions(+), 98 deletions(-) create mode 100644 spiffworkflow-backend/tests/data/interstitial/interstitial.bpmn diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py index dec4c444a..a111a08e4 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py @@ -140,7 +140,7 @@ SPIFFWORKFLOW_BACKEND_ENGINE_STEP_DEFAULT_STRATEGY_BACKGROUND = environ.get( ) SPIFFWORKFLOW_BACKEND_ENGINE_STEP_DEFAULT_STRATEGY_WEB = environ.get( - "SPIFFWORKFLOW_BACKEND_ENGINE_STEP_DEFAULT_STRATEGY_WEB", default="greedy" + "SPIFFWORKFLOW_BACKEND_ENGINE_STEP_DEFAULT_STRATEGY_WEB", default="run_until_user_message" ) # this is only used in CI. use SPIFFWORKFLOW_BACKEND_DATABASE_URI instead for real configuration diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index 2a2953a5f..70c24d14c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py @@ -85,7 +85,7 @@ class ReactJsonSchemaSelectOption(TypedDict): def task_list_my_tasks( - process_instance_id: Optional[int] = None, page: int = 1, per_page: int = 100 + process_instance_id: Optional[int] = None, page: int = 1, per_page: int = 100 ) -> flask.wrappers.Response: """Task_list_my_tasks.""" principal = _find_principal_or_raise() @@ -166,7 +166,7 @@ def task_list_for_me(page: int = 1, per_page: int = 100) -> flask.wrappers.Respo def task_list_for_my_groups( - user_group_identifier: Optional[str] = None, page: int = 1, per_page: int = 100 + user_group_identifier: Optional[str] = None, page: int = 1, per_page: int = 100 ) -> flask.wrappers.Response: """Task_list_for_my_groups.""" return _get_tasks( @@ -178,9 +178,9 @@ def task_list_for_my_groups( def task_data_show( - modified_process_model_identifier: str, - process_instance_id: int, - task_guid: str, + modified_process_model_identifier: str, + process_instance_id: int, + task_guid: str, ) -> flask.wrappers.Response: task_model = _get_task_model_from_guid_or_raise(task_guid, process_instance_id) task_model.data = task_model.json_data() @@ -188,10 +188,10 @@ def task_data_show( def task_data_update( - process_instance_id: str, - modified_process_model_identifier: str, - task_guid: str, - body: Dict, + process_instance_id: str, + modified_process_model_identifier: str, + task_guid: str, + body: Dict, ) -> Response: """Update task data.""" process_instance = ProcessInstanceModel.query.filter(ProcessInstanceModel.id == int(process_instance_id)).first() @@ -241,10 +241,10 @@ def task_data_update( def manual_complete_task( - modified_process_model_identifier: str, - process_instance_id: str, - task_guid: str, - body: Dict, + modified_process_model_identifier: str, + process_instance_id: str, + task_guid: str, + body: Dict, ) -> Response: """Mark a task complete without executing it.""" execute = body.get("execute", True) @@ -353,12 +353,13 @@ def task_show(process_instance_id: int, task_guid: str = "next") -> flask.wrappe _render_instructions_for_end_user(spiff_task, task) return make_response(jsonify(task), 200) + def _render_instructions_for_end_user(spiff_task: SpiffTask, task: Task): """Assure any instructions for end user are processed for jinja syntax.""" if task.properties and "instructionsForEndUser" in task.properties: if task.properties["instructionsForEndUser"]: try: - instructions = _render_jinja_template( + instructions = _render_jinja_template( task.properties["instructionsForEndUser"], spiff_task ) task.properties["instructionsForEndUser"] = instructions @@ -370,9 +371,9 @@ def _render_instructions_for_end_user(spiff_task: SpiffTask, task: Task): def process_data_show( - process_instance_id: int, - process_data_identifier: str, - modified_process_model_identifier: str, + process_instance_id: int, + process_data_identifier: str, + modified_process_model_identifier: str, ) -> flask.wrappers.Response: """Process_data_show.""" process_instance = _find_process_instance_by_id_or_raise(process_instance_id) @@ -392,36 +393,42 @@ def process_data_show( 200, ) -def interstitial(process_instance_id: int): + +def _interstitial_stream(process_instance_id: int): process_instance = _find_process_instance_by_id_or_raise(process_instance_id) processor = ProcessInstanceProcessor(process_instance) - reported_ids = [] # bit of an issue with end tasks showing as getting completed twice. - def get_data(): - spiff_task = processor.next_task() - last_task = None - while last_task != spiff_task: - task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task()) - instructions = _render_instructions_for_end_user(spiff_task, task) - if instructions and spiff_task.id not in reported_ids: - reported_ids.append(spiff_task.id) - yield f'data: {current_app.json.dumps(task)} \n\n' - last_task = spiff_task - processor.do_engine_steps(execution_strategy_name="run_until_user_message") - processor.do_engine_steps(execution_strategy_name="one_at_a_time") - spiff_task = processor.next_task() - # Note, this has to be done in case someone leaves the page, - # which can otherwise cancel this function and leave completed tasks un-registered. - processor.save() # Fixme - maybe find a way not to do this on every method? - return + reported_ids = [] # bit of an issue with end tasks showing as getting completed twice. # return Response(get_data(), mimetype='text/event-stream') - return Response(stream_with_context(get_data()), mimetype='text/event-stream') + + spiff_task = processor.next_task() + last_task = None + while last_task != spiff_task: + task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task()) + instructions = _render_instructions_for_end_user(spiff_task, task) + if instructions and spiff_task.id not in reported_ids: + reported_ids.append(spiff_task.id) + yield f'data: {current_app.json.dumps(task)} \n\n' + last_task = spiff_task + processor.do_engine_steps(execution_strategy_name="run_until_user_message") + processor.do_engine_steps(execution_strategy_name="one_at_a_time") + spiff_task = processor.next_task() + # Note, this has to be done in case someone leaves the page, + # which can otherwise cancel this function and leave completed tasks un-registered. + processor.save() # Fixme - maybe find a way not to do this on every method? + + +def interstitial(process_instance_id: int): + """A Server Side Events Stream for watching the execution of engine tasks in a + process instance. """ + return Response(stream_with_context(_interstitial_stream(process_instance_id)), mimetype='text/event-stream') + def _task_submit_shared( - process_instance_id: int, - task_guid: str, - body: Dict[str, Any], - save_as_draft: bool = False, + process_instance_id: int, + task_guid: str, + body: Dict[str, Any], + save_as_draft: bool = False, ) -> flask.wrappers.Response: principal = _find_principal_or_raise() process_instance = _find_process_instance_by_id_or_raise(process_instance_id) @@ -508,11 +515,12 @@ def _task_submit_shared( "process_instance_id": process_instance_id }), status=202, mimetype="application/json") + def task_submit( - process_instance_id: int, - task_guid: str, - body: Dict[str, Any], - save_as_draft: bool = False, + process_instance_id: int, + task_guid: str, + body: Dict[str, Any], + save_as_draft: bool = False, ) -> flask.wrappers.Response: """Task_submit_user_data.""" with sentry_sdk.start_span(op="controller_action", description="tasks_controller.task_submit"): @@ -520,11 +528,11 @@ def task_submit( def _get_tasks( - processes_started_by_user: bool = True, - has_lane_assignment_id: bool = True, - page: int = 1, - per_page: int = 100, - user_group_identifier: Optional[str] = None, + processes_started_by_user: bool = True, + has_lane_assignment_id: bool = True, + page: int = 1, + per_page: int = 100, + user_group_identifier: Optional[str] = None, ) -> flask.wrappers.Response: """Get_tasks.""" user_id = g.user.id @@ -671,9 +679,9 @@ def _render_jinja_template(unprocessed_template: str, spiff_task: SpiffTask) -> def _get_spiff_task_from_process_instance( - task_guid: str, - process_instance: ProcessInstanceModel, - processor: Union[ProcessInstanceProcessor, None] = None, + task_guid: str, + process_instance: ProcessInstanceModel, + processor: Union[ProcessInstanceProcessor, None] = None, ) -> SpiffTask: """Get_spiff_task_from_process_instance.""" if processor is None: @@ -729,9 +737,8 @@ def _update_form_schema_with_task_data_as_needed(in_dict: dict, task: Task, spif select_options_from_task_data = task.data.get(task_data_var) if isinstance(select_options_from_task_data, list): if all("value" in d and "label" in d for d in select_options_from_task_data): - def map_function( - task_data_select_option: TaskDataSelectOption, + task_data_select_option: TaskDataSelectOption, ) -> ReactJsonSchemaSelectOption: """Map_function.""" return { @@ -769,9 +776,9 @@ def _get_potential_owner_usernames(assigned_user: AliasedClass) -> Any: def _find_human_task_or_raise( - process_instance_id: int, - task_guid: str, - only_tasks_that_can_be_completed: bool = False, + process_instance_id: int, + task_guid: str, + only_tasks_that_can_be_completed: bool = False, ) -> HumanTaskModel: if only_tasks_that_can_be_completed: human_task_query = HumanTaskModel.query.filter_by( diff --git a/spiffworkflow-backend/tests/data/interstitial/interstitial.bpmn b/spiffworkflow-backend/tests/data/interstitial/interstitial.bpmn new file mode 100644 index 000000000..ec386e56f --- /dev/null +++ b/spiffworkflow-backend/tests/data/interstitial/interstitial.bpmn @@ -0,0 +1,123 @@ + + + + + + + + + StartEvent_1 + Activity_16m8jvv + Activity_1qrme8m + Activity_0bi0v5d + Event_1vyxv42 + + + Activity_02ldrj6 + + + + Flow_1rnrr8l + + + + + + + + Flow_1rnrr8l + Flow_011ysja + x = 2 + + + + I am Script Task {{x}} + + Flow_011ysja + Flow_1rab9xv + x = 2 + + + + + I am a manual task + + Flow_1rab9xv + Flow_1icul0s + + + + + I am a manual task in another lane + + Flow_1icul0s + Flow_06qy6r3 + + + + + I am the end task + + Flow_06qy6r3 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py index c5623f47b..778e1a8e4 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -10,6 +10,8 @@ import pytest from flask.app import Flask from flask.testing import FlaskClient from SpiffWorkflow.task import TaskState # type: ignore + +from spiffworkflow_backend.routes.tasks_controller import _interstitial_stream from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec @@ -1611,6 +1613,90 @@ class TestProcessApi(BaseTest): "veryImportantFieldButOnlySometimes": {"ui:widget": "hidden"}, } + def test_interstitial_page( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + + process_group_id = "my_process_group" + process_model_id = "interstitial" + bpmn_file_location = "interstitial" + # Assure we have someone in the finance team + finance_user = self.find_or_create_user("testuser2") + AuthorizationService.import_permissions_from_yaml_file() + process_model_identifier = self.create_group_and_model_with_bpmn( + client, + with_super_admin_user, + process_group_id=process_group_id, + process_model_id=process_model_id, + bpmn_file_location=bpmn_file_location, + ) + headers = self.logged_in_headers(with_super_admin_user) + response = self.create_process_instance_from_process_model_id_with_api( + client, process_model_identifier, headers + ) + assert response.json is not None + process_instance_id = response.json["id"] + + response = client.post( + f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}/run", + headers=headers, + ) + + assert response.json is not None + assert response.json["next_task"] is not None + assert response.json["next_task"]["state"] == 'READY' + assert response.json["next_task"]["title"] == 'Script Task #2' + + # Rather that call the API and deal with the Server Side Events, call the loop directly and covert it to + # a list. It tests all of our code. No reason to test Flasks SSE support. + results = list(_interstitial_stream(process_instance_id)) + json_results = list(map(lambda x: json.loads(x[5:]), results)) # strip the "data:" prefix and convert remaining string to dict. + # There should be 2 results back - + # the first script task should not be returned (it contains no end user instructions) + # The second script task should produce rendered jinja text + # The Manual Task should then return a message as well. + assert len(results) == 2 + assert json_results[0]["state"] == 'READY' + assert json_results[0]["title"] == 'Script Task #2' + assert json_results[0]["properties"]["instructionsForEndUser"] == 'I am Script Task 2' + assert json_results[1]["state"] == 'READY' + assert json_results[1]["title"] == 'Manual Task' + + response = client.put( + f"/v1.0/tasks/{process_instance_id}/{json_results[1]['id']}", + headers=headers, + ) + + assert response.json is not None + + # we should now be on a task that does not belong to the original user, and the interstitial page should know this. + results = list(_interstitial_stream(process_instance_id)) + json_results = list(map(lambda x: json.loads(x[5:]), results)) + assert len(results) == 1 + assert json_results[0]["state"] == 'READY' + assert json_results[0]["can_complete"] == False + assert json_results[0]["title"] == 'Please Approve' + assert json_results[0]["properties"]["instructionsForEndUser"] == 'I am a manual task in another lane' + + # Complete task as the finance user. + response = client.put( + f"/v1.0/tasks/{process_instance_id}/{json_results[0]['id']}", + headers=self.logged_in_headers(finance_user), + ) + + # We should now be on the end task with a valid message, even after loading it many times. + results_1 = list(_interstitial_stream(process_instance_id)) + results_2 = list(_interstitial_stream(process_instance_id)) + results = list(_interstitial_stream(process_instance_id)) + json_results = list(map(lambda x: json.loads(x[5:]), results)) + assert len(json_results) == 1 + assert json_results[0]["state"] == 'COMPLETED' + assert json_results[0]["properties"]["instructionsForEndUser"] == 'I am the end task' + def test_process_instance_list_with_default_list( self, app: Flask, diff --git a/spiffworkflow-frontend/package-lock.json b/spiffworkflow-frontend/package-lock.json index 0578dd911..51d4424c4 100644 --- a/spiffworkflow-frontend/package-lock.json +++ b/spiffworkflow-frontend/package-lock.json @@ -8072,7 +8072,7 @@ }, "node_modules/bpmn-js-spiffworkflow": { "version": "0.0.8", - "resolved": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#69135655f8a5282bcdaef82705c3d522ef5b4464", + "resolved": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#2214ac6432dec77cb2d1362615e6320bfea7df1f", "license": "MIT", "dependencies": { "inherits": "^2.0.4", @@ -38238,7 +38238,7 @@ } }, "bpmn-js-spiffworkflow": { - "version": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#69135655f8a5282bcdaef82705c3d522ef5b4464", + "version": "git+ssh://git@github.com/sartography/bpmn-js-spiffworkflow.git#2214ac6432dec77cb2d1362615e6320bfea7df1f", "from": "bpmn-js-spiffworkflow@sartography/bpmn-js-spiffworkflow#main", "requires": { "inherits": "^2.0.4", diff --git a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx index 227ae1bc5..b5950cdd8 100644 --- a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx +++ b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx @@ -1443,28 +1443,27 @@ export default function ProcessInstanceListTable({ }); if (showActionsColumn) { let buttonElement = null; - if (row.task_id) { - const taskUrl = `/tasks/${row.id}/${row.task_id}`; - const regex = new RegExp(`\\b(${preferredUsername}|${userEmail})\\b`); - let hasAccessToCompleteTask = false; - if ( - canCompleteAllTasks || - (row.potential_owner_usernames || '').match(regex) - ) { - hasAccessToCompleteTask = true; - } - buttonElement = ( - - ); + const interstitialUrl = `/process/${modifyProcessIdentifierForPathParam( + row.process_model_identifier)}/${row.id}/interstitial` + const regex = new RegExp(`\\b(${preferredUsername}|${userEmail})\\b`); + let hasAccessToCompleteTask = false; + if ( + canCompleteAllTasks || + (row.potential_owner_usernames || '').match(regex) + ) { + hasAccessToCompleteTask = true; } + buttonElement = ( + + ); currentRow.push({buttonElement}); } diff --git a/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx b/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx index b36e638cd..cf5910179 100644 --- a/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx @@ -87,10 +87,10 @@ export default function ProcessInterstitial() { ['User Task', 'Manual Task'].includes(myTask.type) ) { return ( -
This task is assigned to another user or group to complete.
+
This next task must be completed by a different person.
); } - return ; + return
; }; if (lastTask) { @@ -114,21 +114,15 @@ export default function ProcessInterstitial() { - - - {data && - data.map((d) => ( - - - - - ))} - -
-

{d.title}

- 		-

{userMessage(d)}

-
+ {data && + data.map((d) => ( +
+
+ Task: {d.title} +
+
{userMessage(d)}
+
+ ))}