From d86c083dfec335d228d08d31ec4e3600bb74b427 Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 24 Jan 2023 15:15:48 -0500 Subject: [PATCH] do not allow deleting primary bpmn file and do not allow instantiating models without a primary bpmn file w/ burnettk --- .flake8 | 7 +++- spiffworkflow-backend/.flake8 | 7 +++- .../routes/process_instances_controller.py | 12 +++++++ .../routes/process_models_controller.py | 11 ++++++ .../integration/test_process_api.py | 34 +++++++++++++++++-- .../src/components/ProcessInstanceRun.tsx | 2 ++ 6 files changed, 69 insertions(+), 4 deletions(-) diff --git a/.flake8 b/.flake8 index 97353aae6..25482610d 100644 --- a/.flake8 +++ b/.flake8 @@ -10,6 +10,11 @@ rst-directives = deprecated per-file-ignores = # More specific globs seem to overwrite the more generic ones so we have # to split them out by directory + # So if you have a rule like: + # tests/*: D102,D103 + # and a rule like: + # tests/test_hey.py: D102 + # THEN, test_hey.py will NOT be excluding D103 # asserts are ok in tests spiffworkflow-backend/tests/*:S101,D102,D103,D101 @@ -37,4 +42,4 @@ per-file-ignores = # TODO: fix the S issues: # S607 Starting a process with a partial executable path # S605 Starting a process with a shell: Seems safe, but may be changed in the future, consider rewriting without shell - spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py:S607,S101,D103,S605 + spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py:S607,S101,S605,D102,D103,D101 diff --git a/spiffworkflow-backend/.flake8 b/spiffworkflow-backend/.flake8 index b42cf5281..66efd2c7f 100644 --- a/spiffworkflow-backend/.flake8 +++ b/spiffworkflow-backend/.flake8 @@ -10,6 +10,11 @@ rst-directives = deprecated per-file-ignores = # More specific globs seem to overwrite the more generic ones so we have # to split them out by directory + # So if you have a rule like: + # tests/*: D102,D103 + # and a rule like: + # tests/test_hey.py: D102 + # THEN, test_hey.py will NOT be excluding D103 # asserts are ok in tests tests/*:S101,D102,D103 @@ -34,4 +39,4 @@ per-file-ignores = # and ignore long comment line src/spiffworkflow_backend/services/logging_service.py:N802,B950 - tests/spiffworkflow_backend/integration/test_process_api.py:S607,S101,D103,S605 + tests/spiffworkflow_backend/integration/test_process_api.py:S607,S101,S605,D102,D103,D101 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 1271f929d..679c2fa70 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -72,6 +72,18 @@ def process_instance_create( process_model_identifier = _un_modify_modified_process_model_id( modified_process_model_identifier ) + + process_model = _get_process_model(process_model_identifier) + if process_model.primary_file_name is None: + raise ApiError( + error_code="process_model_missing_primary_bpmn_file", + message=( + f"Process Model '{process_model_identifier}' does not have a primary" + " bpmn file. One must be set in order to instantiate this model." + ), + status_code=400, + ) + process_instance = ( ProcessInstanceService.create_process_instance_from_process_model_identifier( process_model_identifier, g.user 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 694c01efd..19f064c9b 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py @@ -277,6 +277,17 @@ def process_model_file_delete( """Process_model_file_delete.""" process_model_identifier = modified_process_model_identifier.replace(":", "/") process_model = _get_process_model(process_model_identifier) + if process_model.primary_file_name == file_name: + raise ApiError( + error_code="process_model_file_cannot_be_deleted", + message=( + f"'{file_name}' is the primary bpmn file for" + f" '{process_model_identifier}' and cannot be deleted. Please set" + " another file as the primary before attempting to delete this one." + ), + status_code=400, + ) + try: SpecFileService.delete_file(process_model, file_name) except FileNotFoundError as exception: 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 ebc26ac6d..7213e31ce 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -922,6 +922,28 @@ class TestProcessApi(BaseTest): assert response.json is not None assert response.json["error_code"] == "process_model_file_cannot_be_found" + def test_process_model_file_delete_when_primary_file( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + with_super_admin_user: UserModel, + ) -> None: + process_model_identifier = self.create_group_and_model_with_bpmn( + client, with_super_admin_user + ) + modified_process_model_identifier = process_model_identifier.replace("/", ":") + + response = client.delete( + f"/v1.0/process-models/{modified_process_model_identifier}/files/random_fact.bpmn", + follow_redirects=True, + headers=self.logged_in_headers(with_super_admin_user), + ) + + assert response.status_code == 400 + assert response.json is not None + assert response.json["error_code"] == "process_model_file_cannot_be_deleted" + def test_process_model_file_delete( self, app: Flask, @@ -935,8 +957,16 @@ class TestProcessApi(BaseTest): ) modified_process_model_identifier = process_model_identifier.replace("/", ":") + self.create_spec_file( + client, + process_model_id=process_model_identifier, + file_name="second_file.json", + file_data=b"

HEY

", + user=with_super_admin_user, + ) + response = client.delete( - f"/v1.0/process-models/{modified_process_model_identifier}/files/random_fact.bpmn", + f"/v1.0/process-models/{modified_process_model_identifier}/files/second_file.json", follow_redirects=True, headers=self.logged_in_headers(with_super_admin_user), ) @@ -946,7 +976,7 @@ class TestProcessApi(BaseTest): assert response.json["ok"] response = client.get( - f"/v1.0/process-models/{modified_process_model_identifier}/files/random_fact.svg", + f"/v1.0/process-models/{modified_process_model_identifier}/files/second_file.json", headers=self.logged_in_headers(with_super_admin_user), ) assert response.status_code == 404 diff --git a/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx b/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx index b956abcc3..f7aafed79 100644 --- a/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx +++ b/spiffworkflow-frontend/src/components/ProcessInstanceRun.tsx @@ -116,9 +116,11 @@ export default function ProcessInstanceRun({ }; const processInstanceCreateAndRun = () => { + setErrorObject(null); HttpService.makeCallToBackend({ path: processInstanceCreatePath, successCallback: processModelRun, + failureCallback: setErrorObject, httpMethod: 'POST', }); };