From ccbf374b403b5ab8441b7dec9b630d4af4cdf310 Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Tue, 26 May 2020 20:06:50 -0400 Subject: [PATCH] Loads of bug fixes. Modifed the request_approval to take a list of arguments, which works better for us... today. UpdateStudy correctly handles validation. WorkflowService correctly populates random values from lookup tables. And several fixes down in Spiffworkflow, including a big bug where only the last item in a decision table made it through. --- Pipfile.lock | 14 ++++----- crc/scripts/request_approval.py | 25 ++++++++------- crc/scripts/update_study.py | 13 ++++++-- crc/services/workflow_service.py | 45 ++++++++++++++++++++------- tests/test_request_approval_script.py | 10 +++--- 5 files changed, 70 insertions(+), 37 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 0360e027..54962466 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -296,10 +296,10 @@ }, "flask-sqlalchemy": { "hashes": [ - "sha256:0078d8663330dc05a74bc72b3b6ddc441b9a744e2f56fe60af1a5bfc81334327", - "sha256:6974785d913666587949f7c2946f7001e4fa2cb2d19f4e69ead02e4b8f50b33d" + "sha256:0b656fbf87c5f24109d859bafa791d29751fabbda2302b606881ae5485b557a5", + "sha256:fcfe6df52cd2ed8a63008ca36b86a51fa7a4b70cef1c39e5625f722fca32308e" ], - "version": "==2.4.1" + "version": "==2.4.3" }, "future": { "hashes": [ @@ -727,11 +727,11 @@ }, "sphinx": { "hashes": [ - "sha256:62edfd92d955b868d6c124c0942eba966d54b5f3dcb4ded39e65f74abac3f572", - "sha256:f5505d74cf9592f3b997380f9bdb2d2d0320ed74dd69691e3ee0644b956b8d83" + "sha256:779a519adbd3a70fc7c468af08c5e74829868b0a5b34587b33340e010291856c", + "sha256:ea64df287958ee5aac46be7ac2b7277305b0381d213728c3a49d8bb9b8415807" ], "index": "pypi", - "version": "==3.0.3" + "version": "==3.0.4" }, "sphinxcontrib-applehelp": { "hashes": [ @@ -778,7 +778,7 @@ "spiffworkflow": { "editable": true, "git": "https://github.com/sartography/SpiffWorkflow.git", - "ref": "cb098ee6d55b85bf7795997f4ad5f78c27d15381" + "ref": "c8d87826d496af825a184bdc3f0a751e603cfe44" }, "sqlalchemy": { "hashes": [ diff --git a/crc/scripts/request_approval.py b/crc/scripts/request_approval.py index 929ba34b..1df1a670 100644 --- a/crc/scripts/request_approval.py +++ b/crc/scripts/request_approval.py @@ -10,11 +10,11 @@ class RequestApproval(Script): def get_description(self): return """ Creates an approval request on this workflow, by the given approver_uid(s)," -Takes one argument, which should point to data located in current task. +Takes multiple arguments, which should point to data located in current task +or be quoted strings. Example: - -RequestApproval required_approvals.uids +RequestApproval approver1 "dhf8r" """ def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): @@ -29,17 +29,18 @@ RequestApproval required_approvals.uids ApprovalService.add_approval(study_id, workflow_id, id) def get_uids(self, task, args): - if len(args) != 1: + if len(args) < 1: raise ApiError(code="missing_argument", - message="The RequestApproval script requires 1 argument. The " + message="The RequestApproval script requires at least one argument. The " "the name of the variable in the task data that contains user" - "ids to process.") - - uids = task.workflow.script_engine.evaluate_expression(task, args[0]) - - if not isinstance(uids, str) and not isinstance(uids, list): - raise ApiError(code="invalid_argument", - message="The RequestApproval script requires 1 argument. The " + "id to process. Multiple arguments are accepted.") + uids = [] + for arg in args: + id = task.workflow.script_engine.evaluate_expression(task, arg) + uids.append(id) + if not isinstance(id, str): + raise ApiError(code="invalid_argument", + message="The RequestApproval script requires 1 argument. The " "the name of the variable in the task data that contains user" "ids to process. This must point to an array or a string, but " "it currently points to a %s " % uids.__class__.__name__) diff --git a/crc/scripts/update_study.py b/crc/scripts/update_study.py index 303e41b5..ffb9f68e 100644 --- a/crc/scripts/update_study.py +++ b/crc/scripts/update_study.py @@ -6,6 +6,12 @@ from crc.models.study import StudyModel from crc.scripts.script import Script +class mock_study: + def __init__(self): + self.title = "" + self.principle_investigator_id = "" + + class UpdateStudy(Script): argument_error_message = "You must supply at least one argument to the " \ @@ -21,11 +27,15 @@ Example: UpdateStudy title:PIComputingID.label pi:PIComputingID.value """ def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - self.do_task(task, study_id, workflow_id, *args, **kwargs) + study = mock_study + self.__update_study(task, study, *args) def do_task(self, task, study_id, workflow_id, *args, **kwargs): study = db.session.query(StudyModel).filter(StudyModel.id == study_id).first() + self.__update_study(task, study, *args) + db.session.add(study) + def __update_study(self, task, study, *args): if len(args) < 1: raise ApiError.from_task("missing_argument", self.argument_error_message, task=task) @@ -46,4 +56,3 @@ UpdateStudy title:PIComputingID.label pi:PIComputingID.value else: raise ApiError.from_task("invalid_argument", self.argument_error_message, task=task) - db.session.add(study) diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index b7a5bd45..1f6cfe8f 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -61,7 +61,6 @@ class WorkflowService(object): raise ApiError.from_task_spec("workflow_execution_exception", str(we), we.sender) - @staticmethod def populate_form_with_random_data(task, task_api): """populates a task with random data - useful for testing a spec.""" @@ -72,22 +71,42 @@ class WorkflowService(object): for field in task_api.form.fields: if field.type == "enum": if len(field.options) > 0: - form_data[field.id] = random.choice(field.options) + random_choice = random.choice(field.options) + if isinstance(random_choice, dict): + form_data[field.id] = random.choice(field.options)['id'] + else: + # fixme: why it is sometimes an EnumFormFieldOption, and other times not? + form_data[field.id] = random_choice.id ## Assume it is an EnumFormFieldOption else: raise ApiError.from_task("invalid_enum", "You specified an enumeration field (%s)," " with no options" % field.id, task) - if field.type == "autocomplete": + elif field.type == "autocomplete": lookup_model = LookupService.get_lookup_table(task, field) - if not lookup_model: + if field.has_property(Task.PROP_LDAP_LOOKUP): + form_data[field.id] = { + "label": "dhf8r", + "value": "Dan Funk", + "data": { + "uid": "dhf8r", + "display_name": "Dan Funk", + "given_name": "Dan", + "email_address": "dhf8r@virginia.edu", + "department": "Depertment of Psychocosmographictology", + "affiliation": "Rousabout", + "sponsor_type": "Staff" + } + } + elif lookup_model: + data = db.session.query(LookupDataModel).filter( + LookupDataModel.lookup_file_model == lookup_model).limit(10).all() + options = [] + for d in data: + options.append({"id": d.value, "name": d.label}) + form_data[field.id] = random.choice(options) + else: raise ApiError.from_task("invalid_autocomplete", "The settings for this auto complete field " - "(%s) are incorrect: " % field.id) - data = db.session.query(LookupDataModel).filter(LookupDataModel.lookup_file_model == lookup_model).limit(10).all() - options = [] - for d in data: - options.append({"id": d.value, "name": d.label}) - form_data[field.id] = random.choice(options) - + "are incorrect: %s " % field.id, task) elif field.type == "long": form_data[field.id] = random.randint(1, 1000) elif field.type == 'boolean': @@ -102,6 +121,10 @@ class WorkflowService(object): task.data = {} task.data.update(form_data) + def __get_options(self): + pass + + @staticmethod def _random_string(string_length=10): """Generate a random string of fixed length """ diff --git a/tests/test_request_approval_script.py b/tests/test_request_approval_script.py index 8749f711..142da5c5 100644 --- a/tests/test_request_approval_script.py +++ b/tests/test_request_approval_script.py @@ -16,10 +16,10 @@ class TestRequestApprovalScript(BaseTest): workflow = self.create_workflow('empty_workflow') processor = WorkflowProcessor(workflow) task = processor.next_task() - task.data = {"study": {"approvals": ['dhf8r', 'lb3dp']}} + task.data = {"study": {"approval1": "dhf8r", 'approval2':'lb3dp'}} script = RequestApproval() - script.do_task(task, workflow.study_id, workflow.id, "study.approvals") + script.do_task(task, workflow.study_id, workflow.id, "study.approval1", "study.approval2") self.assertEquals(2, db.session.query(ApprovalModel).count()) def test_do_task_with_incorrect_argument(self): @@ -29,7 +29,7 @@ class TestRequestApprovalScript(BaseTest): workflow = self.create_workflow('empty_workflow') processor = WorkflowProcessor(workflow) task = processor.next_task() - task.data = {"approvals": {'dhf8r':"invalid", 'lb3dp':"invalid"}} + task.data = {"approvals": {'dhf8r':["invalid"], 'lb3dp':"invalid"}} script = RequestApproval() with self.assertRaises(ApiError): script.do_task(task, workflow.study_id, workflow.id, "approvals") @@ -40,9 +40,9 @@ class TestRequestApprovalScript(BaseTest): workflow = self.create_workflow('empty_workflow') processor = WorkflowProcessor(workflow) task = processor.next_task() - task.data = {"approvals": ['dhf8r', 'lb3dp']} + task.data = {"study": {"approval1": "dhf8r", 'approval2':'lb3dp'}} script = RequestApproval() - script.do_task_validate_only(task, workflow.study_id, workflow.id, "approvals") + script.do_task_validate_only(task, workflow.study_id, workflow.id, "study.approval1") self.assertEquals(0, db.session.query(ApprovalModel).count())