diff --git a/config/default.py b/config/default.py index 289c506f..269e2cb3 100644 --- a/config/default.py +++ b/config/default.py @@ -42,6 +42,6 @@ PB_REQUIRED_DOCS_URL = environ.get('PB_REQUIRED_DOCS_URL', default=PB_BASE_URL + PB_STUDY_DETAILS_URL = environ.get('PB_STUDY_DETAILS_URL', default=PB_BASE_URL + "study?studyid=%i") LDAP_URL = environ.get('LDAP_URL', default="ldap.virginia.edu").strip('/') # No trailing slash or http:// -LDAP_TIMEOUT_SEC = int(environ.get('LDAP_TIMEOUT_SEC', default=3)) +LDAP_TIMEOUT_SEC = int(environ.get('LDAP_TIMEOUT_SEC', default=1)) diff --git a/crc/__init__.py b/crc/__init__.py index e705321b..309e3f19 100644 --- a/crc/__init__.py +++ b/crc/__init__.py @@ -73,3 +73,9 @@ def load_example_rrt_data(): from example_data import ExampleDataLoader ExampleDataLoader.clean_db() ExampleDataLoader().load_rrt() + +@app.cli.command() +def clear_db(): + """Load example data into the database.""" + from example_data import ExampleDataLoader + ExampleDataLoader.clean_db() diff --git a/crc/api.yml b/crc/api.yml index b4e45a95..638eb787 100644 --- a/crc/api.yml +++ b/crc/api.yml @@ -808,12 +808,12 @@ paths: $ref: "#/components/schemas/Script" /approval: parameters: - - name: everything + - name: status in: query required: false - description: If set to true, returns all the approvals known to the system. + description: If set to true, returns all the approvals with any status. schema: - type: boolean + type: string - name: as_user in: query required: false diff --git a/crc/api/approval.py b/crc/api/approval.py index 9cd0399c..b23315df 100644 --- a/crc/api/approval.py +++ b/crc/api/approval.py @@ -13,13 +13,13 @@ from crc.services.approval_service import ApprovalService from crc.services.ldap_service import LdapService -def get_approvals(everything=False, as_user=None): - if everything: - approvals = ApprovalService.get_all_approvals(include_cancelled=True) - elif as_user: - approvals = ApprovalService.get_all_approvals(as_user, include_cancelled=False) - else: - approvals = ApprovalService.get_approvals_per_user(g.user.uid, include_cancelled=False) +def get_approvals(status=None, as_user=None): + #status = ApprovalStatus.PENDING.value + user = g.user.uid + if as_user: + user = as_user + approvals = ApprovalService.get_approvals_per_user(user, status, + include_cancelled=False) results = ApprovalSchema(many=True).dump(approvals) return results diff --git a/crc/models/approval.py b/crc/models/approval.py index c60bef1f..0592fbd1 100644 --- a/crc/models/approval.py +++ b/crc/models/approval.py @@ -84,13 +84,13 @@ class Approval(object): instance.associated_files = [] for approval_file in model.approval_files: try: + # fixme: This is slow because we are doing a ton of queries to find the irb code. extra_info = doc_dictionary[approval_file.file_data.file_model.irb_doc_code] except: extra_info = None associated_file = {} associated_file['id'] = approval_file.file_data.file_model.id if extra_info: - irb_doc_code = approval_file.file_data.file_model.irb_doc_code associated_file['name'] = '_'.join((extra_info['category1'], approval_file.file_data.file_model.name)) associated_file['description'] = extra_info['description'] diff --git a/crc/services/approval_service.py b/crc/services/approval_service.py index d05d4e16..5022d1f0 100644 --- a/crc/services/approval_service.py +++ b/crc/services/approval_service.py @@ -9,54 +9,52 @@ from crc.models.approval import ApprovalModel, ApprovalStatus, ApprovalFile, App from crc.models.study import StudyModel from crc.models.workflow import WorkflowModel from crc.services.file_service import FileService -from crc.services.ldap_service import LdapService - class ApprovalService(object): """Provides common tools for working with an Approval""" @staticmethod - def __one_approval_from_study(study, approver_uid = None, include_cancelled=True): + def __one_approval_from_study(study, approver_uid = None, status=None, + include_cancelled=True): """Returns one approval, with all additional approvals as 'related_approvals', the main approval can be pinned to an approver with an optional argument. Will return null if no approvals exist on the study.""" main_approval = None related_approvals = [] - query = db.session.query(ApprovalModel).\ - filter(ApprovalModel.study_id == study.id) + query = db.session.query(ApprovalModel).filter(ApprovalModel.study_id == study.id) if not include_cancelled: query=query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value) - approvals = query.all() # All non-cancelled approvals. - study_approval_status = "" - # IF THIS IS RELATED TO THE CURRENT USER for approval_model in approvals: if approval_model.approver_uid == approver_uid: - main_approval = Approval.from_model(approval_model) + main_approval = approval_model else: - related_approvals.append(Approval.from_model(approval_model)) + related_approvals.append(approval_model) # IF WE ARE JUST RETURNING ALL OF THE APPROVALS PER STUDY if not main_approval and len(related_approvals) > 0: main_approval = related_approvals[0] related_approvals = related_approvals[1:] - if len(related_approvals) > 0: - main_approval.related_approvals = related_approvals - if main_approval is not None: # May be null if the study has no approvals. - main_approval.status = ApprovalService.__calculate_overall_approval_status(main_approval) + final_status = ApprovalService.__calculate_overall_approval_status(main_approval, related_approvals) + if status and final_status != status: return # Now that we are certain of the status, filter on it. + + main_approval = Approval.from_model(main_approval) + main_approval.status = final_status + for ra in related_approvals: + main_approval.related_approvals.append(Approval.from_model(ra)) return main_approval @staticmethod - def __calculate_overall_approval_status(approval): + def __calculate_overall_approval_status(approval, related): # In the case of pending approvals, check to see if there is a related approval # that proceeds this approval - and if it is declined, or still pending, then change # the state of the approval to be Declined, or Waiting respectively. if approval.status == ApprovalStatus.PENDING.value: - for ra in approval.related_approvals: + for ra in related: if ra.id < approval.id: if ra.status == ApprovalStatus.DECLINED.value or ra.status == ApprovalStatus.CANCELED.value: return ra.status # If any prior approval id declined or cancelled so is this approval. @@ -67,14 +65,15 @@ class ApprovalService(object): return approval.status @staticmethod - def get_approvals_per_user(approver_uid, include_cancelled=False): + def get_approvals_per_user(approver_uid, status=None, include_cancelled=False): """Returns a list of approval objects (not db models) for the given approver. """ studies = db.session.query(StudyModel).join(ApprovalModel).\ filter(ApprovalModel.approver_uid == approver_uid).all() approvals = [] for study in studies: - approval = ApprovalService.__one_approval_from_study(study, approver_uid, include_cancelled) + approval = ApprovalService.__one_approval_from_study(study, approver_uid, + status, include_cancelled) if approval: approvals.append(approval) return approvals diff --git a/crc/services/ldap_service.py b/crc/services/ldap_service.py index 61097921..fea99af0 100644 --- a/crc/services/ldap_service.py +++ b/crc/services/ldap_service.py @@ -18,7 +18,7 @@ class LdapService(object): user_or_last_name_search = "(&(objectclass=person)(|(uid=%s*)(sn=%s*)))" cn_single_search = '(&(objectclass=person)(cn=%s*))' cn_double_search = '(&(objectclass=person)(&(cn=%s*)(cn=*%s*)))' - + temp_cache = {} conn = None @staticmethod @@ -43,6 +43,7 @@ class LdapService(object): def user_info(uva_uid): user_info = db.session.query(LdapModel).filter(LdapModel.uid == uva_uid).first() if not user_info: + app.logger.info("No cache for " + uva_uid) search_string = LdapService.uid_search_string % uva_uid conn = LdapService.__get_conn() conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes) @@ -51,6 +52,7 @@ class LdapService(object): entry = conn.entries[0] user_info = LdapModel.from_entry(entry) db.session.add(user_info) + db.session.commit() return user_info @staticmethod diff --git a/tests/test_approvals_api.py b/tests/test_approvals_api.py index 6d95be39..b80eaef0 100644 --- a/tests/test_approvals_api.py +++ b/tests/test_approvals_api.py @@ -68,16 +68,15 @@ class TestApprovals(BaseTest): approval = response[0] self.assertEqual(approval['approver']['uid'], approver_uid) - def test_list_approvals_per_admin(self): - """All approvals will be returned""" - rv = self.app.get('/v1.0/approval?everything=true', headers=self.logged_in_headers()) + def test_list_approvals_as_user(self): + """All approvals as different user""" + rv = self.app.get('/v1.0/approval?as_user=lb3dp', headers=self.logged_in_headers()) self.assert_success(rv) response = json.loads(rv.get_data(as_text=True)) - # Returned approvals should match what's in the db, we should get one approval back - # per study (2 studies), and that approval should have one related approval. - approvals_count = ApprovalModel.query.count() + # Returned approvals should match what's in the db for user ld3dp, we should get one + # approval back per study (2 studies), and that approval should have one related approval. response_count = len(response) self.assertEqual(2, response_count)