diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py index 2a516f9d..201e6b03 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_groups_controller.py @@ -20,11 +20,29 @@ from spiffworkflow_backend.routes.process_api_blueprint import ( _un_modify_modified_process_model_id, ) from spiffworkflow_backend.services.process_model_service import ProcessModelService +from spiffworkflow_backend.services.process_model_service import ( + ProcessModelWithInstancesNotDeletableError, +) def process_group_create(body: dict) -> flask.wrappers.Response: """Add_process_group.""" process_group = ProcessGroup(**body) + + if ProcessModelService.is_process_model_identifier(process_group.id): + raise ApiError( + error_code="process_model_with_id_already_exists", + message=f"Process Model with given id already exists: {process_group.id}", + status_code=400, + ) + + if ProcessModelService.is_process_group_identifier(process_group.id): + raise ApiError( + error_code="process_group_with_id_already_exists", + message=f"Process Group with given id already exists: {process_group.id}", + status_code=400, + ) + ProcessModelService.add_process_group(process_group) _commit_and_push_to_git( f"User: {g.user.username} added process group {process_group.id}" @@ -35,7 +53,16 @@ def process_group_create(body: dict) -> flask.wrappers.Response: def process_group_delete(modified_process_group_id: str) -> flask.wrappers.Response: """Process_group_delete.""" process_group_id = _un_modify_modified_process_model_id(modified_process_group_id) - ProcessModelService().process_group_delete(process_group_id) + + try: + ProcessModelService().process_group_delete(process_group_id) + except ProcessModelWithInstancesNotDeletableError as exception: + raise ApiError( + error_code="existing_instances", + message=str(exception), + status_code=400, + ) from exception + _commit_and_push_to_git( f"User: {g.user.username} deleted process group {process_group_id}" ) @@ -54,6 +81,13 @@ def process_group_update( } process_group_id = _un_modify_modified_process_model_id(modified_process_group_id) + if not ProcessModelService.is_process_group_identifier(process_group_id): + raise ApiError( + error_code="process_group_does_not_exist", + message=f"Process Group with given id does not exist: {process_group_id}", + status_code=400, + ) + process_group = ProcessGroup(id=process_group_id, **body_filtered) ProcessModelService.update_process_group(process_group) _commit_and_push_to_git( 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 8a6e6306..36d5e69b 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py @@ -39,6 +39,9 @@ from spiffworkflow_backend.services.process_instance_report_service import ( ProcessInstanceReportService, ) from spiffworkflow_backend.services.process_model_service import ProcessModelService +from spiffworkflow_backend.services.process_model_service import ( + ProcessModelWithInstancesNotDeletableError, +) from spiffworkflow_backend.services.spec_file_service import ( ProcessModelFileInvalidError, ) @@ -75,6 +78,24 @@ def process_model_create( status_code=400, ) + if ProcessModelService.is_process_model_identifier(process_model_info.id): + raise ApiError( + error_code="process_model_with_id_already_exists", + message=( + f"Process Model with given id already exists: {process_model_info.id}" + ), + status_code=400, + ) + + if ProcessModelService.is_process_group_identifier(process_model_info.id): + raise ApiError( + error_code="process_group_with_id_already_exists", + message=( + f"Process Group with given id already exists: {process_model_info.id}" + ), + status_code=400, + ) + ProcessModelService.add_process_model(process_model_info) _commit_and_push_to_git( f"User: {g.user.username} created process model {process_model_info.id}" @@ -91,7 +112,15 @@ def process_model_delete( ) -> flask.wrappers.Response: """Process_model_delete.""" process_model_identifier = modified_process_model_identifier.replace(":", "/") - ProcessModelService().process_model_delete(process_model_identifier) + try: + ProcessModelService().process_model_delete(process_model_identifier) + except ProcessModelWithInstancesNotDeletableError as exception: + raise ApiError( + error_code="existing_instances", + message=str(exception), + status_code=400, + ) from exception + _commit_and_push_to_git( f"User: {g.user.username} deleted process model {process_model_identifier}" ) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py index 60574508..5c863ab8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_service.py @@ -27,6 +27,10 @@ from spiffworkflow_backend.services.user_service import UserService T = TypeVar("T") +class ProcessModelWithInstancesNotDeletableError(Exception): + """ProcessModelWithInstancesNotDeletableError.""" + + class ProcessModelService(FileSystemService): """ProcessModelService.""" @@ -45,7 +49,7 @@ class ProcessModelService(FileSystemService): return path.replace(os.sep, "/") @classmethod - def is_group(cls, path: str) -> bool: + def is_process_group(cls, path: str) -> bool: """Is_group.""" group_json_path = os.path.join(path, cls.PROCESS_GROUP_JSON_FILE) if os.path.exists(group_json_path): @@ -53,8 +57,8 @@ class ProcessModelService(FileSystemService): return False @classmethod - def is_group_identifier(cls, process_group_identifier: str) -> bool: - """Is_group_identifier.""" + def is_process_group_identifier(cls, process_group_identifier: str) -> bool: + """Is_process_group_identifier.""" if os.path.exists(FileSystemService.root_path()): process_group_path = os.path.abspath( os.path.join( @@ -64,21 +68,21 @@ class ProcessModelService(FileSystemService): ), ) ) - return cls.is_group(process_group_path) + return cls.is_process_group(process_group_path) return False @classmethod - def is_model(cls, path: str) -> bool: - """Is_model.""" + def is_process_model(cls, path: str) -> bool: + """Is_process_model.""" model_json_path = os.path.join(path, cls.PROCESS_MODEL_JSON_FILE) if os.path.exists(model_json_path): return True return False @classmethod - def is_model_identifier(cls, process_model_identifier: str) -> bool: - """Is_model_identifier.""" + def is_process_model_identifier(cls, process_model_identifier: str) -> bool: + """Is_process_model_identifier.""" if os.path.exists(FileSystemService.root_path()): process_model_path = os.path.abspath( os.path.join( @@ -88,7 +92,7 @@ class ProcessModelService(FileSystemService): ), ) ) - return cls.is_model(process_model_path) + return cls.is_process_model(process_model_path) return False @@ -153,12 +157,9 @@ class ProcessModelService(FileSystemService): ProcessInstanceModel.process_model_identifier == process_model_id ).all() if len(instances) > 0: - raise ApiError( - error_code="existing_instances", - message=( - f"We cannot delete the model `{process_model_id}`, there are" - " existing instances that depend on it." - ), + raise ProcessModelWithInstancesNotDeletableError( + f"We cannot delete the model `{process_model_id}`, there are" + " existing instances that depend on it." ) process_model = self.get_process_model(process_model_id) path = self.workflow_path(process_model) @@ -199,7 +200,7 @@ class ProcessModelService(FileSystemService): model_path = os.path.abspath( os.path.join(FileSystemService.root_path(), process_model_id) ) - if cls.is_model(model_path): + if cls.is_process_model(model_path): return cls.get_process_model_from_relative_path(process_model_id) raise ProcessEntityNotFoundError("process_model_not_found") @@ -303,7 +304,7 @@ class ProcessModelService(FileSystemService): FileSystemService.id_string_to_relative_path(process_group_id), ) ) - if cls.is_group(process_group_path): + if cls.is_process_group(process_group_path): return cls.find_or_create_process_group( process_group_path, find_direct_nested_items=find_direct_nested_items, @@ -351,7 +352,7 @@ class ProcessModelService(FileSystemService): for _root, dirs, _files in os.walk(group_path): for dir in dirs: model_dir = os.path.join(group_path, dir) - if ProcessModelService.is_model(model_dir): + if ProcessModelService.is_process_model(model_dir): process_model = self.get_process_model(model_dir) all_nested_models.append(process_model) return all_nested_models @@ -369,13 +370,10 @@ class ProcessModelService(FileSystemService): if len(instances) > 0: problem_models.append(process_model) if len(problem_models) > 0: - raise ApiError( - error_code="existing_instances", - message=( - f"We cannot delete the group `{process_group_id}`, there are" - " models with existing instances inside the group." - f" {problem_models}" - ), + raise ProcessModelWithInstancesNotDeletableError( + f"We cannot delete the group `{process_group_id}`, there are" + " models with existing instances inside the group." + f" {problem_models}" ) shutil.rmtree(path) self.cleanup_process_group_display_order() @@ -406,7 +404,7 @@ class ProcessModelService(FileSystemService): process_groups = [] for item in directory_items: # if item.is_dir() and not item.name[0] == ".": - if item.is_dir() and cls.is_group(item): # type: ignore + if item.is_dir() and cls.is_process_group(item): # type: ignore scanned_process_group = cls.find_or_create_process_group(item.path) process_groups.append(scanned_process_group) return process_groups @@ -453,12 +451,12 @@ class ProcessModelService(FileSystemService): for nested_item in nested_items: if nested_item.is_dir(): # TODO: check whether this is a group or model - if cls.is_group(nested_item.path): + if cls.is_process_group(nested_item.path): # This is a nested group process_group.process_groups.append( cls.find_or_create_process_group(nested_item.path) ) - elif ProcessModelService.is_model(nested_item.path): + elif ProcessModelService.is_process_model(nested_item.path): process_group.process_models.append( cls.__scan_process_model( nested_item.path, diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py b/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py index 7c8515db..f265bf54 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/helpers/base_test.py @@ -139,7 +139,7 @@ class BaseTest: process_group_path = os.path.abspath( os.path.join(FileSystemService.root_path(), process_group_id) ) - if ProcessModelService.is_group(process_group_path): + if ProcessModelService.is_process_group(process_group_path): if exception_notification_addresses is None: exception_notification_addresses = [] @@ -259,9 +259,9 @@ class BaseTest: There must be an existing process model to instantiate. """ - if not ProcessModelService.is_model_identifier(test_process_model_id): + if not ProcessModelService.is_process_model_identifier(test_process_model_id): dirname = os.path.dirname(test_process_model_id) - if not ProcessModelService.is_group_identifier(dirname): + if not ProcessModelService.is_process_group_identifier(dirname): process_group = ProcessGroup(id=dirname, display_name=dirname) ProcessModelService.add_process_group(process_group) basename = os.path.basename(test_process_model_id) diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_acceptance_test_fixtures.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_acceptance_test_fixtures.py index c738c7f6..e4eeecf1 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_acceptance_test_fixtures.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_acceptance_test_fixtures.py @@ -18,15 +18,15 @@ def test_start_dates_are_one_hour_apart(app: Flask) -> None: ) group_identifier = os.path.dirname(process_model_identifier) parent_group_identifier = os.path.dirname(group_identifier) - if not ProcessModelService.is_group(parent_group_identifier): + if not ProcessModelService.is_process_group(parent_group_identifier): process_group = ProcessGroup( id=parent_group_identifier, display_name=parent_group_identifier ) ProcessModelService.add_process_group(process_group) - if not ProcessModelService.is_group(group_identifier): + if not ProcessModelService.is_process_group(group_identifier): process_group = ProcessGroup(id=group_identifier, display_name=group_identifier) ProcessModelService.add_process_group(process_group) - if not ProcessModelService.is_model(process_model_identifier): + if not ProcessModelService.is_process_model(process_model_identifier): process_model = ProcessModelInfo( id=process_model_identifier, display_name=process_model_identifier,