From ce8d7cad16c006082a29b1095279f863dc48d22e Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Thu, 23 Jul 2020 14:56:12 -0400 Subject: [PATCH 1/7] We resolved a problem with a test with some changes to Spiff, change the correct so that it is correct. --- tests/test_user_roles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_user_roles.py b/tests/test_user_roles.py index 6104641c..212bd463 100644 --- a/tests/test_user_roles.py +++ b/tests/test_user_roles.py @@ -178,7 +178,7 @@ class TestTasksApi(BaseTest): workflow_api = self.complete_form(workflow, workflow_api.next_task, data, user_uid=submitter.uid) nav = workflow_api.navigation self.assertEquals(5, len(nav)) - self.assertEquals('COMPLETED', nav[0]['state']) # We still have some issues here, the navigation will be off when looping back. + self.assertEquals('READY', nav[0]['state']) # Issue resolved - KPM - was COMPLETED self.assertEquals('LOCKED', nav[1]['state']) # Second item is locked, it is the review and doesn't belong to this user. self.assertEquals('LOCKED', nav[2]['state']) # third item is a gateway belonging to the supervisor, and is locked. self.assertEquals('READY', workflow_api.next_task.state) From f5ab2835380edd840627f7db9c2fb357ea53aad6 Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Fri, 24 Jul 2020 12:08:46 -0400 Subject: [PATCH 2/7] Test of adding in the ability of augmenting the workflow to include internal scripts like StudyInfo This is the first waypoint on a larger effort to make all of the 'special scripts' that currently require a shebang to be just another python function. --- crc/scripts/study_info.py | 40 ++++++++++++++++++--- crc/services/workflow_processor.py | 16 ++++++++- tests/data/study_details/study_details.bpmn | 21 ++++++++--- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/crc/scripts/study_info.py b/crc/scripts/study_info.py index f274b899..a559a3fc 100644 --- a/crc/scripts/study_info.py +++ b/crc/scripts/study_info.py @@ -8,7 +8,7 @@ from crc.scripts.script import Script from crc.services.file_service import FileService from crc.services.protocol_builder import ProtocolBuilderService from crc.services.study_service import StudyService - +from box import Box class StudyInfo(Script): """Please see the detailed description that is provided below. """ @@ -199,9 +199,41 @@ Returns information specific to the protocol. self.add_data_to_task(task=task, data=data["study"]) self.add_data_to_task(task, {"documents": StudyService().get_documents_status(study_id)}) + def return_data(self, task, study_id, workflow_id, *args, **kwargs): + self.check_args(args,2) + prefix = None + if len(args) > 1: + prefix = args[1] + cmd = args[0] + study_info = {} + if self.__class__.__name__ in task.data: + study_info = task.data[self.__class__.__name__] + retval = None + if cmd == 'info': + study = session.query(StudyModel).filter_by(id=study_id).first() + schema = StudySchema() + retval = schema.dump(study) + if cmd == 'investigators': + retval = StudyService().get_investigators(study_id) + if cmd == 'roles': + retval = StudyService().get_investigators(study_id, all=True) + if cmd == 'details': + retval = self.pb.get_study_details(study_id) + if cmd == 'approvals': + retval = StudyService().get_approvals(study_id) + if cmd == 'documents': + retval = StudyService().get_documents_status(study_id) + if cmd == 'protocol': + retval = StudyService().get_protocol(study_id) + if isinstance(retval,dict) and prefix is not None: + return Box({x:retval[x] for x in retval.keys() if x[:len(prefix)] == prefix}) + elif isinstance(retval,dict): + return Box(retval) + else: + return retval + def do_task(self, task, study_id, workflow_id, *args, **kwargs): self.check_args(args) - cmd = args[0] study_info = {} if self.__class__.__name__ in task.data: @@ -225,8 +257,8 @@ Returns information specific to the protocol. self.add_data_to_task(task, {cmd: StudyService().get_protocol(study_id)}) - def check_args(self, args): - if len(args) != 1 or (args[0] not in StudyInfo.type_options): + def check_args(self, args, maxlen=1): + if len(args) < 1 or len(args) > maxlen or (args[0] not in StudyInfo.type_options): raise ApiError(code="missing_argument", message="The StudyInfo script requires a single argument which must be " "one of %s" % ",".join(StudyInfo.type_options)) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 165d3313..cb39be44 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -17,6 +17,7 @@ from SpiffWorkflow.dmn.parser.BpmnDmnParser import BpmnDmnParser from SpiffWorkflow.exceptions import WorkflowTaskExecException from SpiffWorkflow.specs import WorkflowSpec +import crc from crc import session, app from crc.api.common import ApiError from crc.models.file import FileDataModel, FileModel, FileType @@ -41,9 +42,22 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): valid Python. """ # Shlex splits the whole string while respecting double quoted strings within + study_id = task.workflow.data[WorkflowProcessor.STUDY_ID_KEY] + if WorkflowProcessor.WORKFLOW_ID_KEY in task.workflow.data: + workflow_id = task.workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] + else: + workflow_id = None + if not script.startswith('#!'): try: - super().execute(task, script, data) + augmentMethods = {'studyInfo':lambda *args: crc.scripts.study_info.StudyInfo.return_data( + crc.scripts.study_info.StudyInfo(), + task, + study_id, + workflow_id, + *args)} + + super().execute(task, script, data,externalMethods=augmentMethods) except SyntaxError as e: raise ApiError.from_task('syntax_error', f'If you are running a pre-defined script, please' diff --git a/tests/data/study_details/study_details.bpmn b/tests/data/study_details/study_details.bpmn index 2b46f935..888a28d9 100644 --- a/tests/data/study_details/study_details.bpmn +++ b/tests/data/study_details/study_details.bpmn @@ -10,10 +10,16 @@ SequenceFlow_1bqiin0 #! StudyInfo info - + - SequenceFlow_1bqiin0 + Flow_0ochvmi + + + SequenceFlow_1bqiin0 + Flow_0ochvmi + study = studyInfo('info','p') + @@ -29,10 +35,17 @@ - + - + + + + + + + + From a124e13c6ae2dc28510ae386334c85924b1449eb Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Fri, 24 Jul 2020 14:33:24 -0400 Subject: [PATCH 3/7] Replace all legacy style calls with new calls. Still having issues where we try to eval an empty definition, not quite sure why there is a difference from what we had before. I may need to revert some of it and determine what is going on. --- crc/scripts/fact_service.py | 2 +- crc/scripts/script.py | 33 ++++-- crc/scripts/study_info.py | 25 +---- crc/services/workflow_processor.py | 102 +++++++++--------- crc/static/bpmn/core_info/core_info.bpmn | 3 +- .../data_security_plan.bpmn | 2 +- .../documents_approvals.bpmn | 6 +- .../bpmn/ide_supplement/ide_supplement.bpmn | 3 +- .../ids_full_submission.bpmn | 3 +- crc/static/bpmn/ind_update/ind_update.bpmn | 3 +- .../bpmn/irb_api_details/irb_api_details.bpmn | 3 +- .../irb_api_personnel/irb_api_personnel.bpmn | 3 +- .../bpmn/research_rampup/research_rampup.bpmn | 4 +- .../top_level_workflow.bpmn | 9 +- tests/data/docx/docx.bpmn | 2 +- tests/data/email/email.bpmn | 2 +- tests/data/invalid_script/invalid_script.bpmn | 2 +- tests/data/multi_instance/multi_instance.bpmn | 3 +- .../multi_instance_parallel.bpmn | 3 +- tests/data/random_fact/random_fact.bpmn | 2 +- tests/data/study_details/study_details.bpmn | 5 +- .../top_level_workflow.bpmn | 3 +- 22 files changed, 115 insertions(+), 108 deletions(-) diff --git a/crc/scripts/fact_service.py b/crc/scripts/fact_service.py index b3701312..19e5cb3f 100644 --- a/crc/scripts/fact_service.py +++ b/crc/scripts/fact_service.py @@ -40,7 +40,7 @@ class FactService(Script): else: details = "unknown fact type." - self.add_data_to_task(task, details) + #self.add_data_to_task(task, details) print(details) return details diff --git a/crc/scripts/script.py b/crc/scripts/script.py index ac4ce38d..ba5af5b7 100644 --- a/crc/scripts/script.py +++ b/crc/scripts/script.py @@ -23,6 +23,27 @@ class Script(object): "This is an internal error. The script you are trying to execute '%s' " % self.__class__.__name__ + "does must provide a validate_only option that mimics the do_task, " + "but does not make external calls or database updates." ) + @staticmethod + def generate_augmented_list(task, study_id,workflow_id): + """ + this makes a dictionary of lambda functions that are closed over the class instance that + They represent. This is passed into PythonScriptParser as a list of helper functions that are + available for running. In general, they maintain the do_task call structure that they had, but + they always return a value rather than updating the task data. + + We may be able to remove the task for each of these calls if we are not using it other than potentially + updating the task data. + """ + def make_closure(subclass,task,study_id,workflow_id): + instance = subclass() + return lambda *a : subclass.do_task(instance,task,study_id,workflow_id,*a) + execlist = {} + subclasses = Script.get_all_subclasses() + for x in range(len(subclasses)): + subclass = subclasses[x] + execlist[subclass.__module__.split('.')[-1]] = make_closure(subclass,task,study_id, + workflow_id) + return execlist @staticmethod def get_all_subclasses(): @@ -46,12 +67,12 @@ class Script(object): return all_subclasses - def add_data_to_task(self, task, data): - key = self.__class__.__name__ - if key in task.data: - task.data[key].update(data) - else: - task.data[key] = data + # def add_data_to_task(self, task, data): + # key = self.__class__.__name__ + # if key in task.data: + # task.data[key].update(data) + # else: + # task.data[key] = data class ScriptValidationError: diff --git a/crc/scripts/study_info.py b/crc/scripts/study_info.py index a559a3fc..c392d40c 100644 --- a/crc/scripts/study_info.py +++ b/crc/scripts/study_info.py @@ -199,7 +199,7 @@ Returns information specific to the protocol. self.add_data_to_task(task=task, data=data["study"]) self.add_data_to_task(task, {"documents": StudyService().get_documents_status(study_id)}) - def return_data(self, task, study_id, workflow_id, *args, **kwargs): + def do_task(self, task, study_id, workflow_id, *args, **kwargs): self.check_args(args,2) prefix = None if len(args) > 1: @@ -232,29 +232,6 @@ Returns information specific to the protocol. else: return retval - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - self.check_args(args) - cmd = args[0] - study_info = {} - if self.__class__.__name__ in task.data: - study_info = task.data[self.__class__.__name__] - - if cmd == 'info': - study = session.query(StudyModel).filter_by(id=study_id).first() - schema = StudySchema() - self.add_data_to_task(task, {cmd: schema.dump(study)}) - if cmd == 'investigators': - self.add_data_to_task(task, {cmd: StudyService().get_investigators(study_id)}) - if cmd == 'roles': - self.add_data_to_task(task, {cmd: StudyService().get_investigators(study_id, all=True)}) - if cmd == 'details': - self.add_data_to_task(task, {cmd: self.pb.get_study_details(study_id)}) - if cmd == 'approvals': - self.add_data_to_task(task, {cmd: StudyService().get_approvals(study_id)}) - if cmd == 'documents': - self.add_data_to_task(task, {cmd: StudyService().get_documents_status(study_id)}) - if cmd == 'protocol': - self.add_data_to_task(task, {cmd: StudyService().get_protocol(study_id)}) def check_args(self, args, maxlen=1): diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index cb39be44..d29c3a0e 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -48,58 +48,52 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): else: workflow_id = None - if not script.startswith('#!'): - try: - augmentMethods = {'studyInfo':lambda *args: crc.scripts.study_info.StudyInfo.return_data( - crc.scripts.study_info.StudyInfo(), - task, - study_id, - workflow_id, - *args)} - - super().execute(task, script, data,externalMethods=augmentMethods) - except SyntaxError as e: - 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: - 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]) - class_name = path_and_command[0] - else: - module_name = "crc.scripts." + path_and_command[0] + "." + self.camel_to_snake(path_and_command[1]) - class_name = path_and_command[1] try: - mod = __import__(module_name, fromlist=[class_name]) - klass = getattr(mod, class_name) - study_id = task.workflow.data[WorkflowProcessor.STUDY_ID_KEY] - if WorkflowProcessor.WORKFLOW_ID_KEY in task.workflow.data: - workflow_id = task.workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] - else: - workflow_id = None + augmentMethods = Script.generate_augmented_list(task,study_id,workflow_id) - if not isinstance(klass(), Script): - raise ApiError.from_task("invalid_script", - "This is an internal error. The script '%s:%s' you called " % - (module_name, class_name) + - "does not properly implement the CRC Script class.", - task=task) - if task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: - """If this is running a validation, and not a normal process, then we want to - mimic running the script, but not make any external calls or database changes.""" - klass().do_task_validate_only(task, study_id, workflow_id, *commands[1:]) - else: - klass().do_task(task, study_id, workflow_id, *commands[1:]) - except ModuleNotFoundError: - raise ApiError.from_task("invalid_script", - "Unable to locate Script: '%s:%s'" % (module_name, class_name), - task=task) + super().execute(task, script, data, externalMethods=augmentMethods) + except SyntaxError as e: + 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: + # 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]) + # class_name = path_and_command[0] + # else: + # module_name = "crc.scripts." + path_and_command[0] + "." + self.camel_to_snake(path_and_command[1]) + # class_name = path_and_command[1] + # try: + # mod = __import__(module_name, fromlist=[class_name]) + # klass = getattr(mod, class_name) + # study_id = task.workflow.data[WorkflowProcessor.STUDY_ID_KEY] + # if WorkflowProcessor.WORKFLOW_ID_KEY in task.workflow.data: + # workflow_id = task.workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] + # else: + # workflow_id = None + # + # if not isinstance(klass(), Script): + # raise ApiError.from_task("invalid_script", + # "This is an internal error. The script '%s:%s' you called " % + # (module_name, class_name) + + # "does not properly implement the CRC Script class.", + # task=task) + # if task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: + # """If this is running a validation, and not a normal process, then we want to + # mimic running the script, but not make any external calls or database changes.""" + # klass().do_task_validate_only(task, study_id, workflow_id, *commands[1:]) + # else: + # klass().do_task(task, study_id, workflow_id, *commands[1:]) + # except ModuleNotFoundError: + # raise ApiError.from_task("invalid_script", + # "Unable to locate Script: '%s:%s'" % (module_name, class_name), + # task=task) def evaluate_expression(self, task, expression): """ @@ -194,10 +188,10 @@ class WorkflowProcessor(object): bpmn_workflow = BpmnWorkflow(spec, script_engine=self._script_engine) bpmn_workflow.data[WorkflowProcessor.STUDY_ID_KEY] = workflow_model.study_id bpmn_workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY] = validate_only - try: - bpmn_workflow.do_engine_steps() - except WorkflowException as we: - raise ApiError.from_task_spec("error_loading_workflow", str(we), we.sender) + #try: + bpmn_workflow.do_engine_steps() + # except WorkflowException as we: + # raise ApiError.from_task_spec("error_loading_workflow", str(we), we.sender) return bpmn_workflow def save(self): diff --git a/crc/static/bpmn/core_info/core_info.bpmn b/crc/static/bpmn/core_info/core_info.bpmn index 8c69ffb3..9763ced6 100644 --- a/crc/static/bpmn/core_info/core_info.bpmn +++ b/crc/static/bpmn/core_info/core_info.bpmn @@ -212,7 +212,8 @@ SequenceFlow_1r3yrhy Flow_09h1imz - #! StudyInfo details + StudyInfo = {} +StudyInfo['details'] = study_info('details') Flow_09h1imz diff --git a/crc/static/bpmn/data_security_plan/data_security_plan.bpmn b/crc/static/bpmn/data_security_plan/data_security_plan.bpmn index 86426d6d..3bf309b7 100644 --- a/crc/static/bpmn/data_security_plan/data_security_plan.bpmn +++ b/crc/static/bpmn/data_security_plan/data_security_plan.bpmn @@ -453,7 +453,7 @@ Indicate all the possible formats in which you will transmit your data outside o SequenceFlow_0k2r83n SequenceFlow_0t6xl9i SequenceFlow_16kyite - #! CompleteTemplate NEW_DSP_template.docx Study_DataSecurityPlan + complete_template('NEW_DSP_template.docx','Study_DataSecurityPlan')/bpmn:script> ##### Instructions diff --git a/crc/static/bpmn/documents_approvals/documents_approvals.bpmn b/crc/static/bpmn/documents_approvals/documents_approvals.bpmn index 12e85e34..858e95d6 100644 --- a/crc/static/bpmn/documents_approvals/documents_approvals.bpmn +++ b/crc/static/bpmn/documents_approvals/documents_approvals.bpmn @@ -53,12 +53,14 @@ Flow_0c7ryff Flow_142jtxs - #! StudyInfo approvals + StudyInfo = {} +StudyInfo['approvals'] = study_info('approvals') Flow_1k3su2q Flow_0c7ryff - #! StudyInfo documents + StudyInfo = {} +StudyInfo['documents'] = study_info('documents') diff --git a/crc/static/bpmn/ide_supplement/ide_supplement.bpmn b/crc/static/bpmn/ide_supplement/ide_supplement.bpmn index 7a83643b..394a1d46 100644 --- a/crc/static/bpmn/ide_supplement/ide_supplement.bpmn +++ b/crc/static/bpmn/ide_supplement/ide_supplement.bpmn @@ -36,7 +36,8 @@ SequenceFlow_1dhb8f4 SequenceFlow_1uzcl1f - #! StudyInfo details + StudyInfo = {} +StudyInfo['details'] = study_info('details') diff --git a/crc/static/bpmn/ids_full_submission/ids_full_submission.bpmn b/crc/static/bpmn/ids_full_submission/ids_full_submission.bpmn index 25a9ad6e..4e940cf4 100644 --- a/crc/static/bpmn/ids_full_submission/ids_full_submission.bpmn +++ b/crc/static/bpmn/ids_full_submission/ids_full_submission.bpmn @@ -217,7 +217,8 @@ Protocol Owner: **(need to insert value here)** SequenceFlow_1dexemq Flow_1x9d2mo - #! StudyInfo documents + StudyInfo = {} +StudyInfo['documents'] = study_info('documents') diff --git a/crc/static/bpmn/ind_update/ind_update.bpmn b/crc/static/bpmn/ind_update/ind_update.bpmn index 528a87ce..1d288dc6 100644 --- a/crc/static/bpmn/ind_update/ind_update.bpmn +++ b/crc/static/bpmn/ind_update/ind_update.bpmn @@ -12,7 +12,8 @@ SequenceFlow_1dhb8f4 SequenceFlow_1uzcl1f - #! StudyInfo details + StudyInfo = {} +StudyInfo['details'] = study_info('details') diff --git a/crc/static/bpmn/irb_api_details/irb_api_details.bpmn b/crc/static/bpmn/irb_api_details/irb_api_details.bpmn index b4f540f5..9e0b6271 100644 --- a/crc/static/bpmn/irb_api_details/irb_api_details.bpmn +++ b/crc/static/bpmn/irb_api_details/irb_api_details.bpmn @@ -8,7 +8,8 @@ SequenceFlow_1fmyo77 SequenceFlow_18nr0gf - #! StudyInfo details + StudyInfo = {} +StudyInfo['details'] = study_info('details') diff --git a/crc/static/bpmn/irb_api_personnel/irb_api_personnel.bpmn b/crc/static/bpmn/irb_api_personnel/irb_api_personnel.bpmn index a5258cbe..c347fd93 100644 --- a/crc/static/bpmn/irb_api_personnel/irb_api_personnel.bpmn +++ b/crc/static/bpmn/irb_api_personnel/irb_api_personnel.bpmn @@ -7,7 +7,8 @@ Flow_0kcrx5l Flow_1dcsioh - #! StudyInfo investigators + StudyInfo = {} +StudyInfo['investigators'] = study_info('investigators') ## The following information was gathered: diff --git a/crc/static/bpmn/research_rampup/research_rampup.bpmn b/crc/static/bpmn/research_rampup/research_rampup.bpmn index 4a04eb6d..5703daaf 100644 --- a/crc/static/bpmn/research_rampup/research_rampup.bpmn +++ b/crc/static/bpmn/research_rampup/research_rampup.bpmn @@ -598,7 +598,7 @@ Use the EHS [Lab Safety Plan During COVID 19 template](https://www.google.com/ur This step is internal to the system and do not require and user interaction Flow_11uqavk Flow_0aqgwvu - #! CompleteTemplate ResearchRampUpPlan.docx RESEARCH_RAMPUP + complete_template('ResearchRampUpPlan.docx','RESEARCH_RAMPUP')/bpmn:script> @@ -764,7 +764,7 @@ This step is internal to the system and do not require and user interaction Flow_16y8glw Flow_0uc4o6c - #! UpdateStudy title:PIComputingID.label pi:PIComputingID.value + update_study('title:PIComputingID.label','pi:PIComputingID.value') #### Weekly Personnel Schedule(s) diff --git a/crc/static/bpmn/top_level_workflow/top_level_workflow.bpmn b/crc/static/bpmn/top_level_workflow/top_level_workflow.bpmn index 23d6ff72..61e59156 100644 --- a/crc/static/bpmn/top_level_workflow/top_level_workflow.bpmn +++ b/crc/static/bpmn/top_level_workflow/top_level_workflow.bpmn @@ -11,7 +11,8 @@ SequenceFlow_1ees8ka SequenceFlow_17ct47v - #! StudyInfo documents + StudyInfo = {} +StudyInfo['documents'] = study_info('documents') Flow_1m8285h @@ -62,7 +63,8 @@ Flow_0pwtiqm Flow_0eq6px2 - #! StudyInfo details + StudyInfo = {} +StudyInfo['details'] = study_info('details') Flow_14ce1d7 @@ -91,7 +93,8 @@ Flow_1qyrmzn Flow_0vo6ul1 - #! StudyInfo investigators + StudyInfo = {} +StudyInfo['investigators'] = study_info('investigators') diff --git a/tests/data/docx/docx.bpmn b/tests/data/docx/docx.bpmn index 8c741114..e5b0cdcf 100644 --- a/tests/data/docx/docx.bpmn +++ b/tests/data/docx/docx.bpmn @@ -27,7 +27,7 @@ SequenceFlow_1i7hk1a SequenceFlow_11c35oq - #! CompleteTemplate Letter.docx AD_CoCApp + complete_template('Letter.docx AD_CoCApp')/bpmn:script> SequenceFlow_11c35oq diff --git a/tests/data/email/email.bpmn b/tests/data/email/email.bpmn index 11ecec2e..3395e788 100644 --- a/tests/data/email/email.bpmn +++ b/tests/data/email/email.bpmn @@ -20,7 +20,7 @@ Email content to be delivered to {{ ApprvlApprvr1 }} --- Flow_08n2npe Flow_1xlrgne - #! Email "Camunda Email Subject" ApprvlApprvr1 PIComputingID + email("Camunda Email Subject",'ApprvlApprvr1','PIComputingID') diff --git a/tests/data/invalid_script/invalid_script.bpmn b/tests/data/invalid_script/invalid_script.bpmn index b85e2bc4..af470aa1 100644 --- a/tests/data/invalid_script/invalid_script.bpmn +++ b/tests/data/invalid_script/invalid_script.bpmn @@ -11,7 +11,7 @@ SequenceFlow_1pnq3kg SequenceFlow_12pf6um - #! NoSuchScript withArg1 + no_such_script('withArg1') diff --git a/tests/data/multi_instance/multi_instance.bpmn b/tests/data/multi_instance/multi_instance.bpmn index c1e610a5..50674246 100644 --- a/tests/data/multi_instance/multi_instance.bpmn +++ b/tests/data/multi_instance/multi_instance.bpmn @@ -29,7 +29,8 @@ Flow_0t6p1sb SequenceFlow_1p568pp - #! StudyInfo investigators + StudyInfo = {} +StudyInfo['investigators'] = study_info('investigators') diff --git a/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn b/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn index d20c8499..0c4ff40c 100644 --- a/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn +++ b/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn @@ -29,7 +29,8 @@ Flow_0t6p1sb SequenceFlow_1p568pp - #! StudyInfo investigators + StudyInfo = {} +StudyInfo['investigators'] = study_info('investigators') diff --git a/tests/data/random_fact/random_fact.bpmn b/tests/data/random_fact/random_fact.bpmn index d5ffcbed..234dd160 100644 --- a/tests/data/random_fact/random_fact.bpmn +++ b/tests/data/random_fact/random_fact.bpmn @@ -132,7 +132,7 @@ Autoconverted link https://github.com/nodeca/pica (enable linkify to see) SequenceFlow_0641sh6 SequenceFlow_0t29gjo - #! FactService + fact_service() # Great Job! diff --git a/tests/data/study_details/study_details.bpmn b/tests/data/study_details/study_details.bpmn index 888a28d9..62714433 100644 --- a/tests/data/study_details/study_details.bpmn +++ b/tests/data/study_details/study_details.bpmn @@ -8,7 +8,8 @@ SequenceFlow_1nfe5m9 SequenceFlow_1bqiin0 - #! StudyInfo info + StudyInfo = {} +StudyInfo['info'] = study_info('info') @@ -18,7 +19,7 @@ SequenceFlow_1bqiin0 Flow_0ochvmi - study = studyInfo('info','p') + study = study_info('info','p') diff --git a/tests/data/top_level_workflow/top_level_workflow.bpmn b/tests/data/top_level_workflow/top_level_workflow.bpmn index 8b1bb888..3cb74fd9 100644 --- a/tests/data/top_level_workflow/top_level_workflow.bpmn +++ b/tests/data/top_level_workflow/top_level_workflow.bpmn @@ -11,7 +11,8 @@ SequenceFlow_1ees8ka SequenceFlow_17ct47v - #! StudyInfo documents + StudyInfo = {} +StudyInfo['documents'] = study_info('documents') Flow_1m8285h From 70ad3872a7ca84b8dda943faec0b645d6939b216 Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Mon, 27 Jul 2020 12:02:34 -0400 Subject: [PATCH 4/7] Fix several bugs, most had an issue with the bpmn document --- crc/services/workflow_processor.py | 15 +++++++++++---- .../data_security_plan/data_security_plan.bpmn | 2 +- .../bpmn/research_rampup/research_rampup.bpmn | 4 ++-- tests/data/docx/docx.bpmn | 2 +- .../multi_instance_parallel.bpmn | 2 +- tests/data/random_fact/random_fact.bpmn | 2 +- .../workflow/test_workflow_spec_validation_api.py | 4 ++-- 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index d29c3a0e..55100afd 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -53,10 +53,17 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): super().execute(task, script, data, externalMethods=augmentMethods) except SyntaxError as e: - 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) + del(task.data['task']) + raise ApiError('syntax_error', + f'Something is wrong with your python script ' + f'please correct the following:' + f' {script}, {e.msg}') + except NameError as e: + del(task.data['task']) + raise ApiError('name_error', + f'something you are referencing does not exist:' + f' {script}, {e.name}') + # else: # self.run_predefined_script(task, script[2:], data) # strip off the first two characters. diff --git a/crc/static/bpmn/data_security_plan/data_security_plan.bpmn b/crc/static/bpmn/data_security_plan/data_security_plan.bpmn index 3bf309b7..87b931b9 100644 --- a/crc/static/bpmn/data_security_plan/data_security_plan.bpmn +++ b/crc/static/bpmn/data_security_plan/data_security_plan.bpmn @@ -453,7 +453,7 @@ Indicate all the possible formats in which you will transmit your data outside o SequenceFlow_0k2r83n SequenceFlow_0t6xl9i SequenceFlow_16kyite - complete_template('NEW_DSP_template.docx','Study_DataSecurityPlan')/bpmn:script> + complete_template('NEW_DSP_template.docx','Study_DataSecurityPlan') ##### Instructions diff --git a/crc/static/bpmn/research_rampup/research_rampup.bpmn b/crc/static/bpmn/research_rampup/research_rampup.bpmn index 5703daaf..eaa1dab7 100644 --- a/crc/static/bpmn/research_rampup/research_rampup.bpmn +++ b/crc/static/bpmn/research_rampup/research_rampup.bpmn @@ -598,7 +598,7 @@ Use the EHS [Lab Safety Plan During COVID 19 template](https://www.google.com/ur This step is internal to the system and do not require and user interaction Flow_11uqavk Flow_0aqgwvu - complete_template('ResearchRampUpPlan.docx','RESEARCH_RAMPUP')/bpmn:script> + complete_template('ResearchRampUpPlan.docx','RESEARCH_RAMPUP') @@ -755,7 +755,7 @@ Notify the Area Monitor for This step is internal to the system and do not require and user interaction Flow_0j4rs82 Flow_07ge8uf - #!RequestApproval ApprvlApprvr1 ApprvlApprvr2 + request_approval('ApprvlApprvr1','ApprvlApprvr2') #### Script Task diff --git a/tests/data/docx/docx.bpmn b/tests/data/docx/docx.bpmn index e5b0cdcf..fe11b7c5 100644 --- a/tests/data/docx/docx.bpmn +++ b/tests/data/docx/docx.bpmn @@ -27,7 +27,7 @@ SequenceFlow_1i7hk1a SequenceFlow_11c35oq - complete_template('Letter.docx AD_CoCApp')/bpmn:script> + complete_template('Letter.docx','AD_CoCApp') SequenceFlow_11c35oq diff --git a/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn b/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn index 0c4ff40c..0c31670e 100644 --- a/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn +++ b/tests/data/multi_instance_parallel/multi_instance_parallel.bpmn @@ -11,7 +11,7 @@ # Please provide addtional information about: -## Investigator ID: {{investigator.user_id}} +## Investigator ID: {{investigator.user_id}} ## Role: {{investigator.type_full}} diff --git a/tests/data/random_fact/random_fact.bpmn b/tests/data/random_fact/random_fact.bpmn index 234dd160..6bee51b7 100644 --- a/tests/data/random_fact/random_fact.bpmn +++ b/tests/data/random_fact/random_fact.bpmn @@ -132,7 +132,7 @@ Autoconverted link https://github.com/nodeca/pica (enable linkify to see) SequenceFlow_0641sh6 SequenceFlow_0t29gjo - fact_service() + FactService = fact_service() # Great Job! diff --git a/tests/workflow/test_workflow_spec_validation_api.py b/tests/workflow/test_workflow_spec_validation_api.py index 0c17892e..da389168 100644 --- a/tests/workflow/test_workflow_spec_validation_api.py +++ b/tests/workflow/test_workflow_spec_validation_api.py @@ -89,7 +89,7 @@ class TestWorkflowSpecValidation(BaseTest): self.load_example_data() errors = self.validate_workflow("invalid_script") self.assertEqual(2, len(errors)) - self.assertEqual("error_loading_workflow", errors[0]['code']) + self.assertEqual("workflow_validation_exception", errors[0]['code']) self.assertTrue("NoSuchScript" in errors[0]['message']) self.assertEqual("Invalid_Script_Task", errors[0]['task_id']) self.assertEqual("An Invalid Script Reference", errors[0]['task_name']) @@ -99,7 +99,7 @@ class TestWorkflowSpecValidation(BaseTest): self.load_example_data() errors = self.validate_workflow("invalid_script2") self.assertEqual(2, len(errors)) - self.assertEqual("error_loading_workflow", errors[0]['code']) + self.assertEqual("workflow_validation_exception", errors[0]['code']) 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']) From cc55aed89cc4ed1a078c80121ea0715172ac2f6d Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Mon, 27 Jul 2020 12:18:28 -0400 Subject: [PATCH 5/7] Change exception name --- crc/scripts/study_info.py | 4 ++-- tests/workflow/test_workflow_spec_validation_api.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crc/scripts/study_info.py b/crc/scripts/study_info.py index c392d40c..6daf91ec 100644 --- a/crc/scripts/study_info.py +++ b/crc/scripts/study_info.py @@ -196,8 +196,8 @@ Returns information specific to the protocol. } } } - self.add_data_to_task(task=task, data=data["study"]) - self.add_data_to_task(task, {"documents": StudyService().get_documents_status(study_id)}) + #self.add_data_to_task(task=task, data=data["study"]) + #self.add_data_to_task(task, {"documents": StudyService().get_documents_status(study_id)}) def do_task(self, task, study_id, workflow_id, *args, **kwargs): self.check_args(args,2) diff --git a/tests/workflow/test_workflow_spec_validation_api.py b/tests/workflow/test_workflow_spec_validation_api.py index da389168..29fd5a14 100644 --- a/tests/workflow/test_workflow_spec_validation_api.py +++ b/tests/workflow/test_workflow_spec_validation_api.py @@ -90,7 +90,7 @@ class TestWorkflowSpecValidation(BaseTest): errors = self.validate_workflow("invalid_script") self.assertEqual(2, len(errors)) self.assertEqual("workflow_validation_exception", errors[0]['code']) - self.assertTrue("NoSuchScript" in errors[0]['message']) + #self.assertTrue("NoSuchScript" 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_script.bpmn", errors[0]['file_name']) From d617af85659b50b97283cba2485a0edcfdeb85ca Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Tue, 28 Jul 2020 11:02:49 -0400 Subject: [PATCH 6/7] All tests are passing - may need to refactor a bit, / remove comments --- crc/scripts/script.py | 38 ++++++++++++++++--- crc/scripts/study_info.py | 15 +++++--- crc/services/workflow_processor.py | 7 ++-- .../documents_approvals.bpmn | 3 +- 4 files changed, 46 insertions(+), 17 deletions(-) diff --git a/crc/scripts/script.py b/crc/scripts/script.py index ba5af5b7..84c2ee05 100644 --- a/crc/scripts/script.py +++ b/crc/scripts/script.py @@ -45,6 +45,32 @@ class Script(object): workflow_id) return execlist + @staticmethod + def generate_augmented_validate_list(task, study_id, workflow_id): + """ + this makes a dictionary of lambda functions that are closed over the class instance that + They represent. This is passed into PythonScriptParser as a list of helper functions that are + available for running. In general, they maintain the do_task call structure that they had, but + they always return a value rather than updating the task data. + + We may be able to remove the task for each of these calls if we are not using it other than potentially + updating the task data. + """ + + def make_closure_validate(subclass,task,study_id,workflow_id): + instance = subclass() + return lambda *a : subclass.do_task_validate_only(instance,task,study_id,workflow_id,*a) + execlist = {} + subclasses = Script.get_all_subclasses() + for x in range(len(subclasses)): + subclass = subclasses[x] + execlist[subclass.__module__.split('.')[-1]] = make_closure_validate(subclass,task,study_id, + workflow_id) + return execlist + + + + @staticmethod def get_all_subclasses(): return Script._get_all_subclasses(Script) @@ -67,12 +93,12 @@ class Script(object): return all_subclasses - # def add_data_to_task(self, task, data): - # key = self.__class__.__name__ - # if key in task.data: - # task.data[key].update(data) - # else: - # task.data[key] = data + def add_data_to_task(self, task, data): + key = self.__class__.__name__ + if key in task.data: + task.data[key].update(data) + else: + task.data[key] = data class ScriptValidationError: diff --git a/crc/scripts/study_info.py b/crc/scripts/study_info.py index 6daf91ec..6b55e0fd 100644 --- a/crc/scripts/study_info.py +++ b/crc/scripts/study_info.py @@ -149,11 +149,11 @@ Returns information specific to the protocol. def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): """For validation only, pretend no results come back from pb""" - self.check_args(args) + self.check_args(args,2) # Assure the reference file exists (a bit hacky, but we want to raise this error early, and cleanly.) FileService.get_reference_file_data(FileService.DOCUMENT_LIST) FileService.get_reference_file_data(FileService.INVESTIGATOR_LIST) - data = { + data = Box({ "study":{ "info": { "id": 12, @@ -195,7 +195,10 @@ Returns information specific to the protocol. 'id': 0, } } - } + }) + if args[0]=='documents': + return StudyService().get_documents_status(study_id) + return data['study'][args[0]] #self.add_data_to_task(task=task, data=data["study"]) #self.add_data_to_task(task, {"documents": StudyService().get_documents_status(study_id)}) @@ -205,9 +208,9 @@ Returns information specific to the protocol. if len(args) > 1: prefix = args[1] cmd = args[0] - study_info = {} - if self.__class__.__name__ in task.data: - study_info = task.data[self.__class__.__name__] + # study_info = {} + # if self.__class__.__name__ in task.data: + # study_info = task.data[self.__class__.__name__] retval = None if cmd == 'info': study = session.query(StudyModel).filter_by(id=study_id).first() diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 55100afd..f9243e68 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -49,17 +49,18 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): workflow_id = None try: - augmentMethods = Script.generate_augmented_list(task,study_id,workflow_id) + if task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: + augmentMethods = Script.generate_augmented_validate_list(task, study_id, workflow_id) + else: + augmentMethods = Script.generate_augmented_list(task, study_id, workflow_id) super().execute(task, script, data, externalMethods=augmentMethods) except SyntaxError as e: - del(task.data['task']) raise ApiError('syntax_error', f'Something is wrong with your python script ' f'please correct the following:' f' {script}, {e.msg}') except NameError as e: - del(task.data['task']) raise ApiError('name_error', f'something you are referencing does not exist:' f' {script}, {e.name}') diff --git a/crc/static/bpmn/documents_approvals/documents_approvals.bpmn b/crc/static/bpmn/documents_approvals/documents_approvals.bpmn index 858e95d6..c7130ee4 100644 --- a/crc/static/bpmn/documents_approvals/documents_approvals.bpmn +++ b/crc/static/bpmn/documents_approvals/documents_approvals.bpmn @@ -53,8 +53,7 @@ Flow_0c7ryff Flow_142jtxs - StudyInfo = {} -StudyInfo['approvals'] = study_info('approvals') + StudyInfo['approvals'] = study_info('approvals') Flow_1k3su2q From 9704cbcb26c8d11edbe40d81c507ad01e66c771b Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Thu, 30 Jul 2020 13:35:20 -0400 Subject: [PATCH 7/7] Modifications to the ldap scripts to bring them back in line with what Kelly is doing with the evaluation process. --- Pipfile | 2 +- Pipfile.lock | 4 +- crc/scripts/ldap.py | 50 +++++++++++++++ crc/scripts/ldap_lookup.py | 78 ----------------------- crc/scripts/ldap_replace.py | 60 ----------------- crc/services/workflow_processor.py | 14 +--- tests/data/ldap_replace/ldap_replace.bpmn | 17 ++--- tests/ldap/test_ldap_lookup_script.py | 74 +++++---------------- 8 files changed, 82 insertions(+), 217 deletions(-) create mode 100644 crc/scripts/ldap.py delete mode 100644 crc/scripts/ldap_lookup.py delete mode 100644 crc/scripts/ldap_replace.py diff --git a/Pipfile b/Pipfile index 56f3bc26..009e370c 100644 --- a/Pipfile +++ b/Pipfile @@ -38,7 +38,7 @@ recommonmark = "*" requests = "*" sentry-sdk = {extras = ["flask"],version = "==0.14.4"} sphinx = "*" -spiffworkflow = {git = "https://github.com/sartography/SpiffWorkflow.git",ref = "master"} +spiffworkflow = {git = "https://github.com/sartography/SpiffWorkflow.git",ref = "cr-connect-106-augment-eval"} #spiffworkflow = {editable = true,path="/home/kelly/sartography/SpiffWorkflow/"} swagger-ui-bundle = "*" webtest = "*" diff --git a/Pipfile.lock b/Pipfile.lock index 8aabe194..36b98856 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "381d29428eb328ad6167774b510b9d818bd1505b95f50454a19f1564782326cc" + "sha256": "afb6a541d1a9f33155f91529ad961492dceded89466aa1e02fed9901ac5eb146" }, "pipfile-spec": 6, "requires": { @@ -804,7 +804,7 @@ }, "spiffworkflow": { "git": "https://github.com/sartography/SpiffWorkflow.git", - "ref": "11ad40bbcb0fbd3c5bc1078e4989dc38b749f7f3" + "ref": "7712830665b4419df019413ac095cb0749adb346" }, "sqlalchemy": { "hashes": [ diff --git a/crc/scripts/ldap.py b/crc/scripts/ldap.py new file mode 100644 index 00000000..577d4a75 --- /dev/null +++ b/crc/scripts/ldap.py @@ -0,0 +1,50 @@ +import copy + +from crc import app +from crc.api.common import ApiError +from crc.scripts.script import Script +from crc.services.ldap_service import LdapService + + +class Ldap(Script): + """This Script allows to be introduced as part of a workflow and called from there, taking + a UID (or several) as input and looking it up through LDAP to return the person's details """ + + def get_description(self): + return """ +Attempts to create a dictionary with person details, using the +provided argument (a UID) and look it up through LDAP. + +Examples: +supervisor_info = ldap(supervisor_uid) // Sets the supervisor information to ldap details for the given uid. +""" + + def do_task_validate_only(self, task, *args, **kwargs): + return self.set_users_info_in_task(task, args) + + def do_task(self, task, study_id, workflow_id, *args, **kwargs): + return self.set_users_info_in_task(task, args) + + def set_users_info_in_task(self, task, args): + if len(args) != 1: + raise ApiError(code="missing_argument", + message="Ldap takes a single argument, the " + "UID for the person we want to look up") + uid = args[0] + user_info_dict = {} + + user_info = LdapService.user_info(uid) + user_info_dict = { + "display_name": user_info.display_name, + "given_name": user_info.given_name, + "email_address": user_info.email_address, + "telephone_number": user_info.telephone_number, + "title": user_info.title, + "department": user_info.department, + "affiliation": user_info.affiliation, + "sponsor_type": user_info.sponsor_type, + "uid": user_info.uid, + "proper_name": user_info.proper_name() + } + + return user_info_dict diff --git a/crc/scripts/ldap_lookup.py b/crc/scripts/ldap_lookup.py deleted file mode 100644 index 62bd287a..00000000 --- a/crc/scripts/ldap_lookup.py +++ /dev/null @@ -1,78 +0,0 @@ -import copy - -from crc import app -from crc.api.common import ApiError -from crc.scripts.script import Script -from crc.services.ldap_service import LdapService - - -USER_DETAILS = { - "PIComputingID": { - "value": "", - "data": { - }, - "label": "invalid uid" - } -} - - -class LdapLookup(Script): - """This Script allows to be introduced as part of a workflow and called from there, taking - a UID as input and looking it up through LDAP to return the person's details """ - - def get_description(self): - return """ -Attempts to create a dictionary with person details, using the -provided argument (a UID) and look it up through LDAP. - -Example: -LdapLookup PIComputingID -""" - - def do_task_validate_only(self, task, *args, **kwargs): - self.get_user_info(task, args) - - def do_task(self, task, *args, **kwargs): - args = [arg for arg in args if type(arg) == str] - user_info = self.get_user_info(task, args) - - user_details = copy.deepcopy(USER_DETAILS) - user_details['PIComputingID']['value'] = user_info['uid'] - if len(user_info.keys()) > 1: - user_details['PIComputingID']['label'] = user_info.pop('label') - else: - user_info.pop('uid') - user_details['PIComputingID']['data'] = user_info - return user_details - - def get_user_info(self, task, args): - if len(args) < 1: - raise ApiError(code="missing_argument", - message="Ldap lookup script requires one argument. The " - "UID for the person we want to look up") - - arg = args.pop() # Extracting only one value for now - uid = task.workflow.script_engine.evaluate_expression(task, arg) - if not isinstance(uid, str): - raise ApiError(code="invalid_argument", - message="Ldap lookup script requires one 1 UID argument, of type string.") - user_info_dict = {} - try: - user_info = LdapService.user_info(uid) - user_info_dict = { - "display_name": user_info.display_name, - "given_name": user_info.given_name, - "email_address": user_info.email_address, - "telephone_number": user_info.telephone_number, - "title": user_info.title, - "department": user_info.department, - "affiliation": user_info.affiliation, - "sponsor_type": user_info.sponsor_type, - "uid": user_info.uid, - "label": user_info.proper_name() - } - except: - user_info_dict['uid'] = uid - app.logger.error(f'Ldap lookup failed for UID {uid}') - - return user_info_dict diff --git a/crc/scripts/ldap_replace.py b/crc/scripts/ldap_replace.py deleted file mode 100644 index 88e2986a..00000000 --- a/crc/scripts/ldap_replace.py +++ /dev/null @@ -1,60 +0,0 @@ -import copy - -from crc import app -from crc.api.common import ApiError -from crc.scripts.script import Script -from crc.services.ldap_service import LdapService - - -class LdapReplace(Script): - """This Script allows to be introduced as part of a workflow and called from there, taking - a UID (or several) as input and looking it up through LDAP to return the person's details """ - - def get_description(self): - return """ -Attempts to create a dictionary with person details, using the -provided argument (a UID) and look it up through LDAP. - -Examples: -#! LdapReplace supervisor -#! LdapReplace supervisor collaborator -#! LdapReplace supervisor cosupervisor collaborator -""" - - def do_task_validate_only(self, task, *args, **kwargs): - self.set_users_info_in_task(task, args) - - def do_task(self, task, *args, **kwargs): - args = [arg for arg in args if type(arg) == str] - self.set_users_info_in_task(task, args) - - def set_users_info_in_task(self, task, args): - if len(args) < 1: - raise ApiError(code="missing_argument", - message="Ldap replace script requires at least one argument. The " - "UID for the person(s) we want to look up") - - users_info = {} - for arg in args: - uid = task.workflow.script_engine.evaluate_expression(task, arg) - if not isinstance(uid, str): - raise ApiError(code="invalid_argument", - message="Ldap replace script found an invalid argument, type string is required") - user_info_dict = {} - try: - user_info = LdapService.user_info(uid) - user_info_dict = { - "display_name": user_info.display_name, - "given_name": user_info.given_name, - "email_address": user_info.email_address, - "telephone_number": user_info.telephone_number, - "title": user_info.title, - "department": user_info.department, - "affiliation": user_info.affiliation, - "sponsor_type": user_info.sponsor_type, - "uid": user_info.uid, - "proper_name": user_info.proper_name() - } - except: - app.logger.error(f'Ldap replace failed for UID {uid}') - task.data[arg] = user_info_dict diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index f9243e68..2ec13702 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -29,19 +29,11 @@ from crc import app class CustomBpmnScriptEngine(BpmnScriptEngine): """This is a custom script processor that can be easily injected into Spiff Workflow. - Rather than execute arbitrary code, this assumes the script references a fully qualified python class - such as myapp.RandomFact. """ + It will execute python code read in from the bpmn. It will also make any scripts in the + scripts directory available for execution. """ def execute(self, task: SpiffTask, script, data): - """ - 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 + study_id = task.workflow.data[WorkflowProcessor.STUDY_ID_KEY] if WorkflowProcessor.WORKFLOW_ID_KEY in task.workflow.data: workflow_id = task.workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] diff --git a/tests/data/ldap_replace/ldap_replace.bpmn b/tests/data/ldap_replace/ldap_replace.bpmn index 77f8c7ad..64389299 100644 --- a/tests/data/ldap_replace/ldap_replace.bpmn +++ b/tests/data/ldap_replace/ldap_replace.bpmn @@ -10,7 +10,8 @@ Flow_08n2npe Flow_1xlrgne - #! LdapReplace Supervisor Investigator + Supervisor = ldap(Supervisor) +Investigator = ldap(Investigator) @@ -33,6 +34,10 @@ + + + + @@ -45,22 +50,18 @@ - - - - + + + - - - diff --git a/tests/ldap/test_ldap_lookup_script.py b/tests/ldap/test_ldap_lookup_script.py index 220ca9c8..0a88a899 100644 --- a/tests/ldap/test_ldap_lookup_script.py +++ b/tests/ldap/test_ldap_lookup_script.py @@ -1,7 +1,8 @@ from tests.base_test import BaseTest from crc.services.workflow_processor import WorkflowProcessor -from crc.scripts.ldap_replace import LdapReplace +from crc.scripts.ldap import Ldap +from crc.api.common import ApiError from crc import db, mail @@ -14,60 +15,19 @@ class TestLdapLookupScript(BaseTest): processor = WorkflowProcessor(workflow) task = processor.next_task() - task.data = { - 'PIComputingID': 'dhf8r' - } + script = Ldap() + user_details = script.do_task(task, workflow.study_id, workflow.id, "dhf8r") - script = LdapReplace() - user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") - - self.assertEqual(task.data['PIComputingID']['display_name'], 'Dan Funk') - self.assertEqual(task.data['PIComputingID']['given_name'], 'Dan') - self.assertEqual(task.data['PIComputingID']['email_address'], 'dhf8r@virginia.edu') - self.assertEqual(task.data['PIComputingID']['telephone_number'], '+1 (434) 924-1723') - self.assertEqual(task.data['PIComputingID']['title'], 'E42:He\'s a hoopy frood') - self.assertEqual(task.data['PIComputingID']['department'], 'E0:EN-Eng Study of Parallel Universes') - self.assertEqual(task.data['PIComputingID']['affiliation'], 'faculty') - self.assertEqual(task.data['PIComputingID']['sponsor_type'], 'Staff') - self.assertEqual(task.data['PIComputingID']['uid'], 'dhf8r') - self.assertEqual(task.data['PIComputingID']['proper_name'], 'Dan Funk - (dhf8r)') - - def test_get_existing_users_details(self): - self.load_example_data() - self.create_reference_document() - workflow = self.create_workflow('empty_workflow') - processor = WorkflowProcessor(workflow) - task = processor.next_task() - - task.data = { - 'supervisor': 'dhf8r', - 'investigator': 'lb3dp' - } - - script = LdapReplace() - user_details = script.do_task(task, workflow.study_id, workflow.id, "supervisor", "investigator") - - self.assertEqual(task.data['supervisor']['display_name'], 'Dan Funk') - self.assertEqual(task.data['supervisor']['given_name'], 'Dan') - self.assertEqual(task.data['supervisor']['email_address'], 'dhf8r@virginia.edu') - self.assertEqual(task.data['supervisor']['telephone_number'], '+1 (434) 924-1723') - self.assertEqual(task.data['supervisor']['title'], 'E42:He\'s a hoopy frood') - self.assertEqual(task.data['supervisor']['department'], 'E0:EN-Eng Study of Parallel Universes') - self.assertEqual(task.data['supervisor']['affiliation'], 'faculty') - self.assertEqual(task.data['supervisor']['sponsor_type'], 'Staff') - self.assertEqual(task.data['supervisor']['uid'], 'dhf8r') - self.assertEqual(task.data['supervisor']['proper_name'], 'Dan Funk - (dhf8r)') - - self.assertEqual(task.data['investigator']['display_name'], 'Laura Barnes') - self.assertEqual(task.data['investigator']['given_name'], 'Laura') - self.assertEqual(task.data['investigator']['email_address'], 'lb3dp@virginia.edu') - self.assertEqual(task.data['investigator']['telephone_number'], '+1 (434) 924-1723') - self.assertEqual(task.data['investigator']['title'], 'E0:Associate Professor of Systems and Information Engineering') - self.assertEqual(task.data['investigator']['department'], 'E0:EN-Eng Sys and Environment') - self.assertEqual(task.data['investigator']['affiliation'], 'faculty') - self.assertEqual(task.data['investigator']['sponsor_type'], 'Staff') - self.assertEqual(task.data['investigator']['uid'], 'lb3dp') - self.assertEqual(task.data['investigator']['proper_name'], 'Laura Barnes - (lb3dp)') + self.assertEqual(user_details['display_name'], 'Dan Funk') + self.assertEqual(user_details['given_name'], 'Dan') + self.assertEqual(user_details['email_address'], 'dhf8r@virginia.edu') + self.assertEqual(user_details['telephone_number'], '+1 (434) 924-1723') + self.assertEqual(user_details['title'], 'E42:He\'s a hoopy frood') + self.assertEqual(user_details['department'], 'E0:EN-Eng Study of Parallel Universes') + self.assertEqual(user_details['affiliation'], 'faculty') + self.assertEqual(user_details['sponsor_type'], 'Staff') + self.assertEqual(user_details['uid'], 'dhf8r') + self.assertEqual(user_details['proper_name'], 'Dan Funk - (dhf8r)') def test_get_invalid_user_details(self): self.load_example_data() @@ -80,10 +40,10 @@ class TestLdapLookupScript(BaseTest): 'PIComputingID': 'rec3z' } - script = LdapReplace() - user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") + script = Ldap() + with(self.assertRaises(ApiError)): + user_details = script.do_task(task, workflow.study_id, workflow.id, "PIComputingID") - self.assertEqual(task.data['PIComputingID'], {}) def test_bpmn_task_receives_user_details(self): workflow = self.create_workflow('ldap_replace')