From aa215c560e760c04fbda8077b49829569e21bcdc Mon Sep 17 00:00:00 2001 From: auslin-aot <99173163+auslin-aot@users.noreply.github.com> Date: Wed, 6 Nov 2024 15:31:36 +0530 Subject: [PATCH 1/3] FWF-3876: [Bugfix] Fixed issues while creating major version --- forms-flow-api/requirements.txt | 2 +- forms-flow-api/requirements/prod.txt | 2 +- .../models/form_process_mapper.py | 9 +- .../schemas/form_process_mapper.py | 2 +- .../services/form_process_mapper.py | 104 ++++++++++++------ .../formsflow_api/services/import_support.py | 6 +- 6 files changed, 80 insertions(+), 45 deletions(-) diff --git a/forms-flow-api/requirements.txt b/forms-flow-api/requirements.txt index 337fa78fe8..5b0a9897ec 100644 --- a/forms-flow-api/requirements.txt +++ b/forms-flow-api/requirements.txt @@ -28,7 +28,7 @@ exceptiongroup==1.2.2 flask-jwt-oidc==0.7.0 flask-marshmallow==1.2.1 flask-restx==1.3.0 -formsflow_api_utils @ git+https://github.com/auslin-aot/forms-flow-ai.git@fwf-3769-form-validation-changes#subdirectory=forms-flow-api-utils +formsflow_api_utils @ git+https://github.com/AOT-Technologies/forms-flow-ai.git@develop#subdirectory=forms-flow-api-utils gunicorn==23.0.0 h11==0.14.0 h2==4.1.0 diff --git a/forms-flow-api/requirements/prod.txt b/forms-flow-api/requirements/prod.txt index 668771c586..dfa553abd2 100644 --- a/forms-flow-api/requirements/prod.txt +++ b/forms-flow-api/requirements/prod.txt @@ -17,4 +17,4 @@ markupsafe PyJWT redis lxml -git+https://github.com/auslin-aot/forms-flow-ai.git@fwf-3769-form-validation-changes#subdirectory=forms-flow-api-utils \ No newline at end of file +git+https://github.com/AOT-Technologies/forms-flow-ai.git@develop#subdirectory=forms-flow-api-utils \ No newline at end of file diff --git a/forms-flow-api/src/formsflow_api/models/form_process_mapper.py b/forms-flow-api/src/formsflow_api/models/form_process_mapper.py index afbadf61ac..3b8f08c5bf 100644 --- a/forms-flow-api/src/formsflow_api/models/form_process_mapper.py +++ b/forms-flow-api/src/formsflow_api/models/form_process_mapper.py @@ -389,14 +389,9 @@ def find_all_active_forms( @classmethod def find_forms_by_title(cls, form_title, exclude_id) -> FormProcessMapper: """Find all form process mapper that matches the provided form title.""" - query = cls.query.filter( - and_( - FormProcessMapper.form_name == form_title, - FormProcessMapper.deleted.is_(False), - ) - ) + query = cls.query.filter(FormProcessMapper.form_name == form_title) if exclude_id is not None: - query = query.filter(FormProcessMapper.id != exclude_id) + query = query.filter(FormProcessMapper.parent_form_id != exclude_id) query = cls.tenant_authorization(query=query) return query.all() diff --git a/forms-flow-api/src/formsflow_api/schemas/form_process_mapper.py b/forms-flow-api/src/formsflow_api/schemas/form_process_mapper.py index 69c4a4456b..28dc850703 100644 --- a/forms-flow-api/src/formsflow_api/schemas/form_process_mapper.py +++ b/forms-flow-api/src/formsflow_api/schemas/form_process_mapper.py @@ -20,7 +20,7 @@ class Meta: # pylint: disable=too-few-public-methods process_name = fields.Str(data_key="processName") comments = fields.Str(data_key="comments") is_anonymous = fields.Bool(data_key="anonymous") - status = fields.Str(data_key="status") # active/inactive + status = fields.Str(data_key="status", allow_none=True) # active/inactive created_by = fields.Str(data_key="createdBy") created = fields.Str(data_key="created") modified_by = fields.Str(data_key="modifiedBy") diff --git a/forms-flow-api/src/formsflow_api/services/form_process_mapper.py b/forms-flow-api/src/formsflow_api/services/form_process_mapper.py index b805ec4876..26ef3f83e2 100644 --- a/forms-flow-api/src/formsflow_api/services/form_process_mapper.py +++ b/forms-flow-api/src/formsflow_api/services/form_process_mapper.py @@ -219,7 +219,7 @@ def mark_inactive_and_delete(form_process_mapper_id: int, **kwargs) -> None: @staticmethod def mark_unpublished(form_process_mapper_id): """Mark form process mapper as inactive.""" - mapper = FormProcessMapper.find_form_by_id_active_status( + mapper = FormProcessMapper.find_form_by_id( form_process_mapper_id=form_process_mapper_id ) if mapper: @@ -309,6 +309,7 @@ def validate_process_and_update_mapper(name, mapper): @staticmethod def mapper_create(mapper_json): """Service to handle mapper create.""" + current_app.logger.debug("Creating mapper..") mapper_json["taskVariables"] = json.dumps( mapper_json.get("taskVariables") or [] ) @@ -350,8 +351,9 @@ def create_default_process(cls, process_name, status=ProcessStatus.DRAFT, **kwar return process @staticmethod - def create_form(data, is_designer): + def create_form(data, is_designer): # pylint:disable=too-many-locals """Service to handle form create.""" + current_app.logger.info("Creating form..") # Initialize formio service and get formio token to create the form formio_service = FormioService() form_io_token = formio_service.get_formio_access_token() @@ -359,55 +361,91 @@ def create_form(data, is_designer): response = formio_service.create_form(data, form_io_token) form_id = response.get("_id") parent_form_id = data.get("parentFormId", form_id) - # create default data form mapper table - process_name = response.get("name") - # process key/Id doesn't support numbers & special characters at start - # special characters anywhere so clean them before setting as process key - process_name = FormProcessMapperService.clean_form_name(process_name) + # is_new_form=True if creating a new form, False if creating a new version + is_new_form = parent_form_id == form_id + process_key = None + anonymous = False + description = data.get("description", "") + task_variable = [] + current_app.logger.info(f"Creating new form {is_new_form}") + # If creating new version for a existing form, fetch process key, name from mapper + if not is_new_form: + current_app.logger.debug("Fetching details from mapper") + mapper = FormProcessMapper.get_latest_by_parent_form_id(parent_form_id) + process_name = mapper.process_name + process_key = mapper.process_key + anonymous = mapper.is_anonymous + description = mapper.description + task_variable = json.loads(mapper.task_variable) + else: + # if new form, form name is kept as process_name & process key + process_name = response.get("name") + # process key/Id doesn't support numbers & special characters at start + # special characters anywhere so clean them before setting as process key + process_name = FormProcessMapperService.clean_form_name(process_name) + mapper_data = { "formId": form_id, "formName": response.get("title"), - "description": data.get("description", ""), + "description": description, "formType": response.get("type"), "processKey": process_name, - "processName": process_name, + "processName": process_key if process_key else process_name, "formTypeChanged": True, "parentFormId": parent_form_id, "titleChanged": True, "formRevisionNumber": "V1", + "status": FormProcessMapperStatus.INACTIVE.value, + "anonymous": anonymous, + "task_variable": task_variable, } - # create default data for authorization of the resource - authorization_data = { - "application": { - "resourceId": parent_form_id, - "resourceDetails": {}, - "roles": [], - }, - "designer": { - "resourceId": parent_form_id, - "resourceDetails": {}, - "roles": [], - }, - "form": {"resourceId": parent_form_id, "resourceDetails": {}, "roles": []}, - } + mapper = FormProcessMapperService.mapper_create(mapper_data) - AuthorizationService.create_or_update_resource_authorization( - authorization_data, is_designer=is_designer - ) - # validate process key already exists, if exists append mapper id to process_key. - FormProcessMapperService.validate_process_and_update_mapper( - process_name, mapper - ) + current_app.logger.debug("Creating form log with clone..") FormHistoryService.create_form_log_with_clone( data={ **response, - "parentFormId": data.get("parentFormId", form_id), + "parentFormId": parent_form_id, "newVersion": True, "componentChanged": True, } ) - # create entry in process with default flow. - FormProcessMapperService.create_default_process(process_name) + if is_new_form: + # create default data for authorization of the resource + authorization_data = { + "application": { + "resourceId": parent_form_id, + "resourceDetails": {}, + "roles": [], + }, + "designer": { + "resourceId": parent_form_id, + "resourceDetails": {}, + "roles": [], + }, + "form": { + "resourceId": parent_form_id, + "resourceDetails": {}, + "roles": [], + }, + } + current_app.logger.debug( + "Creating default data for authorization of the resource.." + ) + AuthorizationService.create_or_update_resource_authorization( + authorization_data, is_designer=is_designer + ) + # validate process key already exists, if exists append mapper id to process_key. + updated_process_name = ( + FormProcessMapperService.validate_process_and_update_mapper( + process_name, mapper + ) + ) + process_name = ( + updated_process_name if updated_process_name else process_name + ) + # create entry in process with default flow. + FormProcessMapperService.create_default_process(process_name) return response def _get_form( # pylint: disable=too-many-arguments, too-many-positional-arguments diff --git a/forms-flow-api/src/formsflow_api/services/import_support.py b/forms-flow-api/src/formsflow_api/services/import_support.py index d0005712a7..791153134d 100644 --- a/forms-flow-api/src/formsflow_api/services/import_support.py +++ b/forms-flow-api/src/formsflow_api/services/import_support.py @@ -185,7 +185,9 @@ def validate_form_title(self, title, mapper): """Validate form tile in the form_process_mapper table.""" # Exclude the current mapper from the query current_app.logger.info(f"Validation for form title...{title}") - mappers = FormProcessMapper.find_forms_by_title(title, exclude_id=mapper.id) + mappers = FormProcessMapper.find_forms_by_title( + title, exclude_id=mapper.parent_form_id + ) if mappers: current_app.logger.debug(f"Other mappers matching the title- {mappers}") raise BusinessException(BusinessErrorCode.FORM_EXISTS) @@ -417,7 +419,7 @@ def import_form( "description": mapper.description if form_only else description, } FormProcessMapperService.mapper_create(mapper_data) - FormProcessMapperService.mark_inactive_and_delete(mapper.id) + FormProcessMapperService.mark_unpublished(mapper.id) else: current_app.logger.info("Form import minor version inprogress...") form_id = mapper.form_id From 0361ee750602084add77d5e61c1cf3307cd7e745 Mon Sep 17 00:00:00 2001 From: auslin-aot <99173163+auslin-aot@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:05:54 +0530 Subject: [PATCH 2/3] Changed error message for DMN and BPMN to generic one --- forms-flow-api/src/formsflow_api/constants/__init__.py | 2 +- forms-flow-api/tests/unit/api/test_process.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/forms-flow-api/src/formsflow_api/constants/__init__.py b/forms-flow-api/src/formsflow_api/constants/__init__.py index 4898356016..0d3c27d64a 100644 --- a/forms-flow-api/src/formsflow_api/constants/__init__.py +++ b/forms-flow-api/src/formsflow_api/constants/__init__.py @@ -94,7 +94,7 @@ class BusinessErrorCode(ErrorCodeMixin, Enum): HTTPStatus.BAD_REQUEST, ) PROCESS_EXISTS = ( - "The BPMN name or ID already exists. It must be unique.", + "The Process name or ID already exists. It must be unique.", HTTPStatus.BAD_REQUEST, ) INVALID_PROCESS_VALIDATION_INPUT = ( diff --git a/forms-flow-api/tests/unit/api/test_process.py b/forms-flow-api/tests/unit/api/test_process.py index 5d2be754cd..b865b0869b 100644 --- a/forms-flow-api/tests/unit/api/test_process.py +++ b/forms-flow-api/tests/unit/api/test_process.py @@ -381,7 +381,7 @@ def test_process_validation_invalid(self, app, client, session, jwt): assert response.status_code == 400 assert ( response.json.get("message") - == "The BPMN name or ID already exists. It must be unique." + == "The Process name or ID already exists. It must be unique." ) def test_process_validate_missing_params(app, client, session, jwt): From cd05c4725d1c6d77b958063fa763da0927c023d7 Mon Sep 17 00:00:00 2001 From: auslin-aot <99173163+auslin-aot@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:28:31 +0530 Subject: [PATCH 3/3] Added VIEW_DESIGNS permission to get by process key api --- forms-flow-api/src/formsflow_api/resources/process.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/forms-flow-api/src/formsflow_api/resources/process.py b/forms-flow-api/src/formsflow_api/resources/process.py index 4a9f5cd33a..4eac59949b 100644 --- a/forms-flow-api/src/formsflow_api/resources/process.py +++ b/forms-flow-api/src/formsflow_api/resources/process.py @@ -6,6 +6,7 @@ from flask_restx import Namespace, Resource, fields from formsflow_api_utils.utils import ( CREATE_DESIGNS, + VIEW_DESIGNS, auth, cors_preflight, profiletime, @@ -406,7 +407,7 @@ class ProcessResourceByProcessKey(Resource): """Resource for managing process by process key.""" @staticmethod - @auth.has_one_of_roles([CREATE_DESIGNS]) + @auth.has_one_of_roles([CREATE_DESIGNS, VIEW_DESIGNS]) @profiletime @API.doc( responses={