From b455b73c7b01cc7039c97c4a589ab43d825c6180 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 12 Apr 2022 14:43:34 -0400 Subject: [PATCH 01/11] Don't freak out if there are not files to delete. --- crc/services/git_service.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crc/services/git_service.py b/crc/services/git_service.py index 0feabed8..44a3ee3e 100644 --- a/crc/services/git_service.py +++ b/crc/services/git_service.py @@ -106,9 +106,11 @@ class GitService(object): # get list of untracked files untracked_files = repo.untracked_files - repo.index.add(modified) + if len(modified) > 0: + repo.index.add(modified) repo.index.add(untracked_files) - repo.index.remove(deleted) + if len(deleted) > 0: + repo.index.remove(deleted) repo.index.commit(comment) repo.remotes.origin.push() From 8df20977c3eb1fdf5370837c063b8124c62b95ea Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 12 Apr 2022 15:01:10 -0400 Subject: [PATCH 02/11] Missing a valid user name and password for git. --- config/default.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/default.py b/config/default.py index 04ced891..b04d1791 100644 --- a/config/default.py +++ b/config/default.py @@ -86,9 +86,12 @@ GIT_REMOTE_SERVER = environ.get('GIT_REMOTE_SERVER', None) # example: 'github.c GIT_REMOTE_PATH = environ.get('GIT_REMOTE_PATH', None) # example: 'sartography/crconnect-workflow-specs GIT_BRANCH = environ.get('GIT_BRANCH', None) # example: 'main' GIT_MERGE_BRANCH = environ.get('GIT_MERGE_BRANCH', None) # Example: 'staging' +GIT_USER_NAME = environ.get('GIT_USER_NAME', None) +GIT_USER_PASS = environ.get('GIT_USER_PASS', None) GIT_DISPLAY_PUSH = environ.get('GIT_DISPLAY_PUSH', False) GIT_DISPLAY_MERGE = environ.get('GIT_DISPLAY_MERGE', False) + # Email configuration DEFAULT_SENDER = 'crconnect2@virginia.edu' FALLBACK_EMAILS = ['askresearch@virginia.edu', 'sartographysupport@googlegroups.com'] From 916daf002cf133154bb6096e8c7e0b4b337c63aa Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 13:48:35 -0400 Subject: [PATCH 03/11] Clean up parameters for set_data_common Use key, value instead of args[0], args[1] --- crc/services/data_store_service.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/crc/services/data_store_service.py b/crc/services/data_store_service.py index 31842959..825411e1 100644 --- a/crc/services/data_store_service.py +++ b/crc/services/data_store_service.py @@ -64,23 +64,25 @@ class DataStoreBase(object): study_id, user_id, workflow_id, - workflow_spec_id, script_name, file_id, - *args, - **kwargs): + *args): self.check_args_2(args, script_name=script_name) - if not args[1]: + key = args[0] + if args[1] is None: + value = '' + else: + value = args[1] + if value == '': # We delete the data store if the value is empty - self.delete_data_store(study_id, user_id, file_id, *args) - return - if workflow_spec_id is None and workflow_id is not None: + return self.delete_data_store(study_id, user_id, file_id, *args) + if workflow_id is not None: workflow = session.query(WorkflowModel).filter(WorkflowModel.id == workflow_id).first() workflow_spec_id = workflow.workflow_spec_id # Check if this data store is previously set - query = session.query(DataStoreModel).filter(DataStoreModel.key == args[0]) + query = session.query(DataStoreModel).filter(DataStoreModel.key == key) if study_id: query = query.filter(DataStoreModel.study_id == study_id) if file_id: @@ -90,7 +92,7 @@ class DataStoreBase(object): result = query.order_by(desc(DataStoreModel.last_updated)).all() if result: dsm = result[0] - dsm.value = args[1] + dsm.value = value if task_spec: dsm.task_spec = task_spec if workflow_id: @@ -102,8 +104,8 @@ class DataStoreBase(object): # This just gets rid of all the old unused records self.delete_extra_data_stores(result[1:]) else: - dsm = DataStoreModel(key=args[0], - value=args[1], + dsm = DataStoreModel(key=key, + value=value, study_id=study_id, task_spec=task_spec, user_id=user_id, # Make this available to any User From 721907bf5281ef3c452ca364efc8b93d19a8fb7c Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 13:51:25 -0400 Subject: [PATCH 04/11] Modify test workflow to use new data_store_set script instead of separate study_data_set, user_data_set, and file_data_set scripts --- tests/data/data_store_set/data_store_set.bpmn | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/data/data_store_set/data_store_set.bpmn b/tests/data/data_store_set/data_store_set.bpmn index 16fadd34..da3445ad 100644 --- a/tests/data/data_store_set/data_store_set.bpmn +++ b/tests/data/data_store_set/data_store_set.bpmn @@ -13,13 +13,13 @@ - + - - + + Flow_0cb7y6c @@ -28,10 +28,9 @@ Flow_0cnvihm Flow_10t9bdk - study_data_set(key, value) -user_data_set(key, value) -if file_id: - file_data_set(file_id=file_id, key=key, value=value) + data_store_set(type='study', key=key, value=value) +data_store_set(type='user', key=key, value=value) +data_store_set(type='file', key=key, value=value, file_id=file_id) From e87fae0004ba07aa36655db1cc6cf1b4b2b2648c Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 13:52:20 -0400 Subject: [PATCH 05/11] *** WIP *** Modify delete test to check for both None and empty string --- tests/test_data_store_service.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/test_data_store_service.py b/tests/test_data_store_service.py index 1f5a4b35..3f42285d 100644 --- a/tests/test_data_store_service.py +++ b/tests/test_data_store_service.py @@ -212,11 +212,12 @@ class TestDataStoreValidation(BaseTest): for record in result: self.assertEqual('some_value', record.value) - def test_delete_record_on_empty_value(self): - """If we set a data store with an empty string, + def test_delete_record_on_none_or_empty_string(self): + """If we set a data store with None or an empty string, assert that we delete the record.""" file_id = self.add_test_file() + # Test for empty string form_data = {'key': 'my_key', 'value': 'my_value', 'file_id': file_id} result = self.run_data_store_set(form_data) @@ -233,3 +234,23 @@ class TestDataStoreValidation(BaseTest): result = session.query(DataStoreModel).all() self.assertEqual(0, len(result)) + + # Test for None + form_data = {'key': 'my_second_key', 'value': 'my_second_value', 'file_id': file_id} + result = self.run_data_store_set(form_data) + + self.assertEqual(3, len(result)) + for record in result: + self.assertEqual('my_second_value', record.value) + + workflow = self.create_workflow('data_store_set') + workflow_api = self.get_workflow_api(workflow) + task = workflow_api.next_task + + # The workflow turns the string 'None' into the value None + re_form_data = {'key': 'my_second_key', 'value': 'None', 'file_id': file_id} + self.complete_form(workflow, task, re_form_data) + + result = session.query(DataStoreModel).all() + self.assertEqual(0, len(result)) + From 72f9ac33d06fc42eaada09a356ad575489504770 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 17:57:22 -0400 Subject: [PATCH 06/11] Consolidate data_store scripts --- crc/scripts/data_store_get.py | 75 +++++++++++++++++++++++++++++ crc/scripts/data_store_set.py | 89 +++++++++++++++++++++++++++++++++++ crc/scripts/file_data_get.py | 39 --------------- crc/scripts/file_data_set.py | 68 -------------------------- crc/scripts/study_data_get.py | 17 ------- crc/scripts/study_data_set.py | 28 ----------- crc/scripts/user_data_get.py | 19 -------- crc/scripts/user_data_set.py | 31 ------------ 8 files changed, 164 insertions(+), 202 deletions(-) create mode 100644 crc/scripts/data_store_get.py create mode 100644 crc/scripts/data_store_set.py delete mode 100644 crc/scripts/file_data_get.py delete mode 100644 crc/scripts/file_data_set.py delete mode 100644 crc/scripts/study_data_get.py delete mode 100644 crc/scripts/study_data_set.py delete mode 100644 crc/scripts/user_data_get.py delete mode 100644 crc/scripts/user_data_set.py diff --git a/crc/scripts/data_store_get.py b/crc/scripts/data_store_get.py new file mode 100644 index 00000000..b36babfa --- /dev/null +++ b/crc/scripts/data_store_get.py @@ -0,0 +1,75 @@ +from crc.api.common import ApiError +from crc.scripts.script import Script +from crc.services.data_store_service import DataStoreBase + +from flask import g + + +class ScriptTemplate(Script): + script_name = None + data_store_args = None + data_store_type = None + data_store_file_id = None + data_store_study_id = None + data_store_user_id = None + + def set_args(self, study_id, **kwargs): + self.validate_kw_args(**kwargs) + self.data_store_args = [kwargs['key']] + if 'default' in kwargs.keys(): + self.data_store_args.append(kwargs['default']) + self.data_store_type = kwargs['type'] + if self.data_store_type == 'file': + try: + file_id = int(kwargs['file_id']) + except Exception: + raise ApiError("invalid_file_id", + f"The file_id must be an integer. You passed {kwargs['file_id']}.") + self.script_name = 'file_data_get' + self.data_store_file_id = file_id + self.data_store_study_id = None + self.data_store_user_id = None + elif self.data_store_type == 'study': + self.script_name = 'study_data_get' + self.data_store_study_id = study_id + self.data_store_file_id = None + self.data_store_user_id = None + elif self.data_store_type == 'user': + self.script_name = 'user_data_get' + self.data_store_user_id = g.user.uid + self.data_store_file_id = None + self.data_store_study_id = None + + def get_description(self): + return """Returns a value from the data store. Requires 2 keyword arguments; `type` and `key`. + Type is one of `file`, `study`, or `user`. + Key is the key of the record you want returned. + If type is `file`, then the script expects a third keyword argument of `file_id`.""" + + def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): + self.set_args(study_id, **kwargs) + result = DataStoreBase().get_validate_common(self.script_name, + self.data_store_study_id, + self.data_store_user_id, + self.data_store_file_id, + *self.data_store_args) + return result + + def do_task(self, task, study_id, workflow_id, *args, **kwargs): + self.set_args(study_id, **kwargs) + result = DataStoreBase().get_data_common(self.data_store_study_id, + self.data_store_user_id, + self.script_name, + self.data_store_file_id, + *self.data_store_args) + return result + + @staticmethod + def validate_kw_args(**kwargs): + if 'type' not in kwargs or 'key' not in kwargs: + raise ApiError(code='missing_arguments', + message='The data_store_set script requires 2 keyword arguments; `type` and `key`.') + if kwargs['type'] == 'file' and 'file_id' not in kwargs: + raise ApiError(code='missing_arguments', + message='If `type` is `file`, the data_store_set script requires a third keyword argument of `file_id`.') + return True diff --git a/crc/scripts/data_store_set.py b/crc/scripts/data_store_set.py new file mode 100644 index 00000000..38013f90 --- /dev/null +++ b/crc/scripts/data_store_set.py @@ -0,0 +1,89 @@ +from crc.api.common import ApiError +from crc.scripts.script import Script +from crc.services.data_store_service import DataStoreBase +from crc.services.document_service import DocumentService +from crc.services.user_file_service import UserFileService + +from flask import g + + +class DataStoreSet(Script): + script_name = None + data_store_args = None + data_store_type = None + data_store_file_id = None + data_store_study_id = None + data_store_user_id = None + + def set_args(self, study_id, **kwargs): + self.validate_kw_args(**kwargs) + self.data_store_args = [kwargs['key'], kwargs['value']] + self.data_store_type = kwargs['type'] + if self.data_store_type == 'file': + try: + file_id = int(kwargs['file_id']) + except Exception: + raise ApiError("invalid_file_id", + f"The file_id must be an integer. You passed {kwargs['file_id']}.") + self.script_name = 'file_data_set' + self.data_store_file_id = file_id + self.data_store_study_id = None + self.data_store_user_id = None + elif self.data_store_type == 'study': + self.script_name = 'study_data_set' + self.data_store_study_id = study_id + self.data_store_file_id = None + self.data_store_user_id = None + elif self.data_store_type == 'user': + self.script_name = 'user_data_set' + self.data_store_user_id = g.user.uid + self.data_store_file_id = None + self.data_store_study_id = None + + def get_description(self): + return """Sets a data store. Takes 3 mandatory keyword arguments; `type`, `key`, and `value`. + Type is one of `file`, `study`, or `user`. + Key and value are defined by the user. + If type is `file`, then the script expects a fourth keyword argument of `file_id`.""" + + def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): + self.set_args(study_id, **kwargs) + result = DataStoreBase().set_validate_common(task.id, + self.data_store_study_id, + workflow_id, + self.script_name, + self.data_store_user_id, + self.data_store_file_id, + *self.data_store_args) + return result + + def do_task(self, task, study_id, workflow_id, *args, **kwargs): + self.set_args(study_id, **kwargs) + + if self.data_store_type == 'file' and self.data_store_args[0] == 'irb_code': + irb_doc_code = kwargs['value'] + UserFileService.update_irb_code(self.data_store_file_id, irb_doc_code) + + return DataStoreBase().set_data_common(task.id, + self.data_store_study_id, + self.data_store_user_id, + workflow_id, + self.script_name, + self.data_store_file_id, + *self.data_store_args) + + @staticmethod + def validate_kw_args(**kwargs): + if 'type' not in kwargs or 'key' not in kwargs or 'value' not in kwargs: + raise ApiError(code='missing_arguments', + message='The data_store_set script requires 3 keyword arguments; `type`, `key`, and `value`.') + if kwargs['type'] == 'file' and 'file_id' not in kwargs: + raise ApiError(code='missing_arguments', + message='If `type` is `file`, the data_store_set script requires a fourth keyword argument of `file_id`.') + if kwargs['type'] == 'file' \ + and kwargs['key'] == 'irb_code' \ + and not DocumentService.is_allowed_document(kwargs.get('value')): + raise ApiError(code="invalid_form_field_key", + message="When setting an irb_code, the value must be a valid document code. " + f"The value {kwargs.get('value')} is not a valid document code.") + return True diff --git a/crc/scripts/file_data_get.py b/crc/scripts/file_data_get.py deleted file mode 100644 index 2396ea59..00000000 --- a/crc/scripts/file_data_get.py +++ /dev/null @@ -1,39 +0,0 @@ -from crc.api.common import ApiError -from crc.services.data_store_service import DataStoreBase -from crc.scripts.script import Script - - -class FileDataGet(Script, DataStoreBase): - def get_description(self): - return """Gets user data from the data store - takes two keyword arguments arguments: 'file_id' and 'key' """ - - def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - self.validate_kw_args(**kwargs) - my_args = [kwargs['key']] - if 'default' in kwargs.keys(): - my_args.append(kwargs['default']) - result = self.get_validate_common('file_data_get', None, None, kwargs['file_id'], *my_args) - return result - - @staticmethod - def validate_kw_args(**kwargs): - if kwargs.get('key', None) is None: - raise ApiError(code="missing_argument", - message=f"The 'file_data_get' script requires a keyword argument of 'key'") - - if kwargs.get('file_id', None) is None: - raise ApiError(code="missing_argument", - message=f"The 'file_data_get' script requires a keyword argument of 'file_id'") - return True - - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - self.validate_kw_args(**kwargs) - my_args = [kwargs['key']] - if 'default' in kwargs.keys(): - my_args.append(kwargs['default']) - - return self.get_data_common(None, - None, - 'file_data_get', - kwargs['file_id'], - *my_args) diff --git a/crc/scripts/file_data_set.py b/crc/scripts/file_data_set.py deleted file mode 100644 index b3a2b8f3..00000000 --- a/crc/scripts/file_data_set.py +++ /dev/null @@ -1,68 +0,0 @@ -from crc.api.common import ApiError -from crc.services.data_store_service import DataStoreBase -from crc.scripts.script import Script -from crc.services.document_service import DocumentService -from crc.services.user_file_service import UserFileService - - -class FileDataSet(Script, DataStoreBase): - def get_description(self): - return """Sets data the data store - takes three keyword arguments arguments: 'file_id', 'key' and 'value'""" - - def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - self.validate_kw_args(**kwargs) - my_args = [kwargs['key'], kwargs['value']] - file_id = kwargs['file_id'] - result = self.set_validate_common(task.id, - study_id, - workflow_id, - 'file_data_set', - None, - file_id, - *my_args) - return result - - @staticmethod - def validate_kw_args(**kwargs): - if kwargs.get('key', None) is None: - raise ApiError(code="missing_argument", - message=f"The 'file_data_get' script requires a keyword argument of 'key'") - if kwargs.get('file_id', None) is None: - raise ApiError(code="missing_argument", - message=f"The 'file_data_get' script requires a keyword argument of 'file_id'") - if kwargs.get('value', None) is None: - raise ApiError(code="missing_argument", - message=f"The 'file_data_get' script requires a keyword argument of 'value'") - - if kwargs['key'] == 'irb_code' and not DocumentService.is_allowed_document(kwargs.get('value')): - raise ApiError("invalid_form_field_key", - "When setting an irb_code, the form field id must match a known document in the " - "irb_documents.xlsx reference file. This code is not found in that file '%s'" % - kwargs.get('value')) - - return True - - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - self.validate_kw_args(**kwargs) - my_args = [kwargs['key'], kwargs['value']] - - try: - fileid = int(kwargs['file_id']) - except Exception: - raise ApiError("invalid_file_id", - "Attempting to update DataStore for an invalid file_id '%s'" % kwargs['file_id']) - - del(kwargs['file_id']) - if kwargs['key'] == 'irb_code': - irb_doc_code = kwargs['value'] - UserFileService.update_irb_code(fileid, irb_doc_code) - - return self.set_data_common(task.id, - None, - None, - workflow_id, - None, - 'file_data_set', - fileid, - *my_args, - **kwargs) diff --git a/crc/scripts/study_data_get.py b/crc/scripts/study_data_get.py deleted file mode 100644 index a4803964..00000000 --- a/crc/scripts/study_data_get.py +++ /dev/null @@ -1,17 +0,0 @@ -from crc.services.data_store_service import DataStoreBase -from crc.scripts.script import Script - - -class StudyDataGet(Script, DataStoreBase): - def get_description(self): - return """Gets study data from the data store.""" - - def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - return self.get_validate_common('study_data_get', study_id, None, None, *args) - - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - return self.get_data_common(study_id, - None, - 'study_data_get', - None, - *args) diff --git a/crc/scripts/study_data_set.py b/crc/scripts/study_data_set.py deleted file mode 100644 index 81647dac..00000000 --- a/crc/scripts/study_data_set.py +++ /dev/null @@ -1,28 +0,0 @@ -from crc.services.data_store_service import DataStoreBase -from crc.scripts.script import Script - - -class StudyDataSet(Script, DataStoreBase): - def get_description(self): - return """Sets study data from the data store. Takes two positional arguments key and value""" - - def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - result = self.set_validate_common(task.id, - study_id, - workflow_id, - 'study_data_set', - None, - None, - *args) - return result - - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - return self.set_data_common(task.id, - study_id, - None, - workflow_id, - None, - 'study_data_set', - None, - *args, - **kwargs) diff --git a/crc/scripts/user_data_get.py b/crc/scripts/user_data_get.py deleted file mode 100644 index 4e7572fc..00000000 --- a/crc/scripts/user_data_get.py +++ /dev/null @@ -1,19 +0,0 @@ -from flask import g - -from crc.services.data_store_service import DataStoreBase -from crc.scripts.script import Script - - -class UserDataGet(Script, DataStoreBase): - def get_description(self): - return """Gets user data from the data store - takes only one argument 'key' """ - - def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - return self.get_validate_common('user_data_get', None, g.user.uid, None, *args) - - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - return self.get_data_common(None, - g.user.uid, - 'user_data_get', - None, - *args) diff --git a/crc/scripts/user_data_set.py b/crc/scripts/user_data_set.py deleted file mode 100644 index a58d05dd..00000000 --- a/crc/scripts/user_data_set.py +++ /dev/null @@ -1,31 +0,0 @@ -from flask import g - -from crc.services.data_store_service import DataStoreBase -from crc.scripts.script import Script - - -class UserDataSet(Script, DataStoreBase): - def get_description(self): - return """Sets user data to the data store these are positional arguments key and value. - example: user_data_set('my_key','my_value') - """ - - def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): - self.set_validate_common(task.id, - study_id, - workflow_id, - 'user_data_set', - g.user.uid, - None, - *args) - - def do_task(self, task, study_id, workflow_id, *args, **kwargs): - return self.set_data_common(task.id, - None, - g.user.uid, - workflow_id, - None, - 'user_data_set', - None, - *args, - **kwargs) From 076eb8747fbaa73bf5f7fe0e4c522413d4ecbce0 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 18:01:52 -0400 Subject: [PATCH 07/11] Finish bug fix for deleting data_stores when value is None or empty string Cleanup: - Make sure workflow_spec_id has a value - Change extra if conditionals to elif conditionals --- crc/services/data_store_service.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crc/services/data_store_service.py b/crc/services/data_store_service.py index 825411e1..249065fa 100644 --- a/crc/services/data_store_service.py +++ b/crc/services/data_store_service.py @@ -70,13 +70,11 @@ class DataStoreBase(object): self.check_args_2(args, script_name=script_name) key = args[0] - if args[1] is None: - value = '' - else: - value = args[1] - if value == '': + value = args[1] + if value == '' or value is None: # We delete the data store if the value is empty return self.delete_data_store(study_id, user_id, file_id, *args) + workflow_spec_id = None if workflow_id is not None: workflow = session.query(WorkflowModel).filter(WorkflowModel.id == workflow_id).first() workflow_spec_id = workflow.workflow_spec_id @@ -85,9 +83,9 @@ class DataStoreBase(object): query = session.query(DataStoreModel).filter(DataStoreModel.key == key) if study_id: query = query.filter(DataStoreModel.study_id == study_id) - if file_id: + elif file_id: query = query.filter(DataStoreModel.file_id == file_id) - if user_id: + elif user_id: query = query.filter(DataStoreModel.user_id == user_id) result = query.order_by(desc(DataStoreModel.last_updated)).all() if result: From be24b598b89b138eda6574d90d0177c421bd4e84 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 18:03:23 -0400 Subject: [PATCH 08/11] Fix call to set_data_common - no longer have workflow_spec_id, add type of data set --- crc/services/workflow_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index 13ce9f11..cebd163e 100755 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -470,7 +470,7 @@ class WorkflowService(object): field.id in data and data[field.id]: file_id = data[field.get_property(Task.FIELD_PROP_FILE_DATA)]["id"] data_args = (field.id, data[field.id]) - DataStoreBase().set_data_common(task.id, None, None, None, None, None, file_id, *data_args) + DataStoreBase().set_data_common(task.id, None, None, None, 'file_data_set', file_id, *data_args) @staticmethod def evaluate_property(property_name, field, task, task_data=None): From 19da7ecd7e2eef1b97cdf3e36d5b2475d928109f Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 18:07:13 -0400 Subject: [PATCH 09/11] Fix test workflows to use new data_store scripts --- .../data_store_set_2/data_store_set_2.bpmn | 17 +++-- .../data_store_validation.bpmn | 26 ++++--- .../data/file_data_store/file_data_store.bpmn | 40 +++++------ .../study_associates_validation.bpmn | 2 +- .../study_sponsors_data_store.bpmn | 68 +++++++++---------- tests/study/test_study_details_documents.py | 10 +-- 6 files changed, 80 insertions(+), 83 deletions(-) diff --git a/tests/data/data_store_set_2/data_store_set_2.bpmn b/tests/data/data_store_set_2/data_store_set_2.bpmn index d4a6597c..5788a658 100644 --- a/tests/data/data_store_set_2/data_store_set_2.bpmn +++ b/tests/data/data_store_set_2/data_store_set_2.bpmn @@ -8,10 +8,9 @@ Flow_1er0zqt Flow_1vocvzo - study_data_set(key, value) -user_data_set(key, value) -if file_id: - file_data_set(file_id=file_id, key=key, value=value) + data_store_set(type='study', key=key, value=value) +data_store_set(type='user', key=key, value=value) +data_store_set(type='file', key=key, value=value, file_id=file_id) Flow_1vocvzo @@ -38,10 +37,10 @@ if file_id: - + - + @@ -51,15 +50,15 @@ if file_id: - - - + + + diff --git a/tests/data/data_store_validation/data_store_validation.bpmn b/tests/data/data_store_validation/data_store_validation.bpmn index ca4783e2..0416c620 100644 --- a/tests/data/data_store_validation/data_store_validation.bpmn +++ b/tests/data/data_store_validation/data_store_validation.bpmn @@ -8,27 +8,25 @@ Flow_0nstzm8 Flow_08r3ga0 - study_data_set('study_data_key', 'study_data_value') -file_data_set(file_id=1, key='file_data_key', value='file_data_value') -user_data_set('user_data_key', 'user_data_value') + data_store_set(type='study', key='study_data_key', value='study_data_value') +data_store_set(type='user', key='user_data_key', value='user_data_value') +data_store_set(type='file', key='file_data_key', value='file_data_value', file_id=1) Flow_08r3ga0 Flow_02l0u2v - -previous_study_data_value = study_data_get('previous_study_data_key') -previous_file_data_value = file_data_get(file_id=1, key='previous_file_data_key') -previous_user_data_value = user_data_get('previous_user_data_key') + previous_study_data_value = data_store_get(type='study', key='previous_study_data_key') +previous_file_data_value = data_store_get(type='file', file_id=1, key='previous_file_data_key') +previous_user_data_value = data_store_get(type='user', key='previous_user_data_key') -study_data_value = study_data_get('study_data_key') -file_data_value = file_data_get(file_id=1, key='file_data_key') -user_data_value = user_data_get('user_data_key') +study_data_value = data_store_get(type='study', key='study_data_key') +file_data_value = data_store_get(type='file', file_id=1, key='file_data_key') +user_data_value = data_store_get(type='user', key='user_data_key') -bad_study_data_value = study_data_get('bad_study_data_key', 'bad_study_data_value') -bad_file_data_value = file_data_get(file_id=1, key='bad_file_data_key', default='bad_file_data_value') -bad_user_data_value = user_data_get('bad_user_data_key', 'bad_user_data_value') - +bad_study_data_value = data_store_get(type='study', key='bad_study_data_key', default='bad_study_data_value') +bad_file_data_value = data_store_get(type='file', file_id=1, key='bad_file_data_key', default='bad_file_data_value') +bad_user_data_value = data_store_get(type='user', key='bad_user_data_key', default='bad_user_data_value') diff --git a/tests/data/file_data_store/file_data_store.bpmn b/tests/data/file_data_store/file_data_store.bpmn index 91939c17..de8b569c 100644 --- a/tests/data/file_data_store/file_data_store.bpmn +++ b/tests/data/file_data_store/file_data_store.bpmn @@ -1,5 +1,5 @@ - + SequenceFlow_1pnq3kg @@ -22,7 +22,7 @@ fileurl = documents['UVACompl_PRCAppr'].files[0]['url'] filename = documents['UVACompl_PRCAppr'].files[0]['name'] -file_data_set(file_id=fileid,key='test',value='me') +data_store_set(type='file', file_id=fileid, key='test', value='me') Flow_02bgcrp @@ -32,54 +32,54 @@ file_data_set(file_id=fileid,key='test',value='me') Flow_0z7kamo Flow_15mmymi - output=file_data_get(file_id=fileid,key='test') + output=data_store_get(type='file', file_id=fileid, key='test') Flow_15mmymi Flow_02bgcrp - output2=file_data_get(file_id=fileid,key='unobtainium',default='nope') + output2=data_store_get(type='file', file_id=fileid, key='unobtainium', default='nope') - + - - + + - - + + - - + + - + - - - - - - + + + + + + - + - + diff --git a/tests/data/study_associates_validation/study_associates_validation.bpmn b/tests/data/study_associates_validation/study_associates_validation.bpmn index 54dff5c9..3663328f 100644 --- a/tests/data/study_associates_validation/study_associates_validation.bpmn +++ b/tests/data/study_associates_validation/study_associates_validation.bpmn @@ -32,7 +32,7 @@ for assoc in assoc_list: assoc_info = assoc assoc_status = "Found" assoc_cid = assoc_info["uid"] - if assoc_cid != study_data_get("sdsPI_ComputingID", False): + if assoc_cid != data_store_get(type='study', key="sdsPI_ComputingID", value=False): assoc_status = "New" diff --git a/tests/data/study_sponsors_data_store/study_sponsors_data_store.bpmn b/tests/data/study_sponsors_data_store/study_sponsors_data_store.bpmn index 48ecf5b6..8a229853 100644 --- a/tests/data/study_sponsors_data_store/study_sponsors_data_store.bpmn +++ b/tests/data/study_sponsors_data_store/study_sponsors_data_store.bpmn @@ -1,5 +1,5 @@ - + SequenceFlow_1nfe5m9 @@ -18,24 +18,24 @@ SequenceFlow_1bqiin0 Flow_09cika8 - study_data_set('testme','newval') + data_store_set(type='study', key='testme', value='newval') Flow_09cika8 Flow_1oeqjuy - out = study_data_get('testme','bogus') + out = data_store_get(type='study', key='testme', default='bogus') Flow_1oeqjuy Flow_0g9waf3 - study_data_set('testme','badval') + data_store_set(type='study', key='testme', value='badval') - + Flow_0g9waf3 Flow_05136ua - empty = user_data_get('testme','empty') + empty = data_store_get(type='user', key='testme', default='empty') @@ -44,7 +44,7 @@ Flow_00s638e - user_data_get('nothing','default') == 'default' + data_store_get(type='user', key='nothing', default='default') == 'default' Flow_00s638e @@ -53,6 +53,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -70,51 +97,24 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/study/test_study_details_documents.py b/tests/study/test_study_details_documents.py index 359c0f16..583b8f11 100644 --- a/tests/study/test_study_details_documents.py +++ b/tests/study/test_study_details_documents.py @@ -12,7 +12,7 @@ from crc.models.study import StudyModel from crc.scripts.study_info import StudyInfo from crc.services.study_service import StudyService from crc.services.workflow_processor import WorkflowProcessor -from crc.scripts.file_data_set import FileDataSet +from crc.scripts.data_store_set import DataStoreSet from crc.services.document_service import DocumentService from crc.services.user_file_service import UserFileService from crc.services.reference_file_service import ReferenceFileService @@ -104,7 +104,7 @@ class TestStudyDetailsDocumentsScript(BaseTest): binary_data=b'1234', irb_doc_code=irb_code) processor = WorkflowProcessor(workflow_model) task = processor.next_task() - FileDataSet().do_task(task, study.id, workflow_model.id, key="ginger", value="doodle", file_id=file.id) + DataStoreSet().do_task(task, study.id, workflow_model.id, type='file', key="ginger", value="doodle", file_id=file.id) docs = StudyInfo().do_task(task, study.id, workflow_model.id, "documents") self.assertTrue(isinstance(docs, Box)) docs = StudyService.get_documents_status(study.id, force=True) @@ -127,7 +127,7 @@ class TestStudyDetailsDocumentsScript(BaseTest): binary_data=b'1234', irb_doc_code=irb_code) processor = WorkflowProcessor(workflow_model) task = processor.next_task() - FileDataSet().do_task(task, study.id, workflow_model.id, key="irb_code", value="Study_App_Doc", file_id=file.id) + DataStoreSet().do_task(task, study.id, workflow_model.id, type='file', key="irb_code", value="Study_App_Doc", file_id=file.id) docs = StudyInfo().do_task(task, study.id, workflow_model.id, "documents") self.assertTrue(isinstance(docs, Box)) self.assertEqual(1, len(docs.Study_App_Doc.files)) @@ -151,5 +151,5 @@ class TestStudyDetailsDocumentsScript(BaseTest): processor = WorkflowProcessor(workflow_model) task = processor.next_task() with self.assertRaises(ApiError): - FileDataSet().do_task(task, study.id, workflow_model.id, key="irb_code", value="My_Pretty_Pony", - file_id=file.id) + DataStoreSet().do_task(task, study.id, workflow_model.id, type='file', + key="irb_code", value="My_Pretty_Pony", file_id=file.id) From ea657cbbc2b12faae1ee695c97edb600a39340f5 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 18:09:18 -0400 Subject: [PATCH 10/11] Change value from string 'None' to value None to test deleting data_stores. This as a work around, because the form field is set to string for other uses --- tests/data/data_store_set/data_store_set.bpmn | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/data/data_store_set/data_store_set.bpmn b/tests/data/data_store_set/data_store_set.bpmn index da3445ad..7f592b8b 100644 --- a/tests/data/data_store_set/data_store_set.bpmn +++ b/tests/data/data_store_set/data_store_set.bpmn @@ -28,10 +28,11 @@ Flow_0cnvihm Flow_10t9bdk - data_store_set(type='study', key=key, value=value) + if value == 'None': + value = None +data_store_set(type='study', key=key, value=value) data_store_set(type='user', key=key, value=value) -data_store_set(type='file', key=key, value=value, file_id=file_id) - +data_store_set(type='file', key=key, value=value, file_id=file_id) From 6c1fcc5cee04667692586face186f5d4abe8001d Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Apr 2022 18:10:35 -0400 Subject: [PATCH 11/11] Make sure we don't delete records when value is set to False --- tests/test_data_store_service.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_data_store_service.py b/tests/test_data_store_service.py index 3f42285d..1cbdc814 100644 --- a/tests/test_data_store_service.py +++ b/tests/test_data_store_service.py @@ -254,3 +254,24 @@ class TestDataStoreValidation(BaseTest): result = session.query(DataStoreModel).all() self.assertEqual(0, len(result)) + def test_do_not_delete_record_on_false_value(self): + file_id = self.add_test_file() + + form_data = {'key': 'my_key', 'value': 'my_value', 'file_id': file_id} + result = self.run_data_store_set(form_data) + + self.assertEqual(3, len(result)) + for record in result: + self.assertEqual('my_value', record.value) + + workflow = self.create_workflow('data_store_set') + workflow_api = self.get_workflow_api(workflow) + task = workflow_api.next_task + + re_form_data = {'key': 'my_key', 'value': False, 'file_id': file_id} + self.complete_form(workflow, task, re_form_data) + + result = session.query(DataStoreModel).all() + self.assertEqual(3, len(result)) + for record in result: + self.assertEqual('false', record.value)