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 <jasquat@users.noreply.github.com>

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))
+    updated_at_in_seconds: int = db.Column(db.Integer)
-    bpmn_version_control_identifier: str = db.Column(db.String(255))
+    created_at_in_seconds: int = db.Column(db.Integer)
-    last_milestone_bpmn_name: str = db.Column(db.String(255))
+
+    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

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:
+        try:
-            bpmn_xml_file_contents = SpecFileService.get_data(
+            bpmn_xml_file_contents = GitService.get_file_contents_for_revision_if_git_revision(
-                process_model_with_diagram, name_of_file_with_diagram
+                process_model=process_model_with_diagram,
-            ).decode("utf-8")
+                revision=process_instance.bpmn_version_control_identifier,
-        else:
+                file_name=name_of_file_with_diagram,
-            try:
+            )
-                bpmn_xml_file_contents = GitService.get_instance_file_contents_for_revision(
+        except GitCommandError as ex:
-                    process_model_with_diagram,
+            process_instance.bpmn_xml_file_contents_retrieval_error = str(ex)
-                    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()

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,
+                form_file=form_schema_file_name,
-                task_model,
+                task_model=task_model,
-                process_model_with_form,
+                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,
+                    form_file=form_ui_schema_file_name,
-                    task_model,
+                    task_model=task_model,
-                    process_model_with_form,
+                    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,

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

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,