From 35e7de6e506e30a71466cf8b03f5dd661d57456c Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 6 Mar 2023 17:07:04 -0500 Subject: [PATCH 1/6] Just some minor stuff: 1) Don't overwrite the message of all Workflow Task Exceptions to say "Something went wrong" 2) log errors when correlation evaluations fail (just so we are aware something isn't working correctly) 3) Dont make all evaluation fuctions require a current task context to execute -- that should be optional. --- .../exceptions/api_error.py | 2 +- .../models/message_instance.py | 8 +++++++- .../services/process_instance_processor.py | 18 +++++++----------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py index 39ab1335..ca8c5125 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py @@ -149,7 +149,7 @@ class ApiError(Exception): # Note that WorkflowDataExceptions are also WorkflowTaskExceptions return ApiError.from_task( error_code, - message, + message + ". " + str(exp), exp.task, line_number=exp.line_number, offset=exp.offset, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py index 99168ec3..c97c4860 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py @@ -6,6 +6,7 @@ from typing import Optional from typing import TYPE_CHECKING from SpiffWorkflow.bpmn.PythonScriptEngine import PythonScriptEngine # type: ignore +from flask import current_app from sqlalchemy import ForeignKey from sqlalchemy.event import listens_for from sqlalchemy.orm import relationship @@ -135,10 +136,15 @@ class MessageInstanceModel(SpiffworkflowBaseDBModel): result = expression_engine._evaluate( correlation_key.retrieval_expression, payload ) - except Exception: + except Exception as e: # the failure of a payload evaluation may not mean that matches for these # message instances can't happen with other messages. So don't error up. # fixme: Perhaps log some sort of error. + current_app.logger.warning( + "Error evaluating correlation key when comparing send and receive messages." + + f"Expression {correlation_key.retrieval_expression} failed with the error " + + str(e) + ) return False if result != expected_value: return False diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py index 8cd663af..6e1e43eb 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -318,19 +318,15 @@ class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore def __get_augment_methods(self, task: Optional[SpiffTask]) -> Dict[str, Callable]: """__get_augment_methods.""" - tld = current_app.config["THREAD_LOCAL_DATA"] - + tld = current_app.config.get("THREAD_LOCAL_DATA") if not hasattr(tld, "process_model_identifier"): - raise MissingProcessInfoError( - "Could not find process_model_identifier from app config" - ) + process_model_identifier = None + else: + process_model_identifier = tld.process_model_identifier if not hasattr(tld, "process_instance_id"): - raise MissingProcessInfoError( - "Could not find process_instance_id from app config" - ) - - process_model_identifier = tld.process_model_identifier - process_instance_id = tld.process_instance_id + process_instance_id = None + else: + process_instance_id = tld.process_instance_id script_attributes_context = ScriptAttributesContext( task=task, environment_identifier=current_app.config["ENV_IDENTIFIER"], From 4fbb2b90fa550aa047c30ac809c3f32e9825aed0 Mon Sep 17 00:00:00 2001 From: burnettk Date: Mon, 6 Mar 2023 17:13:48 -0500 Subject: [PATCH 2/6] cypress pilot command --- spiffworkflow-frontend/bin/cypress_pilot | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100755 spiffworkflow-frontend/bin/cypress_pilot diff --git a/spiffworkflow-frontend/bin/cypress_pilot b/spiffworkflow-frontend/bin/cypress_pilot new file mode 100755 index 00000000..213282d2 --- /dev/null +++ b/spiffworkflow-frontend/bin/cypress_pilot @@ -0,0 +1,24 @@ +#!/usr/bin/env bash + +function error_handler() { + >&2 echo "Exited with BAD EXIT CODE '${2}' in ${0} script at line: ${1}." + exit "$2" +} +trap 'error_handler ${LINENO} $?' ERR +set -o errtrace -o errexit -o nounset -o pipefail + +# see also: npx cypress run --env grep="can filter",grepFilterSpecs=true +# https://github.com/cypress-io/cypress/tree/develop/npm/grep#pre-filter-specs-grepfilterspecs + +command="${1:-}" +if [[ -z "$command" ]]; then + command=open +else + shift +fi + +if [[ -z "CYPRESS_SPIFFWORKFLOW_FRONTEND_AUTH_WITH_KEYCLOAK" ]]; then + export CYPRESS_SPIFFWORKFLOW_FRONTEND_AUTH_WITH_KEYCLOAK=true +fi + +./node_modules/.bin/cypress "$command" -c specPattern="cypress/pilot/**/*.cy.{js,jsx,ts,tsx}" --e2e --browser chrome "$@" From 51f7b604082f089ea60816288e847c5943c0d8a3 Mon Sep 17 00:00:00 2001 From: burnettk Date: Mon, 6 Mar 2023 22:04:03 -0500 Subject: [PATCH 3/6] force light mode to avoid making the site look broken --- spiffworkflow-frontend/src/routes/TaskShow.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spiffworkflow-frontend/src/routes/TaskShow.tsx b/spiffworkflow-frontend/src/routes/TaskShow.tsx index 56813225..53ac8c0c 100644 --- a/spiffworkflow-frontend/src/routes/TaskShow.tsx +++ b/spiffworkflow-frontend/src/routes/TaskShow.tsx @@ -269,7 +269,13 @@ export default function TaskShow() { } return (
- + {/* + https://www.npmjs.com/package/@uiw/react-md-editor switches to dark mode by default by respecting @media (prefers-color-scheme: dark) + This makes it look like our site is broken, so until the rest of the site supports dark mode, turn off dark mode for this component. + */} +
+ +
); }; From d7f5e561d1d4f94694039900743d8259eb19964e Mon Sep 17 00:00:00 2001 From: burnettk Date: Mon, 6 Mar 2023 23:23:10 -0500 Subject: [PATCH 4/6] lint and show version control identifier on process model show --- .../spiffworkflow_backend/models/message_instance.py | 10 ++++++---- .../src/spiffworkflow_backend/models/process_model.py | 5 ++++- .../routes/process_models_controller.py | 7 +++++++ spiffworkflow-frontend/src/interfaces.ts | 1 + spiffworkflow-frontend/src/routes/ProcessModelShow.tsx | 3 +++ 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py index c97c4860..5985e3ae 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/message_instance.py @@ -5,8 +5,8 @@ from typing import Any from typing import Optional from typing import TYPE_CHECKING -from SpiffWorkflow.bpmn.PythonScriptEngine import PythonScriptEngine # type: ignore from flask import current_app +from SpiffWorkflow.bpmn.PythonScriptEngine import PythonScriptEngine # type: ignore from sqlalchemy import ForeignKey from sqlalchemy.event import listens_for from sqlalchemy.orm import relationship @@ -141,9 +141,11 @@ class MessageInstanceModel(SpiffworkflowBaseDBModel): # message instances can't happen with other messages. So don't error up. # fixme: Perhaps log some sort of error. current_app.logger.warning( - "Error evaluating correlation key when comparing send and receive messages." + - f"Expression {correlation_key.retrieval_expression} failed with the error " + - str(e) + "Error evaluating correlation key when comparing send and receive" + " messages." + + f"Expression {correlation_key.retrieval_expression} failed with" + " the error " + + str(e) ) return False if result != expected_value: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py index d39f0d74..8ae6595c 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_model.py @@ -38,9 +38,12 @@ class ProcessModelInfo: files: list[File] | None = field(default_factory=list[File]) fault_or_suspend_on_exception: str = NotificationType.fault.value exception_notification_addresses: list[str] = field(default_factory=list) - parent_groups: list[ProcessGroupLite] | None = None metadata_extraction_paths: list[dict[str, str]] | None = None + # just for the API + parent_groups: list[ProcessGroupLite] | None = None + bpmn_version_control_identifier: str | None = None + def __post_init__(self) -> None: """__post_init__.""" self.sort_index = self.id 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 1a628c4c..845349ff 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py @@ -30,6 +30,7 @@ from spiffworkflow_backend.routes.process_api_blueprint import _get_process_mode from spiffworkflow_backend.routes.process_api_blueprint import ( _un_modify_modified_process_model_id, ) +from spiffworkflow_backend.services.git_service import GitCommandError from spiffworkflow_backend.services.git_service import GitService from spiffworkflow_backend.services.git_service import MissingGitConfigsError from spiffworkflow_backend.services.process_instance_report_service import ( @@ -200,6 +201,12 @@ def process_model_show( process_model.parent_groups = ProcessModelService.get_parent_group_array( process_model.id ) + try: + current_git_revision = GitService.get_current_revision() + except GitCommandError: + current_git_revision = "" + + process_model.bpmn_version_control_identifier = current_git_revision return make_response(jsonify(process_model), 200) diff --git a/spiffworkflow-frontend/src/interfaces.ts b/spiffworkflow-frontend/src/interfaces.ts index 4c28376c..a8d73690 100644 --- a/spiffworkflow-frontend/src/interfaces.ts +++ b/spiffworkflow-frontend/src/interfaces.ts @@ -171,6 +171,7 @@ export interface ProcessModel { metadata_extraction_paths?: MetadataExtractionPath[]; fault_or_suspend_on_exception?: string; exception_notification_addresses?: string[]; + bpmn_version_control_identifier?: string; } export interface ProcessGroup { diff --git a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx index c7f83596..8e532b5a 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx @@ -498,6 +498,9 @@ export default function ProcessModelShow() { From 2bb3e09c0d573b73f33b220499caa1840e153df3 Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 7 Mar 2023 09:05:06 -0500 Subject: [PATCH 5/6] fixed mypy issues --- .../models/script_attributes_context.py | 4 ++-- .../scripts/markdown_file_download_link.py | 7 +++++++ .../scripts/save_process_instance_metadata.py | 7 +++++++ .../src/spiffworkflow_backend/scripts/script.py | 8 ++++++++ .../services/process_instance_processor.py | 15 +++++++-------- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/script_attributes_context.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/script_attributes_context.py index 9405c8b3..8be7f44e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/script_attributes_context.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/script_attributes_context.py @@ -11,5 +11,5 @@ class ScriptAttributesContext: task: Optional[SpiffTask] environment_identifier: str - process_instance_id: int - process_model_identifier: str + process_instance_id: Optional[int] + process_model_identifier: Optional[str] diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py index cb03ae24..de1c16c8 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py @@ -7,6 +7,7 @@ from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.models.script_attributes_context import ( ScriptAttributesContext, ) +from spiffworkflow_backend.scripts.script import ProcessModelIdentifierMissingError from spiffworkflow_backend.scripts.script import Script @@ -36,6 +37,12 @@ class GetMarkdownFileDownloadLink(Script): digest = parts[2].split(",")[1][-64:] label = parts[1].split("=")[1] process_model_identifier = script_attributes_context.process_model_identifier + if process_model_identifier is None: + raise ProcessModelIdentifierMissingError( + "The process model identifier was not given to script" + " 'markdown_file_download_link'. This script needs to be run from" + " within the context of a process model." + ) modified_process_model_identifier = ( ProcessModelInfo.modify_process_identifier_for_path_param( process_model_identifier diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py index 3d2054df..52bd7b38 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py @@ -8,6 +8,7 @@ from spiffworkflow_backend.models.process_instance_metadata import ( from spiffworkflow_backend.models.script_attributes_context import ( ScriptAttributesContext, ) +from spiffworkflow_backend.scripts.script import ProcessInstanceIdMissingError from spiffworkflow_backend.scripts.script import Script @@ -26,6 +27,12 @@ class SaveProcessInstanceMetadata(Script): ) -> Any: """Run.""" metadata_dict = args[0] + if script_attributes_context.process_instance_id is None: + raise ProcessInstanceIdMissingError( + "The process instance id was not given to script" + " 'save_process_instance_metadata'. This script needs to be run from" + " within the context of a process instance." + ) for key, value in metadata_dict.items(): pim = ProcessInstanceMetadataModel.query.filter_by( process_instance_id=script_attributes_context.process_instance_id, diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py index 897bc1be..337c67ee 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py @@ -26,6 +26,14 @@ class ScriptUnauthorizedForUserError(Exception): """ScriptUnauthorizedForUserError.""" +class ProcessInstanceIdMissingError(Exception): + pass + + +class ProcessModelIdentifierMissingError(Exception): + pass + + class Script: """Provides an abstract class that defines how scripts should work, this must be extended in all Script Tasks.""" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py index 6e1e43eb..923c3e7e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -319,14 +319,13 @@ class CustomBpmnScriptEngine(PythonScriptEngine): # type: ignore def __get_augment_methods(self, task: Optional[SpiffTask]) -> Dict[str, Callable]: """__get_augment_methods.""" tld = current_app.config.get("THREAD_LOCAL_DATA") - if not hasattr(tld, "process_model_identifier"): - process_model_identifier = None - else: - process_model_identifier = tld.process_model_identifier - if not hasattr(tld, "process_instance_id"): - process_instance_id = None - else: - process_instance_id = tld.process_instance_id + process_model_identifier = None + process_instance_id = None + if tld: + if hasattr(tld, "process_model_identifier"): + process_model_identifier = tld.process_model_identifier + if hasattr(tld, "process_instance_id"): + process_instance_id = tld.process_instance_id script_attributes_context = ScriptAttributesContext( task=task, environment_identifier=current_app.config["ENV_IDENTIFIER"], From fd5969d8ad531d4e082308f10e5c7cc0b9f5b223 Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 7 Mar 2023 09:17:32 -0500 Subject: [PATCH 6/6] move common script errors to script so it can keep the error messaging consistent across scripts --- .../scripts/markdown_file_download_link.py | 11 +++++----- .../scripts/save_process_instance_metadata.py | 7 ++----- .../spiffworkflow_backend/scripts/script.py | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py index de1c16c8..1a60c407 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/markdown_file_download_link.py @@ -7,7 +7,6 @@ from spiffworkflow_backend.models.process_model import ProcessModelInfo from spiffworkflow_backend.models.script_attributes_context import ( ScriptAttributesContext, ) -from spiffworkflow_backend.scripts.script import ProcessModelIdentifierMissingError from spiffworkflow_backend.scripts.script import Script @@ -38,10 +37,8 @@ class GetMarkdownFileDownloadLink(Script): label = parts[1].split("=")[1] process_model_identifier = script_attributes_context.process_model_identifier if process_model_identifier is None: - raise ProcessModelIdentifierMissingError( - "The process model identifier was not given to script" - " 'markdown_file_download_link'. This script needs to be run from" - " within the context of a process model." + raise self.get_proces_model_identifier_is_missing_error( + "markdown_file_download_link" ) modified_process_model_identifier = ( ProcessModelInfo.modify_process_identifier_for_path_param( @@ -49,6 +46,10 @@ class GetMarkdownFileDownloadLink(Script): ) ) process_instance_id = script_attributes_context.process_instance_id + if process_instance_id is None: + raise self.get_proces_instance_id_is_missing_error( + "save_process_instance_metadata" + ) url = current_app.config["SPIFFWORKFLOW_BACKEND_URL"] url += ( f"/v1.0/process-data-file-download/{modified_process_model_identifier}/" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py index 52bd7b38..19cb34fd 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/save_process_instance_metadata.py @@ -8,7 +8,6 @@ from spiffworkflow_backend.models.process_instance_metadata import ( from spiffworkflow_backend.models.script_attributes_context import ( ScriptAttributesContext, ) -from spiffworkflow_backend.scripts.script import ProcessInstanceIdMissingError from spiffworkflow_backend.scripts.script import Script @@ -28,10 +27,8 @@ class SaveProcessInstanceMetadata(Script): """Run.""" metadata_dict = args[0] if script_attributes_context.process_instance_id is None: - raise ProcessInstanceIdMissingError( - "The process instance id was not given to script" - " 'save_process_instance_metadata'. This script needs to be run from" - " within the context of a process instance." + raise self.get_proces_instance_id_is_missing_error( + "save_process_instance_metadata" ) for key, value in metadata_dict.items(): pim = ProcessInstanceMetadataModel.query.filter_by( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py index 337c67ee..12b7fdc7 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/script.py @@ -57,6 +57,26 @@ class Script: + "does not properly implement the run function.", ) + def get_proces_instance_id_is_missing_error( + self, script_name: str + ) -> ProcessInstanceIdMissingError: + """Return the error so we can raise it from the script and mypy will be happy.""" + raise ProcessInstanceIdMissingError( + "The process instance id was not given to script" + f" '{script_name}'. This script needs to be run from" + " within the context of a process instance." + ) + + def get_proces_model_identifier_is_missing_error( + self, script_name: str + ) -> ProcessModelIdentifierMissingError: + """Return the error so we can raise it from the script and mypy will be happy.""" + return ProcessModelIdentifierMissingError( + "The process model identifier was not given to script" + f" '{script_name}'. This script needs to be run from" + " within the context of a process model." + ) + @staticmethod def requires_privileged_permissions() -> bool: """It seems safer to default to True and make safe functions opt in for any user to run them.