Drop backwards compatibility of scripts. While this will cause some initial pain, it's less confusing and error prone, and we are still in the development phase of the project. Were this going straight to production we would likely want to keep this backwards compatibility.

Don't parse on spaces if this is python code, so we avoid any errors in processing - spaces should be valid.

crc/services/workflow_processor.py

@@ -33,27 +33,27 @@ class CustomBpmnScriptEngine(BpmnScriptEngine):
 
     def execute(self, task: SpiffTask, script, data):
         """
-        Assume that the script read in from the BPMN file is a fully qualified python class. Instantiate
-        that class, pass in any data available to the current task so that it might act on it.
-        Assume that the class implements the "do_task" method.
-
-        This allows us to reference custom code from the BPMN diagram.
+        Functions in two modes.
+        1. If the command is proceeded by #! then this is assumed to be a python script, and will
+           attempt to load that python module and execute the do_task method on that script.  Scripts
+           must be located in the scripts package and they must extend the script.py class.
+        2. If not proceeded by the #! this will attempt to execute the script directly and assumes it is
+           valid Python.
         """
         # Shlex splits the whole string while respecting double quoted strings within
-        commands = shlex.split(script)
-        failedOnce = False
-        prevError = ''
-        if commands[0] != '#!':
+        if not script.startswith('#!'):
             try:
-                super().execute(task,script,data)
+                super().execute(task, script, data)
             except SyntaxError as e:
-                failedOnce = True
-                prevError = script
-                app.logger.warning('We experienced a syntax error, but we are going to try the old method on '
-                                   '"%s"'%script)
+                raise ApiError.from_task('syntax_error',
+                                         f'If you are running a pre-defined script, please'
+                                         f' proceed the script with "#!", otherwise this is assumed to be'
+                                         f' pure python: {script}, {e.msg}', task=task)
         else:
-            commands = commands[1:]
+            self.run_predefined_script(task, script[2:], data)  # strip off the first two characters.
 
+    def run_predefined_script(self, task: SpiffTask, script, data):
+        commands = shlex.split(script)
         path_and_command = commands[0].rsplit(".", 1)
         if len(path_and_command) == 1:
             module_name = "crc.scripts." + self.camel_to_snake(path_and_command[0])
@@ -83,10 +83,6 @@ class CustomBpmnScriptEngine(BpmnScriptEngine):
             else:
                 klass().do_task(task, study_id, workflow_id, *commands[1:])
         except ModuleNotFoundError:
-            if failedOnce:
-                raise ApiError.from_task("invalid_script",
-                                         "Script had a syntax error: '%s'" % (prevError),
-                                         task=task)
             raise ApiError.from_task("invalid_script",
                  "Unable to locate Script: '%s:%s'" % (module_name, class_name),
                  task=task)

crc/static/bpmn/research_rampup/research_rampup.bpmn

@@ -755,7 +755,7 @@ Notify the Area Monitor for
 This step is internal to the system and do not require and user interaction</bpmn:documentation>
       <bpmn:incoming>Flow_0j4rs82</bpmn:incoming>
       <bpmn:outgoing>Flow_07ge8uf</bpmn:outgoing>
-      <bpmn:script>RequestApproval ApprvlApprvr1 ApprvlApprvr2</bpmn:script>
+      <bpmn:script>#!RequestApproval ApprvlApprvr1 ApprvlApprvr2</bpmn:script>
     </bpmn:scriptTask>
     <bpmn:scriptTask id="Activity_1u58hox" name="Update Request">
       <bpmn:documentation>#### Script Task

tests/workflow/test_workflow_spec_validation_api.py

@@ -66,7 +66,6 @@ class TestWorkflowSpecValidation(BaseTest):
             errors.extend(ApiErrorSchema(many=True).load(json_data))
         self.assertEqual(0, len(errors), json.dumps(errors))
 
-
     def test_invalid_expression(self):
         self.load_example_data()
         errors = self.validate_workflow("invalid_expression")
@@ -103,12 +102,10 @@ class TestWorkflowSpecValidation(BaseTest):
         errors = self.validate_workflow("invalid_script2")
         self.assertEqual(2, len(errors))
         self.assertEqual("error_loading_workflow", errors[0]['code'])
-        self.assertTrue("syntax error" in errors[0]['message'])
         self.assertEqual("Invalid_Script_Task", errors[0]['task_id'])
         self.assertEqual("An Invalid Script Reference", errors[0]['task_name'])
         self.assertEqual("invalid_script2.bpmn", errors[0]['file_name'])
 
-
     def test_repeating_sections_correctly_populated(self):
         self.load_example_data()
         spec_model = self.load_test_spec('repeat_form')