From b12af9f3bc625a6b12cfa0b8d908b378b6be9442 Mon Sep 17 00:00:00 2001 From: jasquat <2487833+jasquat@users.noreply.github.com> Date: Wed, 4 Oct 2023 15:23:42 -0400 Subject: [PATCH] Feature/pin form schemas to revision (#526) * task show loads the correct revision of the json schema form w/ burnettk * display error if form cannot be found at revision w/ burnettk --------- Co-authored-by: jasquat --- .../models/process_instance.py | 12 ++-- .../routes/process_instances_controller.py | 26 +++----- .../routes/tasks_controller.py | 64 +++++++++++++------ .../services/git_service.py | 37 +++++++++-- .../services/process_instance_service.py | 2 +- 5 files changed, 91 insertions(+), 50 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py index bd5be1c0..0f327032 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py @@ -89,13 +89,15 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel): start_in_seconds: int | None = db.Column(db.Integer, index=True) end_in_seconds: int | None = db.Column(db.Integer, index=True) task_updated_at_in_seconds: int = db.Column(db.Integer, nullable=True) - updated_at_in_seconds: int = db.Column(db.Integer) - created_at_in_seconds: int = db.Column(db.Integer) status: str = db.Column(db.String(50), index=True) - bpmn_version_control_type: str = db.Column(db.String(50)) - bpmn_version_control_identifier: str = db.Column(db.String(255)) - last_milestone_bpmn_name: str = db.Column(db.String(255)) + updated_at_in_seconds: int = db.Column(db.Integer) + created_at_in_seconds: int = db.Column(db.Integer) + + bpmn_version_control_type: str | None = db.Column(db.String(50)) + # this could also be a blank string for older instances since we were putting blank strings in here as well + bpmn_version_control_identifier: str | None = db.Column(db.String(255)) + last_milestone_bpmn_name: str | None = db.Column(db.String(255)) bpmn_xml_file_contents: str | None = None bpmn_xml_file_contents_retrieval_error: str | None = None 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 949c4c27..bc5af50e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -46,7 +46,6 @@ from spiffworkflow_backend.services.process_instance_queue_service import Proces from spiffworkflow_backend.services.process_instance_report_service import ProcessInstanceReportService from spiffworkflow_backend.services.process_instance_service import ProcessInstanceService from spiffworkflow_backend.services.process_model_service import ProcessModelService -from spiffworkflow_backend.services.spec_file_service import SpecFileService from spiffworkflow_backend.services.task_service import TaskService # from spiffworkflow_backend.services.process_instance_report_service import ( @@ -682,10 +681,6 @@ def _get_process_instance( process_identifier: str | None = None, ) -> flask.wrappers.Response: process_model_identifier = modified_process_model_identifier.replace(":", "/") - try: - current_version_control_revision = GitService.get_current_revision() - except GitCommandError: - current_version_control_revision = "" process_model_with_diagram = None name_of_file_with_diagram = None @@ -706,19 +701,14 @@ def _get_process_instance( if process_model_with_diagram and name_of_file_with_diagram: bpmn_xml_file_contents = None - if process_instance.bpmn_version_control_identifier == current_version_control_revision: - bpmn_xml_file_contents = SpecFileService.get_data( - process_model_with_diagram, name_of_file_with_diagram - ).decode("utf-8") - else: - try: - bpmn_xml_file_contents = GitService.get_instance_file_contents_for_revision( - process_model_with_diagram, - process_instance.bpmn_version_control_identifier, - file_name=name_of_file_with_diagram, - ) - except GitCommandError as ex: - process_instance.bpmn_xml_file_contents_retrieval_error = str(ex) + try: + bpmn_xml_file_contents = GitService.get_file_contents_for_revision_if_git_revision( + process_model=process_model_with_diagram, + revision=process_instance.bpmn_version_control_identifier, + file_name=name_of_file_with_diagram, + ) + except GitCommandError as ex: + process_instance.bpmn_xml_file_contents_retrieval_error = str(ex) process_instance.bpmn_xml_file_contents = bpmn_xml_file_contents process_instance_as_dict = process_instance.serialized_with_metadata() diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index 69681b1c..d7a56254 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py @@ -54,6 +54,8 @@ from spiffworkflow_backend.services.authorization_service import HumanTaskNotFou from spiffworkflow_backend.services.authorization_service import UserDoesNotHaveAccessToTaskError from spiffworkflow_backend.services.error_handling_service import ErrorHandlingService from spiffworkflow_backend.services.file_system_service import FileSystemService +from spiffworkflow_backend.services.git_service import GitCommandError +from spiffworkflow_backend.services.git_service import GitService from spiffworkflow_backend.services.jinja_service import JinjaService from spiffworkflow_backend.services.process_instance_processor import ProcessInstanceProcessor from spiffworkflow_backend.services.process_instance_queue_service import ProcessInstanceIsAlreadyLockedError @@ -452,9 +454,10 @@ def task_show( ) form_dict = _prepare_form_data( - form_schema_file_name, - task_model, - process_model_with_form, + form_file=form_schema_file_name, + task_model=task_model, + process_model=process_model_with_form, + revision=process_instance.bpmn_version_control_identifier, ) _update_form_schema_with_task_data_as_needed(form_dict, task_model.data) @@ -464,9 +467,10 @@ def task_show( if form_ui_schema_file_name: ui_form_contents = _prepare_form_data( - form_ui_schema_file_name, - task_model, - process_model_with_form, + form_file=form_ui_schema_file_name, + task_model=task_model, + process_model=process_model_with_form, + revision=process_instance.bpmn_version_control_identifier, ) if ui_form_contents: task_model.form_ui_schema = ui_form_contents @@ -940,31 +944,51 @@ def _get_tasks( return make_response(jsonify(response_json), 200) -def _prepare_form_data(form_file: str, task_model: TaskModel, process_model: ProcessModelInfo) -> dict: +def _prepare_form_data( + form_file: str, task_model: TaskModel, process_model: ProcessModelInfo, revision: str | None = None +) -> dict: if task_model.data is None: return {} - file_contents = SpecFileService.get_data(process_model, form_file).decode("utf-8") + try: + file_contents = GitService.get_file_contents_for_revision_if_git_revision( + process_model=process_model, + revision=revision, + file_name=form_file, + ) + except GitCommandError as exception: + raise ( + ApiError( + error_code="git_error_loading_form", + message=( + f"Could not load form schema from: {form_file}. Was git history rewritten such that revision" + f" '{revision}' no longer exists? Error was: {str(exception)}" + ), + status_code=400, + ) + ) from exception + try: form_contents = JinjaService.render_jinja_template(file_contents, task=task_model) - try: - # form_contents is a str - hot_dict: dict = json.loads(form_contents) - return hot_dict - except Exception as exception: - raise ( - ApiError( - error_code="error_loading_form", - message=f"Could not load form schema from: {form_file}. Error was: {str(exception)}", - status_code=400, - ) - ) from exception except TaskModelError as wfe: wfe.add_note(f"Error in Json Form File '{form_file}'") api_error = ApiError.from_workflow_exception("instructions_error", str(wfe), exp=wfe) api_error.file_name = form_file raise api_error + try: + # form_contents is a str + hot_dict: dict = json.loads(form_contents) + return hot_dict + except Exception as exception: + raise ( + ApiError( + error_code="error_loading_form", + message=f"Could not load form schema from: {form_file}. Error was: {str(exception)}", + status_code=400, + ) + ) from exception + def _get_spiff_task_from_process_instance( task_guid: str, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py index 92798025..83221d5e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py @@ -10,6 +10,7 @@ from spiffworkflow_backend.config import ConfigurationError from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.services.data_setup_service import DataSetupService from spiffworkflow_backend.services.file_system_service import FileSystemService +from spiffworkflow_backend.services.spec_file_service import SpecFileService class MissingGitConfigsError(Exception): @@ -47,19 +48,43 @@ class GitService: cls, process_model: ProcessModelInfo, revision: str, - file_name: str | None = None, + file_name: str, ) -> str: bpmn_spec_absolute_dir = current_app.config["SPIFFWORKFLOW_BACKEND_BPMN_SPEC_ABSOLUTE_DIR"] process_model_relative_path = FileSystemService.process_model_relative_path(process_model) - file_name_to_use = file_name - if file_name_to_use is None: - file_name_to_use = process_model.primary_file_name shell_command = [ "show", - f"{revision}:{process_model_relative_path}/{file_name_to_use}", + f"{revision}:{process_model_relative_path}/{file_name}", ] return cls.run_shell_command_to_get_stdout(shell_command, context_directory=bpmn_spec_absolute_dir) + @classmethod + def get_file_contents_for_revision_if_git_revision( + cls, + process_model: ProcessModelInfo, + file_name: str, + revision: str | None = None, + ) -> str: + try: + current_version_control_revision = cls.get_current_revision() + except GitCommandError: + current_version_control_revision = None + file_contents = None + if ( + revision is None + or revision == "" + or current_version_control_revision is None + or revision == current_version_control_revision + ): + file_contents = SpecFileService.get_data(process_model, file_name).decode("utf-8") + else: + file_contents = GitService.get_instance_file_contents_for_revision( + process_model, + revision, + file_name=file_name, + ) + return file_contents + @classmethod def commit( cls, @@ -92,7 +117,7 @@ class GitService: if raise_on_missing: raise MissingGitConfigsError( "Missing config for SPIFFWORKFLOW_BACKEND_GIT_SOURCE_BRANCH. " - "This is required for publishing process models" + "This is required for committing and publishing process models" ) return False return True diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py index cf68ad78..f52d9036 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py @@ -94,7 +94,7 @@ class ProcessInstanceService: try: current_git_revision = GitService.get_current_revision() except GitCommandError: - current_git_revision = "" + current_git_revision = None process_instance_model = ProcessInstanceModel( status=ProcessInstanceStatus.not_started.value, process_initiator_id=user.id,