From 66bd628c58a07d04a723f47a48e17b0a46d7cd2c Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 2 May 2023 16:31:29 -0400 Subject: [PATCH 1/9] disallow saving a process model file if it has changed w/ burnettk --- .../src/spiffworkflow_backend/api.yml | 7 ++++ .../src/spiffworkflow_backend/models/file.py | 40 +++++-------------- .../models/process_model.py | 2 +- .../routes/process_models_controller.py | 29 +++++++++----- .../services/spec_file_service.py | 1 - spiffworkflow-frontend/src/interfaces.ts | 1 + .../src/routes/ProcessModelEditDiagram.tsx | 3 ++ 7 files changed, 39 insertions(+), 44 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 0ea5291ce..0bec6684f 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -1245,6 +1245,13 @@ paths: put: operationId: spiffworkflow_backend.routes.process_models_controller.process_model_file_update summary: save the contents to the given file + parameters: + - name: file_contents_hash + in: query + required: true + description: The hash of the file contents that originally came with the file. + schema: + type: string tags: - Process Model Files requestBody: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py index eb8d706db..5a29aec1c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py @@ -1,13 +1,10 @@ -"""File.""" +from __future__ import annotations from dataclasses import dataclass +from typing import Any from dataclasses import field from datetime import datetime from typing import Optional -import marshmallow -from marshmallow import INCLUDE -from marshmallow import Schema - from spiffworkflow_backend.helpers.spiff_enum import SpiffEnum from spiffworkflow_backend.models.spec_reference import SpecReference @@ -77,7 +74,7 @@ class File: references: Optional[list[SpecReference]] = None file_contents: Optional[bytes] = None process_model_id: Optional[str] = None - process_group_id: Optional[str] = None + file_contents_hash: Optional[str] = None def __post_init__(self) -> None: """__post_init__.""" @@ -102,28 +99,9 @@ class File: ) return instance - -class FileSchema(Schema): - """FileSchema.""" - - class Meta: - """Meta.""" - - model = File - fields = [ - "id", - "name", - "content_type", - "last_modified", - "type", - "size", - "data_store", - "user_uid", - "url", - "file_contents", - "references", - "process_group_id", - "process_model_id", - ] - unknown = INCLUDE - references = marshmallow.fields.List(marshmallow.fields.Nested("SpecReferenceSchema")) + @property + def serialized(self) -> dict[str, Any]: + dictionary = self.__dict__ + if isinstance(self.file_contents, bytes): + dictionary['file_contents'] = self.file_contents.decode('utf-8') + return dictionary diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py index c1f57fbb6..a5a6a9240 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py @@ -87,7 +87,7 @@ class ProcessModelInfoSchema(Schema): display_order = marshmallow.fields.Integer(allow_none=True) primary_file_name = marshmallow.fields.String(allow_none=True) primary_process_id = marshmallow.fields.String(allow_none=True) - files = marshmallow.fields.List(marshmallow.fields.Nested("FileSchema")) + files = marshmallow.fields.List(marshmallow.fields.Nested("File")) fault_or_suspend_on_exception = marshmallow.fields.String() exception_notification_addresses = marshmallow.fields.List(marshmallow.fields.String) metadata_extraction_paths = marshmallow.fields.List( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py index 880cabda7..84376eca2 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py @@ -1,4 +1,5 @@ """APIs for dealing with process groups, process models, and process instances.""" +from hashlib import sha256 import json import os import re @@ -18,7 +19,6 @@ from werkzeug.datastructures import FileStorage from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.interfaces import IdToProcessGroupMapping -from spiffworkflow_backend.models.file import FileSchema from spiffworkflow_backend.models.process_group import ProcessGroup from spiffworkflow_backend.models.process_instance_report import ( ProcessInstanceReportModel, @@ -245,10 +245,9 @@ def process_model_list( return make_response(jsonify(response_json), 200) -def process_model_file_update(modified_process_model_identifier: str, file_name: str) -> flask.wrappers.Response: - """Process_model_file_update.""" +def process_model_file_update(modified_process_model_identifier: str, file_name: str, file_contents_hash: str) -> flask.wrappers.Response: message = f"User: {g.user.username} clicked save for" - return _create_or_update_process_model_file(modified_process_model_identifier, message, 200) + return _create_or_update_process_model_file(modified_process_model_identifier, message, 200, file_contents_hash=file_contents_hash) def process_model_file_delete(modified_process_model_identifier: str, file_name: str) -> flask.wrappers.Response: @@ -293,7 +292,6 @@ def process_model_file_create( def process_model_file_show(modified_process_model_identifier: str, file_name: str) -> Any: - """Process_model_file_show.""" process_model_identifier = modified_process_model_identifier.replace(":", "/") process_model = _get_process_model(process_model_identifier) files = SpecFileService.get_files(process_model, file_name) @@ -309,8 +307,10 @@ def process_model_file_show(modified_process_model_identifier: str, file_name: s file = files[0] file_contents = SpecFileService.get_data(process_model, file.name) file.file_contents = file_contents + file_contents_hash = sha256(file_contents).hexdigest() + file.file_contents_hash = file_contents_hash file.process_model_id = process_model.id - return FileSchema().dump(file) + return make_response(jsonify(file), 200) # { @@ -477,6 +477,7 @@ def _create_or_update_process_model_file( modified_process_model_identifier: str, message_for_git_commit: str, http_status_to_return: int, + file_contents_hash: Optional[str], ) -> flask.wrappers.Response: """_create_or_update_process_model_file.""" process_model_identifier = modified_process_model_identifier.replace(":", "/") @@ -498,6 +499,16 @@ def _create_or_update_process_model_file( status_code=400, ) + current_file_contents_bytes = SpecFileService.get_data(process_model, request_file.filename) + if current_file_contents_bytes and file_contents_hash: + current_file_contents_hash = sha256(current_file_contents_bytes).hexdigest() + if current_file_contents_hash != file_contents_hash: + raise ApiError( + error_code="process_model_file_has_changed", + message=f"Process model file: {request_file.filename} was already changed by someone else. Please reload before making changes.", + status_code=409, + ) + file = None try: file = SpecFileService.update_file(process_model, request_file.filename, request_file_contents) @@ -514,8 +525,4 @@ def _create_or_update_process_model_file( file.process_model_id = process_model.id _commit_and_push_to_git(f"{message_for_git_commit} {process_model_identifier}/{file.name}") - return Response( - json.dumps(FileSchema().dump(file)), - status=http_status_to_return, - mimetype="application/json", - ) + return make_response(jsonify(file), http_status_to_return) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py index 0b2f963a4..1f7d1e0f2 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py @@ -175,7 +175,6 @@ class SpecFileService(FileSystemService): @classmethod def update_file(cls, process_model_info: ProcessModelInfo, file_name: str, binary_data: bytes) -> File: - """Update_file.""" SpecFileService.assert_valid_file_name(file_name) cls.validate_bpmn_xml(file_name, binary_data) diff --git a/spiffworkflow-frontend/src/interfaces.ts b/spiffworkflow-frontend/src/interfaces.ts index cc36484cd..c50b516f3 100644 --- a/spiffworkflow-frontend/src/interfaces.ts +++ b/spiffworkflow-frontend/src/interfaces.ts @@ -114,6 +114,7 @@ export interface ProcessFile { size: number; type: string; file_contents?: string; + file_contents_hash?: string; } export interface ProcessInstanceMetadata { diff --git a/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx b/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx index c82bc7a2f..6cd81fb7d 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelEditDiagram.tsx @@ -206,6 +206,9 @@ export default function ProcessModelEditDiagram() { httpMethod = 'POST'; } else { url += `/${fileNameWithExtension}`; + if (processModelFile && processModelFile.file_contents_hash) { + url += `?file_contents_hash=${processModelFile.file_contents_hash}`; + } } if (!fileNameWithExtension) { handleShowFileNameEditor(); From 38428cb6a4b3cf52a3e5ecc25cfbac3b030cf521 Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 2 May 2023 17:09:29 -0400 Subject: [PATCH 2/9] fixed tests w/ burnettk --- .../src/spiffworkflow_backend/api.yml | 2 +- .../src/spiffworkflow_backend/models/file.py | 16 ++++---- .../routes/process_models_controller.py | 41 +++++++++++-------- .../helpers/base_test.py | 2 - .../integration/test_process_api.py | 41 +++++++------------ .../src/routes/ProcessModelShow.tsx | 1 - 6 files changed, 47 insertions(+), 56 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 0bec6684f..be4d25a9e 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -1248,7 +1248,7 @@ paths: parameters: - name: file_contents_hash in: query - required: true + required: false description: The hash of the file contents that originally came with the file. schema: type: string diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py index 5a29aec1c..3b4d72e23 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/file.py @@ -1,9 +1,9 @@ from __future__ import annotations + from dataclasses import dataclass -from typing import Any from dataclasses import field from datetime import datetime -from typing import Optional +from typing import Any from spiffworkflow_backend.helpers.spiff_enum import SpiffEnum from spiffworkflow_backend.models.spec_reference import SpecReference @@ -71,10 +71,10 @@ class File: type: str last_modified: datetime size: int - references: Optional[list[SpecReference]] = None - file_contents: Optional[bytes] = None - process_model_id: Optional[str] = None - file_contents_hash: Optional[str] = None + references: list[SpecReference] | None = None + file_contents: bytes | None = None + process_model_id: str | None = None + file_contents_hash: str | None = None def __post_init__(self) -> None: """__post_init__.""" @@ -88,7 +88,7 @@ class File: content_type: str, last_modified: datetime, file_size: int, - ) -> "File": + ) -> File: """From_file_system.""" instance = cls( name=file_name, @@ -103,5 +103,5 @@ class File: def serialized(self) -> dict[str, Any]: dictionary = self.__dict__ if isinstance(self.file_contents, bytes): - dictionary['file_contents'] = self.file_contents.decode('utf-8') + dictionary["file_contents"] = self.file_contents.decode("utf-8") return dictionary diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py index 84376eca2..7d33b67d7 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py @@ -1,8 +1,8 @@ """APIs for dealing with process groups, process models, and process instances.""" -from hashlib import sha256 import json import os import re +from hashlib import sha256 from typing import Any from typing import Dict from typing import Optional @@ -245,9 +245,13 @@ def process_model_list( return make_response(jsonify(response_json), 200) -def process_model_file_update(modified_process_model_identifier: str, file_name: str, file_contents_hash: str) -> flask.wrappers.Response: +def process_model_file_update( + modified_process_model_identifier: str, file_name: str, file_contents_hash: str +) -> flask.wrappers.Response: message = f"User: {g.user.username} clicked save for" - return _create_or_update_process_model_file(modified_process_model_identifier, message, 200, file_contents_hash=file_contents_hash) + return _create_or_update_process_model_file( + modified_process_model_identifier, message, 200, file_contents_hash=file_contents_hash + ) def process_model_file_delete(modified_process_model_identifier: str, file_name: str) -> flask.wrappers.Response: @@ -297,10 +301,8 @@ def process_model_file_show(modified_process_model_identifier: str, file_name: s files = SpecFileService.get_files(process_model, file_name) if len(files) == 0: raise ApiError( - error_code="unknown file", - message=( - f"No information exists for file {file_name} it does not exist in workflow {process_model_identifier}." - ), + error_code="process_model_file_not_found", + message=f"File {file_name} not found in workflow {process_model_identifier}.", status_code=404, ) @@ -477,7 +479,7 @@ def _create_or_update_process_model_file( modified_process_model_identifier: str, message_for_git_commit: str, http_status_to_return: int, - file_contents_hash: Optional[str], + file_contents_hash: Optional[str] = None, ) -> flask.wrappers.Response: """_create_or_update_process_model_file.""" process_model_identifier = modified_process_model_identifier.replace(":", "/") @@ -499,15 +501,20 @@ def _create_or_update_process_model_file( status_code=400, ) - current_file_contents_bytes = SpecFileService.get_data(process_model, request_file.filename) - if current_file_contents_bytes and file_contents_hash: - current_file_contents_hash = sha256(current_file_contents_bytes).hexdigest() - if current_file_contents_hash != file_contents_hash: - raise ApiError( - error_code="process_model_file_has_changed", - message=f"Process model file: {request_file.filename} was already changed by someone else. Please reload before making changes.", - status_code=409, - ) + if file_contents_hash is not None: + current_file_contents_bytes = SpecFileService.get_data(process_model, request_file.filename) + if current_file_contents_bytes and file_contents_hash: + current_file_contents_hash = sha256(current_file_contents_bytes).hexdigest() + if current_file_contents_hash != file_contents_hash: + raise ApiError( + error_code="process_model_file_has_changed", + message=( + f"Process model file: {request_file.filename} was already changed by someone else. If you made" + " changes you do not want to lose, click the Download button and make sure your changes are" + " in the resulting file. If you do not need your changes, you can safely reload this page." + ), + status_code=409, + ) file = None try: diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py b/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py index d89ed3581..3a5290ed2 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py @@ -113,7 +113,6 @@ class BaseTest: process_group_id: str, display_name: str = "", ) -> str: - """Create_process_group.""" process_group = ProcessGroup(id=process_group_id, display_name=display_name, display_order=0, admin=False) response = client.post( "/v1.0/process-groups", @@ -138,7 +137,6 @@ class BaseTest: primary_file_name: Optional[str] = None, user: Optional[UserModel] = None, ) -> TestResponse: - """Create_process_model.""" if process_model_id is not None: # make sure we have a group process_group_id, _ = os.path.split(process_model_id) 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 ce72759b1..2c4aa5067 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -3,6 +3,7 @@ import io import json import os import time +from hashlib import sha256 from typing import Any from typing import Dict @@ -232,7 +233,6 @@ class TestProcessApi(BaseTest): with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """Test_primary_process_id_updates_via_xml.""" process_group_id = "test_group" process_model_id = "sample" process_model_identifier = f"{process_group_id}/{process_model_id}" @@ -258,10 +258,11 @@ class TestProcessApi(BaseTest): updated_bpmn_file_data_string = bpmn_file_data_string.replace(old_string, new_string) updated_bpmn_file_data_bytes = bytearray(updated_bpmn_file_data_string, "utf-8") data = {"file": (io.BytesIO(updated_bpmn_file_data_bytes), bpmn_file_name)} + file_contents_hash = sha256(bpmn_file_data_bytes).hexdigest() modified_process_model_id = process_model_identifier.replace("/", ":") response = client.put( - f"/v1.0/process-models/{modified_process_model_id}/files/{bpmn_file_name}", + f"/v1.0/process-models/{modified_process_model_id}/files/{bpmn_file_name}?file_contents_hash={file_contents_hash}", data=data, follow_redirects=True, content_type="multipart/form-data", @@ -780,13 +781,12 @@ class TestProcessApi(BaseTest): with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """Test_process_model_file_update.""" process_model_identifier = self.create_group_and_model_with_bpmn(client, with_super_admin_user) modified_process_model_id = process_model_identifier.replace("/", ":") data = {"key1": "THIS DATA"} response = client.put( - f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg", + f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg?file_contents_hash=does_not_matter", data=data, follow_redirects=True, content_type="multipart/form-data", @@ -803,13 +803,12 @@ class TestProcessApi(BaseTest): with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """Test_process_model_file_update.""" process_model_identifier = self.create_group_and_model_with_bpmn(client, with_super_admin_user) modified_process_model_id = process_model_identifier.replace("/", ":") data = {"file": (io.BytesIO(b""), "random_fact.svg")} response = client.put( - f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg", + f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg?file_contents_hash=does_not_matter", data=data, follow_redirects=True, content_type="multipart/form-data", @@ -827,30 +826,22 @@ class TestProcessApi(BaseTest): with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """Test_process_model_file_update.""" process_group_id = "test_group" - process_group_description = "Test Group" - process_model_id = "random_fact" + process_model_id = "simple_form" process_model_identifier = f"{process_group_id}/{process_model_id}" - self.create_process_group_with_api(client, with_super_admin_user, process_group_id, process_group_description) - self.create_process_model_with_api( - client, + bpmn_file_name = "simple_form.json" + load_test_spec( process_model_id=process_model_identifier, - user=with_super_admin_user, - ) - - bpmn_file_name = "random_fact.bpmn" - original_file = load_test_spec( - process_model_id=process_model_id, - bpmn_file_name=bpmn_file_name, - process_model_source_directory="random_fact", + process_model_source_directory="simple_form", ) + bpmn_file_data_bytes = self.get_test_data_file_contents(bpmn_file_name, process_model_id) + file_contents_hash = sha256(bpmn_file_data_bytes).hexdigest() modified_process_model_id = process_model_identifier.replace("/", ":") new_file_contents = b"THIS_IS_NEW_DATA" - data = {"file": (io.BytesIO(new_file_contents), "random_fact.svg")} + data = {"file": (io.BytesIO(new_file_contents), bpmn_file_name)} response = client.put( - f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg", + f"/v1.0/process-models/{modified_process_model_id}/files/{bpmn_file_name}?file_contents_hash={file_contents_hash}", data=data, follow_redirects=True, content_type="multipart/form-data", @@ -862,12 +853,11 @@ class TestProcessApi(BaseTest): assert response.json["file_contents"] is not None response = client.get( - f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg", + f"/v1.0/process-models/{modified_process_model_id}/files/simple_form.json", headers=self.logged_in_headers(with_super_admin_user), ) assert response.status_code == 200 updated_file = json.loads(response.get_data(as_text=True)) - assert original_file != updated_file assert updated_file["file_contents"] == new_file_contents.decode() def test_process_model_file_delete_when_bad_process_model( @@ -879,9 +869,6 @@ class TestProcessApi(BaseTest): ) -> None: """Test_process_model_file_update.""" process_model_identifier = self.create_group_and_model_with_bpmn(client, with_super_admin_user) - # self.create_spec_file(client, user=with_super_admin_user) - - # process_model = load_test_spec("random_fact") bad_process_model_identifier = f"x{process_model_identifier}" modified_bad_process_model_identifier = bad_process_model_identifier.replace("/", ":") response = client.delete( diff --git a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx index f0abcfb84..3b105dde2 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx @@ -8,7 +8,6 @@ import { Favorite, Edit, View, - ArrowRight, // @ts-ignore } from '@carbon/icons-react'; import { From 424894b5aea47888cb1590b5fad156469e1fe56e Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 3 May 2023 17:08:22 -0400 Subject: [PATCH 3/9] Test and updates to assure that when a task has a boundary event, and you return to that event, and then progress one step, you don't get stuck with a task that can't ever be completed. Let SpiffWorkflow determine what tasks we need to update in the DB using the task_state_change date on the tasks. --- .../services/process_instance_processor.py | 43 +++++---- .../boundary_event_reset.bpmn | 51 +++++++++++ .../boundary_event_reset/sub_with_timer.bpmn | 89 +++++++++++++++++++ .../unit/test_process_instance_processor.py | 47 ++++++++++ 4 files changed, 212 insertions(+), 18 deletions(-) create mode 100644 spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn create mode 100644 spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn 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 2ff469b3f..4da0724f6 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -1115,59 +1115,54 @@ class ProcessInstanceProcessor: def manual_complete_task(self, task_id: str, execute: bool) -> None: """Mark the task complete optionally executing it.""" - spiff_tasks_updated = {} start_in_seconds = time.time() spiff_task = self.bpmn_process_instance.get_task_from_id(UUID(task_id)) event_type = ProcessInstanceEventType.task_skipped.value + start_time = time.time() + if execute: current_app.logger.info( f"Manually executing Task {spiff_task.task_spec.name} of process" f" instance {self.process_instance_model.id}" ) - # Executing a subworkflow manually will restart its subprocess and allow stepping through it + # Executing a sub-workflow manually will restart its subprocess and allow stepping through it if isinstance(spiff_task.task_spec, SubWorkflowTask): subprocess = self.bpmn_process_instance.get_subprocess(spiff_task) # We have to get to the actual start event - for task in self.bpmn_process_instance.get_tasks(workflow=subprocess): - task.complete() - spiff_tasks_updated[task.id] = task - if isinstance(task.task_spec, StartEvent): + for spiff_task in self.bpmn_process_instance.get_tasks(workflow=subprocess): + spiff_task.run() + if isinstance(spiff_task.task_spec, StartEvent): break else: - spiff_task.complete() - spiff_tasks_updated[spiff_task.id] = spiff_task - for child in spiff_task.children: - spiff_tasks_updated[child.id] = child + spiff_task.run() event_type = ProcessInstanceEventType.task_executed_manually.value else: spiff_logger = logging.getLogger("spiff") spiff_logger.info(f"Skipped task {spiff_task.task_spec.name}", extra=spiff_task.log_info()) - spiff_task._set_state(TaskState.COMPLETED) - for child in spiff_task.children: - child.task_spec._update(child) - spiff_tasks_updated[child.id] = child + spiff_task.complete() spiff_task.workflow.last_task = spiff_task - spiff_tasks_updated[spiff_task.id] = spiff_task - end_in_seconds = time.time() if isinstance(spiff_task.task_spec, EndEvent): for task in self.bpmn_process_instance.get_tasks(TaskState.DEFINITE_MASK, workflow=spiff_task.workflow): task.complete() - spiff_tasks_updated[task.id] = task # A subworkflow task will become ready when its workflow is complete. Engine steps would normally # then complete it, but we have to do it ourselves here. for task in self.bpmn_process_instance.get_tasks(TaskState.READY): if isinstance(task.task_spec, SubWorkflowTask): task.complete() - spiff_tasks_updated[task.id] = task task_service = TaskService( process_instance=self.process_instance_model, serializer=self._serializer, bpmn_definition_to_task_definitions_mappings=self.bpmn_definition_to_task_definitions_mappings, ) + + spiff_tasks_updated = {} + for task in self.bpmn_process_instance.get_tasks(): + if task.last_state_change > start_time: + spiff_tasks_updated[task.id] = task for updated_spiff_task in spiff_tasks_updated.values(): ( bpmn_process, @@ -1216,6 +1211,13 @@ class ProcessInstanceProcessor: raise TaskNotFoundError( f"Cannot find a task with guid '{to_task_guid}' for process instance '{process_instance.id}'" ) + # If this task model has a parent boundary event, reset to that point instead, so we can reset all the boundary timers, etc... + parent_id = to_task_model.properties_json.get('parent','') + parent = TaskModel.query.filter_by(guid=parent_id).first() + is_boundary_parent = False + if parent and parent.task_definition.typename == '_BoundaryEventParent': + to_task_model = parent + is_boundary_parent = True # Will need to complete this task at the end so we are on the correct process. # NOTE: run ALL queries before making changes to ensure we get everything before anything changes parent_bpmn_processes, task_models_of_parent_bpmn_processes = TaskService.task_models_of_parent_bpmn_processes( @@ -1320,6 +1322,11 @@ class ProcessInstanceProcessor: db.session.commit() processor = ProcessInstanceProcessor(process_instance) + + # If this as a boundary event parent, run it, so we get back to an active task. + if is_boundary_parent: + processor.do_engine_steps(execution_strategy_name='one_at_a_time') + processor.save() processor.suspend() diff --git a/spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn b/spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn new file mode 100644 index 000000000..01ce0c5a9 --- /dev/null +++ b/spiffworkflow-backend/tests/data/boundary_event_reset/boundary_event_reset.bpmn @@ -0,0 +1,51 @@ + + + + + Flow_1ist4rn + + + + Flow_1xbry1g + + + + Flow_1ist4rn + Flow_0vzi07z + + + + Flow_0vzi07z + Flow_1xbry1g + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn b/spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn new file mode 100644 index 000000000..adffceda2 --- /dev/null +++ b/spiffworkflow-backend/tests/data/boundary_event_reset/sub_with_timer.bpmn @@ -0,0 +1,89 @@ + + + + + Flow_1e5apvr + + + + + Flow_110vf76 + + + + Flow_1hy0t7d + + 'P14D' + + + + + Flow_1xbdri7 + + + + Flow_1e5apvr + Flow_0vtgres + + + Flow_1hy0t7d + Flow_1xbdri7 + + + Flow_0vtgres + Flow_110vf76 + y='1000' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 bfda1eb34..2495a6b51 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 @@ -434,6 +434,53 @@ class TestProcessInstanceProcessor(BaseTest): assert process_instance.status == "complete" + def test_properly_resets_process_on_tasks_with_boundary_events( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + self.create_process_group_with_api(client, with_super_admin_user, "test_group", "test_group") + process_model = load_test_spec( + process_model_id="test_group/boundary_event_reset", + process_model_source_directory="boundary_event_reset", + ) + process_instance = self.create_process_instance_from_process_model( + process_model=process_model, user=with_super_admin_user + ) + processor = ProcessInstanceProcessor(process_instance) + processor.do_engine_steps(save=True) + assert len(process_instance.active_human_tasks) == 1 + human_task_one = process_instance.active_human_tasks[0] + spiff_manual_task = processor.bpmn_process_instance.get_task_from_id(UUID(human_task_one.task_id)) + ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, with_super_admin_user, human_task_one) + assert ( + len(process_instance.active_human_tasks) == 1 + ), "expected 1 active human tasks after 2nd one is completed" + assert process_instance.active_human_tasks[0].task_title == 'Final' + + # Reset the process back to the task within the call activity that contains a timer_boundary event. + reset_to_spiff_task = processor.__class__.get_task_by_bpmn_identifier( + 'manual_task_1', processor.bpmn_process_instance + ) + processor.suspend() + processor = ProcessInstanceProcessor(process_instance) + ProcessInstanceProcessor.reset_process(process_instance, str(reset_to_spiff_task.id)) + human_task_one = process_instance.active_human_tasks[0] + assert human_task_one.task_title == 'Manual Task #1' + processor = ProcessInstanceProcessor(process_instance) + processor.manual_complete_task(str(spiff_manual_task.id), execute=True) + processor = ProcessInstanceProcessor(process_instance) + processor.resume() + processor.do_engine_steps(save=True) + process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first() + + assert (len(process_instance.active_human_tasks) == 1) + assert process_instance.active_human_tasks[0].task_title == 'Final', \ + "once we reset, resume, and complete the task, we should be back to the Final step again, and not" \ + "stuck waiting for the call activity to complete (which was happening in a bug I'm fixing right now)" + def test_properly_saves_tasks_when_running( self, app: Flask, From fc7d3c39077cc8e5e40a808f14193d1926b29b2b Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 3 May 2023 17:29:33 -0400 Subject: [PATCH 4/9] run_pyl --- .../spiffworkflow_backend/config/__init__.py | 12 +++---- .../models/process_instance.py | 6 ++-- .../services/process_instance_processor.py | 21 +++++++------ .../unit/test_process_instance_processor.py | 31 ++++++++++--------- 4 files changed, 37 insertions(+), 33 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py b/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py index 7711c36f9..eaf67f6c9 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py @@ -18,13 +18,13 @@ def setup_database_uri(app: Flask) -> None: if app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_URI") is None: database_name = f"spiffworkflow_backend_{app.config['ENV_IDENTIFIER']}" if app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_TYPE") == "sqlite": - app.config["SQLALCHEMY_DATABASE_URI"] = ( - f"sqlite:///{app.instance_path}/db_{app.config['ENV_IDENTIFIER']}.sqlite3" - ) + app.config[ + "SQLALCHEMY_DATABASE_URI" + ] = f"sqlite:///{app.instance_path}/db_{app.config['ENV_IDENTIFIER']}.sqlite3" elif app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_TYPE") == "postgres": - app.config["SQLALCHEMY_DATABASE_URI"] = ( - f"postgresql://spiffworkflow_backend:spiffworkflow_backend@localhost:5432/{database_name}" - ) + app.config[ + "SQLALCHEMY_DATABASE_URI" + ] = f"postgresql://spiffworkflow_backend:spiffworkflow_backend@localhost:5432/{database_name}" else: # use pswd to trick flake8 with hardcoded passwords db_pswd = app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_PASSWORD") diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py index b3ab709df..303532af5 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py @@ -129,9 +129,9 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel): def serialized_with_metadata(self) -> dict[str, Any]: process_instance_attributes = self.serialized process_instance_attributes["process_metadata"] = self.process_metadata - process_instance_attributes["process_model_with_diagram_identifier"] = ( - self.process_model_with_diagram_identifier - ) + process_instance_attributes[ + "process_model_with_diagram_identifier" + ] = self.process_model_with_diagram_identifier return process_instance_attributes @property 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 a42abf4f2..d2f532327 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -423,9 +423,9 @@ class ProcessInstanceProcessor: tld.process_instance_id = process_instance_model.id # we want this to be the fully qualified path to the process model including all group subcomponents - current_app.config["THREAD_LOCAL_DATA"].process_model_identifier = ( - f"{process_instance_model.process_model_identifier}" - ) + current_app.config[ + "THREAD_LOCAL_DATA" + ].process_model_identifier = f"{process_instance_model.process_model_identifier}" self.process_instance_model = process_instance_model self.process_model_service = ProcessModelService() @@ -585,9 +585,9 @@ class ProcessInstanceProcessor: bpmn_subprocess_definition.bpmn_identifier ] = bpmn_process_definition_dict spiff_bpmn_process_dict["subprocess_specs"][bpmn_subprocess_definition.bpmn_identifier]["task_specs"] = {} - bpmn_subprocess_definition_bpmn_identifiers[bpmn_subprocess_definition.id] = ( - bpmn_subprocess_definition.bpmn_identifier - ) + bpmn_subprocess_definition_bpmn_identifiers[ + bpmn_subprocess_definition.id + ] = bpmn_subprocess_definition.bpmn_identifier task_definitions = TaskDefinitionModel.query.filter( TaskDefinitionModel.bpmn_process_definition_id.in_( # type: ignore @@ -1211,11 +1211,12 @@ class ProcessInstanceProcessor: raise TaskNotFoundError( f"Cannot find a task with guid '{to_task_guid}' for process instance '{process_instance.id}'" ) - # If this task model has a parent boundary event, reset to that point instead, so we can reset all the boundary timers, etc... - parent_id = to_task_model.properties_json.get('parent','') + # If this task model has a parent boundary event, reset to that point instead, + # so we can reset all the boundary timers, etc... + parent_id = to_task_model.properties_json.get("parent", "") parent = TaskModel.query.filter_by(guid=parent_id).first() is_boundary_parent = False - if parent and parent.task_definition.typename == '_BoundaryEventParent': + if parent and parent.task_definition.typename == "_BoundaryEventParent": to_task_model = parent is_boundary_parent = True # Will need to complete this task at the end so we are on the correct process. @@ -1325,7 +1326,7 @@ class ProcessInstanceProcessor: # If this as a boundary event parent, run it, so we get back to an active task. if is_boundary_parent: - processor.do_engine_steps(execution_strategy_name='one_at_a_time') + processor.do_engine_steps(execution_strategy_name="one_at_a_time") processor.save() processor.suspend() 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 2495a6b51..92e19919a 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 @@ -435,11 +435,11 @@ class TestProcessInstanceProcessor(BaseTest): assert process_instance.status == "complete" def test_properly_resets_process_on_tasks_with_boundary_events( - self, - app: Flask, - client: FlaskClient, - with_db_and_bpmn_file_cleanup: None, - with_super_admin_user: UserModel, + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, ) -> None: self.create_process_group_with_api(client, with_super_admin_user, "test_group", "test_group") process_model = load_test_spec( @@ -454,21 +454,23 @@ class TestProcessInstanceProcessor(BaseTest): assert len(process_instance.active_human_tasks) == 1 human_task_one = process_instance.active_human_tasks[0] spiff_manual_task = processor.bpmn_process_instance.get_task_from_id(UUID(human_task_one.task_id)) - ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, with_super_admin_user, human_task_one) + ProcessInstanceService.complete_form_task( + processor, spiff_manual_task, {}, with_super_admin_user, human_task_one + ) assert ( - len(process_instance.active_human_tasks) == 1 + len(process_instance.active_human_tasks) == 1 ), "expected 1 active human tasks after 2nd one is completed" - assert process_instance.active_human_tasks[0].task_title == 'Final' + assert process_instance.active_human_tasks[0].task_title == "Final" # Reset the process back to the task within the call activity that contains a timer_boundary event. - reset_to_spiff_task = processor.__class__.get_task_by_bpmn_identifier( - 'manual_task_1', processor.bpmn_process_instance + reset_to_spiff_task: SpiffTask = processor.__class__.get_task_by_bpmn_identifier( + "manual_task_1", processor.bpmn_process_instance ) processor.suspend() processor = ProcessInstanceProcessor(process_instance) ProcessInstanceProcessor.reset_process(process_instance, str(reset_to_spiff_task.id)) human_task_one = process_instance.active_human_tasks[0] - assert human_task_one.task_title == 'Manual Task #1' + assert human_task_one.task_title == "Manual Task #1" processor = ProcessInstanceProcessor(process_instance) processor.manual_complete_task(str(spiff_manual_task.id), execute=True) processor = ProcessInstanceProcessor(process_instance) @@ -476,10 +478,11 @@ class TestProcessInstanceProcessor(BaseTest): processor.do_engine_steps(save=True) process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first() - assert (len(process_instance.active_human_tasks) == 1) - assert process_instance.active_human_tasks[0].task_title == 'Final', \ - "once we reset, resume, and complete the task, we should be back to the Final step again, and not" \ + assert len(process_instance.active_human_tasks) == 1 + assert process_instance.active_human_tasks[0].task_title == "Final", ( + "once we reset, resume, and complete the task, we should be back to the Final step again, and not" "stuck waiting for the call activity to complete (which was happening in a bug I'm fixing right now)" + ) def test_properly_saves_tasks_when_running( self, From fa85a06efef976ca2b0af253012ca6a179fd0083 Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 4 May 2023 08:00:37 -0400 Subject: [PATCH 5/9] allow adding waiting_for and task_title columns in instance list page --- .../routes/process_instances_controller.py | 4 ++-- .../services/process_instance_report_service.py | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) 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 fa8f62de1..e7dfa206c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -298,8 +298,8 @@ def process_instance_report_show( def process_instance_report_column_list( process_model_identifier: Optional[str] = None, ) -> flask.wrappers.Response: - """Process_instance_report_column_list.""" table_columns = ProcessInstanceReportService.builtin_column_options() + system_report_column_options = ProcessInstanceReportService.system_report_column_options() columns_for_metadata_query = ( db.session.query(ProcessInstanceMetadataModel.key) .order_by(ProcessInstanceMetadataModel.key) @@ -315,7 +315,7 @@ def process_instance_report_column_list( columns_for_metadata_strings = [ {"Header": i[0], "accessor": i[0], "filterable": True} for i in columns_for_metadata ] - return make_response(jsonify(table_columns + columns_for_metadata_strings), 200) + return make_response(jsonify(table_columns + system_report_column_options + columns_for_metadata_strings), 200) def process_instance_show_for_me( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py index 57af17eae..89417e637 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py @@ -320,7 +320,7 @@ class ProcessInstanceReportService: @classmethod def builtin_column_options(cls) -> list[ReportMetadataColumn]: - """Builtin_column_options.""" + """Columns that are actually in the process instance table.""" return_value: list[ReportMetadataColumn] = [ {"Header": "Id", "accessor": "id", "filterable": False}, { @@ -339,6 +339,15 @@ class ProcessInstanceReportService: ] return return_value + @classmethod + def system_report_column_options(cls) -> list[ReportMetadataColumn]: + """Columns that are used with certain system reports.""" + return_value: list[ReportMetadataColumn] = [ + {"Header": "Task", "accessor": "task_title", "filterable": False}, + {"Header": "Waiting For", "accessor": "waiting_for", "filterable": False}, + ] + return return_value + @classmethod def get_filter_value(cls, filters: list[FilterValue], filter_key: str) -> Any: for filter in filters: From f65b301635148ee7f905cf324f9afe8ec72df1ab Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 4 May 2023 09:45:01 -0400 Subject: [PATCH 6/9] do not raise errors if a process cannot be found in the spec reference cache when trying to get a task trace for an error --- .../spiffworkflow_backend/config/__init__.py | 12 ++--- .../exceptions/api_error.py | 4 +- .../models/process_instance.py | 6 +-- .../services/process_instance_processor.py | 12 ++--- .../services/task_service.py | 17 +++++-- .../integration/test_for_good_errors.py | 46 +++++++++---------- 6 files changed, 53 insertions(+), 44 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py b/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py index eaf67f6c9..7711c36f9 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/__init__.py @@ -18,13 +18,13 @@ def setup_database_uri(app: Flask) -> None: if app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_URI") is None: database_name = f"spiffworkflow_backend_{app.config['ENV_IDENTIFIER']}" if app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_TYPE") == "sqlite": - app.config[ - "SQLALCHEMY_DATABASE_URI" - ] = f"sqlite:///{app.instance_path}/db_{app.config['ENV_IDENTIFIER']}.sqlite3" + app.config["SQLALCHEMY_DATABASE_URI"] = ( + f"sqlite:///{app.instance_path}/db_{app.config['ENV_IDENTIFIER']}.sqlite3" + ) elif app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_TYPE") == "postgres": - app.config[ - "SQLALCHEMY_DATABASE_URI" - ] = f"postgresql://spiffworkflow_backend:spiffworkflow_backend@localhost:5432/{database_name}" + app.config["SQLALCHEMY_DATABASE_URI"] = ( + f"postgresql://spiffworkflow_backend:spiffworkflow_backend@localhost:5432/{database_name}" + ) else: # use pswd to trick flake8 with hardcoded passwords db_pswd = app.config.get("SPIFFWORKFLOW_BACKEND_DATABASE_PASSWORD") diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py index f5abf4231..9cd2c4d31 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py @@ -127,8 +127,8 @@ class ApiError(Exception): instance.task_trace = TaskModelError.get_task_trace(task_model) try: - spec_reference = TaskService.get_spec_reference_from_bpmn_process(task_model.bpmn_process) - instance.file_name = spec_reference.file_name + spec_reference_filename = TaskService.get_spec_filename_from_bpmn_process(task_model.bpmn_process) + instance.file_name = spec_reference_filename except Exception as exception: current_app.logger.error(exception) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py index 303532af5..b3ab709df 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py @@ -129,9 +129,9 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel): def serialized_with_metadata(self) -> dict[str, Any]: process_instance_attributes = self.serialized process_instance_attributes["process_metadata"] = self.process_metadata - process_instance_attributes[ - "process_model_with_diagram_identifier" - ] = self.process_model_with_diagram_identifier + process_instance_attributes["process_model_with_diagram_identifier"] = ( + self.process_model_with_diagram_identifier + ) return process_instance_attributes @property 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 d2f532327..938c7868f 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -423,9 +423,9 @@ class ProcessInstanceProcessor: tld.process_instance_id = process_instance_model.id # we want this to be the fully qualified path to the process model including all group subcomponents - current_app.config[ - "THREAD_LOCAL_DATA" - ].process_model_identifier = f"{process_instance_model.process_model_identifier}" + current_app.config["THREAD_LOCAL_DATA"].process_model_identifier = ( + f"{process_instance_model.process_model_identifier}" + ) self.process_instance_model = process_instance_model self.process_model_service = ProcessModelService() @@ -585,9 +585,9 @@ class ProcessInstanceProcessor: bpmn_subprocess_definition.bpmn_identifier ] = bpmn_process_definition_dict spiff_bpmn_process_dict["subprocess_specs"][bpmn_subprocess_definition.bpmn_identifier]["task_specs"] = {} - bpmn_subprocess_definition_bpmn_identifiers[ - bpmn_subprocess_definition.id - ] = bpmn_subprocess_definition.bpmn_identifier + bpmn_subprocess_definition_bpmn_identifiers[bpmn_subprocess_definition.id] = ( + bpmn_subprocess_definition.bpmn_identifier + ) task_definitions = TaskDefinitionModel.query.filter( TaskDefinitionModel.bpmn_process_definition_id.in_( # type: ignore diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index b0f1c2227..1cea59092 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py @@ -89,15 +89,15 @@ class TaskModelError(Exception): task_definition = task_model.task_definition task_bpmn_name = TaskService.get_name_for_display(task_definition) bpmn_process = task_model.bpmn_process - spec_reference = TaskService.get_spec_reference_from_bpmn_process(bpmn_process) + spec_reference_filename = TaskService.get_spec_filename_from_bpmn_process(bpmn_process) - task_trace = [f"{task_bpmn_name} ({spec_reference.file_name})"] + task_trace = [f"{task_bpmn_name} ({spec_reference_filename})"] while bpmn_process.guid is not None: caller_task_model = TaskModel.query.filter_by(guid=bpmn_process.guid).first() bpmn_process = BpmnProcessModel.query.filter_by(id=bpmn_process.direct_parent_process_id).first() - spec_reference = TaskService.get_spec_reference_from_bpmn_process(bpmn_process) + spec_reference_filename = TaskService.get_spec_filename_from_bpmn_process(bpmn_process) task_trace.append( - f"{TaskService.get_name_for_display(caller_task_model.task_definition)} ({spec_reference.file_name})" + f"{TaskService.get_name_for_display(caller_task_model.task_definition)} ({spec_reference_filename})" ) return task_trace @@ -630,6 +630,15 @@ class TaskService: result.append({"event": event_definition, "label": extensions["signalButtonLabel"]}) return result + @classmethod + def get_spec_filename_from_bpmn_process(cls, bpmn_process: BpmnProcessModel) -> Optional[str]: + """Just return the filename if the bpmn process is found in spec reference cache.""" + try: + filename: Optional[str] = cls.get_spec_reference_from_bpmn_process(bpmn_process).file_name + return filename + except SpecReferenceNotFoundError: + return None + @classmethod def get_spec_reference_from_bpmn_process(cls, bpmn_process: BpmnProcessModel) -> SpecReferenceCache: """Get the bpmn file for a given task model. diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_for_good_errors.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_for_good_errors.py index 59c1499a2..790545946 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_for_good_errors.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_for_good_errors.py @@ -15,27 +15,6 @@ from spiffworkflow_backend.routes.tasks_controller import _dequeued_interstitial class TestForGoodErrors(BaseTest): """Assure when certain errors happen when rendering a jinaj2 error that it makes some sense.""" - def get_next_user_task( - self, - process_instance_id: int, - client: FlaskClient, - with_super_admin_user: UserModel, - ) -> Any: - # Call this to assure all engine-steps are fully processed before we search for human tasks. - _dequeued_interstitial_stream(process_instance_id) - - """Returns the next available user task for a given process instance, if possible.""" - human_tasks = ( - db.session.query(HumanTaskModel).filter(HumanTaskModel.process_instance_id == process_instance_id).all() - ) - assert len(human_tasks) > 0, "No human tasks found for process." - human_task = human_tasks[0] - response = client.get( - f"/v1.0/tasks/{process_instance_id}/{human_task.task_id}", - headers=self.logged_in_headers(with_super_admin_user), - ) - return response - def test_invalid_form( self, app: Flask, @@ -61,7 +40,7 @@ class TestForGoodErrors(BaseTest): headers=self.logged_in_headers(with_super_admin_user), ) assert response.status_code == 200 - response = self.get_next_user_task(process_instance_id, client, with_super_admin_user) + response = self._get_next_user_task(process_instance_id, client, with_super_admin_user) assert response.json is not None assert response.json["error_type"] == "TemplateSyntaxError" assert response.json["line_number"] == 3 @@ -88,7 +67,7 @@ class TestForGoodErrors(BaseTest): f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model.id)}/{process_instance.id}/run", headers=self.logged_in_headers(with_super_admin_user), ) - response = self.get_next_user_task(process_instance.id, client, with_super_admin_user) + response = self._get_next_user_task(process_instance.id, client, with_super_admin_user) assert response.status_code == 400 assert response.json is not None @@ -99,3 +78,24 @@ class TestForGoodErrors(BaseTest): assert "instructions for end user" in response.json["message"] assert "Jinja2" in response.json["message"] assert "unexpected '='" in response.json["message"] + + def _get_next_user_task( + self, + process_instance_id: int, + client: FlaskClient, + with_super_admin_user: UserModel, + ) -> Any: + # Call this to assure all engine-steps are fully processed before we search for human tasks. + _dequeued_interstitial_stream(process_instance_id) + + """Returns the next available user task for a given process instance, if possible.""" + human_tasks = ( + db.session.query(HumanTaskModel).filter(HumanTaskModel.process_instance_id == process_instance_id).all() + ) + assert len(human_tasks) > 0, "No human tasks found for process." + human_task = human_tasks[0] + response = client.get( + f"/v1.0/tasks/{process_instance_id}/{human_task.task_id}", + headers=self.logged_in_headers(with_super_admin_user), + ) + return response From 81a2a5d383bafeb1ce3dfc961c5b6dbb6adf3d04 Mon Sep 17 00:00:00 2001 From: burnettk Date: Thu, 4 May 2023 10:23:52 -0400 Subject: [PATCH 7/9] adding permissions for send-event --- .../services/authorization_service.py | 8 ++++++-- .../unit/test_authorization_service.py | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 8842c3ece..fd3110543 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -84,6 +84,7 @@ PATH_SEGMENTS_FOR_PERMISSION_ALL = [ {"path": "/process-instance-terminate", "relevant_permissions": ["create"]}, {"path": "/process-data", "relevant_permissions": ["read"]}, {"path": "/process-data-file-download", "relevant_permissions": ["read"]}, + {"path": "/send-event", "relevant_permissions": ["create"]}, {"path": "/task-data", "relevant_permissions": ["read", "update"]}, ] @@ -531,9 +532,12 @@ class AuthorizationService: # we were thinking that if you can start an instance, you ought to be able to: # 1. view your own instances. # 2. view the logs for these instances. + # 3. click on buttons in user tasks that sends signal events to these instances if permission_set == "start": - target_uri = f"/process-instances/{process_related_path_segment}" - permissions_to_assign.append(PermissionToAssign(permission="create", target_uri=target_uri)) + path_prefixes_that_allow_create_access = ["process-instances", "send-event"] + for path_prefix in path_prefixes_that_allow_create_access: + target_uri = f"/{path_prefix}/{process_related_path_segment}" + permissions_to_assign.append(PermissionToAssign(permission="create", target_uri=target_uri)) # giving people access to all logs for an instance actually gives them a little bit more access # than would be optimal. ideally, you would only be able to view the logs for instances that you started diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py index 2d43963df..7a55698d1 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -125,6 +125,7 @@ class TestAuthorizationService(BaseTest): expected_permissions = sorted( [ ("/event-error-details/some-process-group:some-process-model:*", "read"), + ("/send-event/some-process-group:some-process-model:*", "create"), ("/logs/some-process-group:some-process-model:*", "read"), ("/logs/typeahead-filter-values/some-process-group:some-process-model:*", "read"), ("/process-data/some-process-group:some-process-model:*", "read"), From 968a2fb1866f159e4386d2ba071ab1641d99feae Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 4 May 2023 11:20:38 -0400 Subject: [PATCH 8/9] added endpoint for send-signal-event for users so we can handle permissions more easily w/ burnettk --- .../src/spiffworkflow_backend/api.yml | 25 +++++++++++-- .../routes/process_api_blueprint.py | 19 ---------- .../routes/process_instances_controller.py | 35 +++++++++++++++++++ .../services/authorization_service.py | 4 +-- .../unit/test_authorization_service.py | 1 - .../src/routes/ProcessInstanceShow.tsx | 2 +- 6 files changed, 60 insertions(+), 26 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 0ea5291ce..656ddcfc9 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -1588,9 +1588,9 @@ paths: required: true description: The unique id of the process instance schema: - type: string + type: integer post: - operationId: spiffworkflow_backend.routes.process_api_blueprint.send_bpmn_event + operationId: spiffworkflow_backend.routes.process_instances_controller.send_bpmn_event summary: Send a BPMN event to the process tags: - Process Instances @@ -1779,6 +1779,27 @@ paths: schema: $ref: "#/components/schemas/OkTrue" + /tasks/{process_instance_id}/send-user-signal-event: + parameters: + - name: process_instance_id + in: path + required: true + description: The unique id of the process instance + schema: + type: integer + post: + operationId: spiffworkflow_backend.routes.process_instances_controller.send_user_signal_event + summary: Send a BPMN event to the process + tags: + - Process Instances + responses: + "200": + description: Event Sent Successfully + content: + application/json: + schema: + $ref: "#/components/schemas/Workflow" + /messages: parameters: - name: process_instance_id diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py index 3380d76de..e7b8b3fde 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -189,25 +189,6 @@ def _get_required_parameter_or_raise(parameter: str, post_body: dict[str, Any]) return return_value -def send_bpmn_event( - modified_process_model_identifier: str, - process_instance_id: str, - body: Dict, -) -> Response: - """Send a bpmn event to a workflow.""" - process_instance = ProcessInstanceModel.query.filter(ProcessInstanceModel.id == int(process_instance_id)).first() - if process_instance: - processor = ProcessInstanceProcessor(process_instance) - processor.send_bpmn_event(body) - task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task()) - return make_response(jsonify(task), 200) - else: - raise ApiError( - error_code="send_bpmn_event_error", - message=f"Could not send event to Instance: {process_instance_id}", - ) - - def _commit_and_push_to_git(message: str) -> None: """Commit_and_push_to_git.""" if current_app.config["SPIFFWORKFLOW_BACKEND_GIT_COMMIT_ON_SAVE"]: 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 fa8f62de1..f13507ede 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -642,6 +642,41 @@ def process_instance_find_by_id( return make_response(jsonify(response_json), 200) +def send_user_signal_event( + process_instance_id: int, + body: Dict, +) -> Response: + """Send a user signal event to a process instance.""" + process_instance = _find_process_instance_for_me_or_raise(process_instance_id) + return _send_bpmn_event(process_instance, body) + + +def send_bpmn_event( + modified_process_model_identifier: str, + process_instance_id: int, + body: Dict, +) -> Response: + """Send a bpmn event to a process instance.""" + process_instance = ProcessInstanceModel.query.filter(ProcessInstanceModel.id == int(process_instance_id)).first() + if process_instance: + return _send_bpmn_event(process_instance, body) + else: + raise ApiError( + error_code="send_bpmn_event_error", + message=f"Could not send event to Instance: {process_instance_id}", + ) + + +def _send_bpmn_event( + process_instance: ProcessInstanceModel, + body: dict +) -> Response: + processor = ProcessInstanceProcessor(process_instance) + processor.send_bpmn_event(body) + task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task()) + return make_response(jsonify(task), 200) + + def _get_process_instance( modified_process_model_identifier: str, process_instance: ProcessInstanceModel, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index fd3110543..68f99636d 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -84,7 +84,6 @@ PATH_SEGMENTS_FOR_PERMISSION_ALL = [ {"path": "/process-instance-terminate", "relevant_permissions": ["create"]}, {"path": "/process-data", "relevant_permissions": ["read"]}, {"path": "/process-data-file-download", "relevant_permissions": ["read"]}, - {"path": "/send-event", "relevant_permissions": ["create"]}, {"path": "/task-data", "relevant_permissions": ["read", "update"]}, ] @@ -532,9 +531,8 @@ class AuthorizationService: # we were thinking that if you can start an instance, you ought to be able to: # 1. view your own instances. # 2. view the logs for these instances. - # 3. click on buttons in user tasks that sends signal events to these instances if permission_set == "start": - path_prefixes_that_allow_create_access = ["process-instances", "send-event"] + path_prefixes_that_allow_create_access = ["process-instances"] for path_prefix in path_prefixes_that_allow_create_access: target_uri = f"/{path_prefix}/{process_related_path_segment}" permissions_to_assign.append(PermissionToAssign(permission="create", target_uri=target_uri)) diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py index 7a55698d1..2d43963df 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -125,7 +125,6 @@ class TestAuthorizationService(BaseTest): expected_permissions = sorted( [ ("/event-error-details/some-process-group:some-process-model:*", "read"), - ("/send-event/some-process-group:some-process-model:*", "create"), ("/logs/some-process-group:some-process-model:*", "read"), ("/logs/typeahead-filter-values/some-process-group:some-process-model:*", "read"), ("/process-data/some-process-group:some-process-model:*", "read"), diff --git a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx b/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx index f1718ab7b..48e87877a 100644 --- a/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessInstanceShow.tsx @@ -733,7 +733,7 @@ export default function ProcessInstanceShow({ variant }: OwnProps) { if ('payload' in eventToSend) eventToSend.payload = JSON.parse(eventPayload); HttpService.makeCallToBackend({ - path: `/send-event/${modifiedProcessModelId}/${params.process_instance_id}`, + path: targetUris.processInstanceSendEventPath, httpMethod: 'POST', successCallback: saveTaskDataResult, failureCallback: addError, From 5c1d106aa88f4ad4b215d1a5c1654024cdcaf512 Mon Sep 17 00:00:00 2001 From: jasquat Date: Thu, 4 May 2023 11:31:37 -0400 Subject: [PATCH 9/9] pyl w/ burnettk --- .../spiffworkflow_backend/routes/process_api_blueprint.py | 1 - .../routes/process_instances_controller.py | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py index e7b8b3fde..df749d99e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -31,7 +31,6 @@ from spiffworkflow_backend.services.process_caller_service import ProcessCallerS from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, ) -from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService from spiffworkflow_backend.services.process_model_service import ProcessModelService 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 f13507ede..df992e384 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -667,10 +667,7 @@ def send_bpmn_event( ) -def _send_bpmn_event( - process_instance: ProcessInstanceModel, - body: dict -) -> Response: +def _send_bpmn_event(process_instance: ProcessInstanceModel, body: dict) -> Response: processor = ProcessInstanceProcessor(process_instance) processor.send_bpmn_event(body) task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task())