Allow setting the type of approvals you want back, by status.

Some very minor performance enhancements, that will add up on the Approvers page.
This commit is contained in:
Dan Funk 2020-06-05 17:49:55 -04:00
parent f0904e75a6
commit 6861991d8f
8 changed files with 43 additions and 37 deletions

View File

@ -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") 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_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))

View File

@ -73,3 +73,9 @@ def load_example_rrt_data():
from example_data import ExampleDataLoader from example_data import ExampleDataLoader
ExampleDataLoader.clean_db() ExampleDataLoader.clean_db()
ExampleDataLoader().load_rrt() ExampleDataLoader().load_rrt()
@app.cli.command()
def clear_db():
"""Load example data into the database."""
from example_data import ExampleDataLoader
ExampleDataLoader.clean_db()

View File

@ -808,12 +808,12 @@ paths:
$ref: "#/components/schemas/Script" $ref: "#/components/schemas/Script"
/approval: /approval:
parameters: parameters:
- name: everything - name: status
in: query in: query
required: false 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: schema:
type: boolean type: string
- name: as_user - name: as_user
in: query in: query
required: false required: false

View File

@ -13,13 +13,13 @@ from crc.services.approval_service import ApprovalService
from crc.services.ldap_service import LdapService from crc.services.ldap_service import LdapService
def get_approvals(everything=False, as_user=None): def get_approvals(status=None, as_user=None):
if everything: #status = ApprovalStatus.PENDING.value
approvals = ApprovalService.get_all_approvals(include_cancelled=True) user = g.user.uid
elif as_user: if as_user:
approvals = ApprovalService.get_all_approvals(as_user, include_cancelled=False) user = as_user
else: approvals = ApprovalService.get_approvals_per_user(user, status,
approvals = ApprovalService.get_approvals_per_user(g.user.uid, include_cancelled=False) include_cancelled=False)
results = ApprovalSchema(many=True).dump(approvals) results = ApprovalSchema(many=True).dump(approvals)
return results return results

View File

@ -84,13 +84,13 @@ class Approval(object):
instance.associated_files = [] instance.associated_files = []
for approval_file in model.approval_files: for approval_file in model.approval_files:
try: 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] extra_info = doc_dictionary[approval_file.file_data.file_model.irb_doc_code]
except: except:
extra_info = None extra_info = None
associated_file = {} associated_file = {}
associated_file['id'] = approval_file.file_data.file_model.id associated_file['id'] = approval_file.file_data.file_model.id
if extra_info: if extra_info:
irb_doc_code = approval_file.file_data.file_model.irb_doc_code
associated_file['name'] = '_'.join((extra_info['category1'], associated_file['name'] = '_'.join((extra_info['category1'],
approval_file.file_data.file_model.name)) approval_file.file_data.file_model.name))
associated_file['description'] = extra_info['description'] associated_file['description'] = extra_info['description']

View File

@ -9,54 +9,52 @@ from crc.models.approval import ApprovalModel, ApprovalStatus, ApprovalFile, App
from crc.models.study import StudyModel from crc.models.study import StudyModel
from crc.models.workflow import WorkflowModel from crc.models.workflow import WorkflowModel
from crc.services.file_service import FileService from crc.services.file_service import FileService
from crc.services.ldap_service import LdapService
class ApprovalService(object): class ApprovalService(object):
"""Provides common tools for working with an Approval""" """Provides common tools for working with an Approval"""
@staticmethod @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', """Returns one approval, with all additional approvals as 'related_approvals',
the main approval can be pinned to an approver with an optional argument. the main approval can be pinned to an approver with an optional argument.
Will return null if no approvals exist on the study.""" Will return null if no approvals exist on the study."""
main_approval = None main_approval = None
related_approvals = [] related_approvals = []
query = db.session.query(ApprovalModel).\ query = db.session.query(ApprovalModel).filter(ApprovalModel.study_id == study.id)
filter(ApprovalModel.study_id == study.id)
if not include_cancelled: if not include_cancelled:
query=query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value) query=query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value)
approvals = query.all() # All non-cancelled approvals. approvals = query.all() # All non-cancelled approvals.
study_approval_status = ""
# IF THIS IS RELATED TO THE CURRENT USER
for approval_model in approvals: for approval_model in approvals:
if approval_model.approver_uid == approver_uid: if approval_model.approver_uid == approver_uid:
main_approval = Approval.from_model(approval_model) main_approval = approval_model
else: 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 WE ARE JUST RETURNING ALL OF THE APPROVALS PER STUDY
if not main_approval and len(related_approvals) > 0: if not main_approval and len(related_approvals) > 0:
main_approval = related_approvals[0] main_approval = related_approvals[0]
related_approvals = related_approvals[1:] 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. 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 return main_approval
@staticmethod @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 # 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 # 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. # the state of the approval to be Declined, or Waiting respectively.
if approval.status == ApprovalStatus.PENDING.value: if approval.status == ApprovalStatus.PENDING.value:
for ra in approval.related_approvals: for ra in related:
if ra.id < approval.id: if ra.id < approval.id:
if ra.status == ApprovalStatus.DECLINED.value or ra.status == ApprovalStatus.CANCELED.value: 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. 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 return approval.status
@staticmethod @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 """Returns a list of approval objects (not db models) for the given
approver. """ approver. """
studies = db.session.query(StudyModel).join(ApprovalModel).\ studies = db.session.query(StudyModel).join(ApprovalModel).\
filter(ApprovalModel.approver_uid == approver_uid).all() filter(ApprovalModel.approver_uid == approver_uid).all()
approvals = [] approvals = []
for study in studies: 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: if approval:
approvals.append(approval) approvals.append(approval)
return approvals return approvals

View File

@ -18,7 +18,7 @@ class LdapService(object):
user_or_last_name_search = "(&(objectclass=person)(|(uid=%s*)(sn=%s*)))" user_or_last_name_search = "(&(objectclass=person)(|(uid=%s*)(sn=%s*)))"
cn_single_search = '(&(objectclass=person)(cn=%s*))' cn_single_search = '(&(objectclass=person)(cn=%s*))'
cn_double_search = '(&(objectclass=person)(&(cn=%s*)(cn=*%s*)))' cn_double_search = '(&(objectclass=person)(&(cn=%s*)(cn=*%s*)))'
temp_cache = {}
conn = None conn = None
@staticmethod @staticmethod
@ -43,6 +43,7 @@ class LdapService(object):
def user_info(uva_uid): def user_info(uva_uid):
user_info = db.session.query(LdapModel).filter(LdapModel.uid == uva_uid).first() user_info = db.session.query(LdapModel).filter(LdapModel.uid == uva_uid).first()
if not user_info: if not user_info:
app.logger.info("No cache for " + uva_uid)
search_string = LdapService.uid_search_string % uva_uid search_string = LdapService.uid_search_string % uva_uid
conn = LdapService.__get_conn() conn = LdapService.__get_conn()
conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes) conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes)
@ -51,6 +52,7 @@ class LdapService(object):
entry = conn.entries[0] entry = conn.entries[0]
user_info = LdapModel.from_entry(entry) user_info = LdapModel.from_entry(entry)
db.session.add(user_info) db.session.add(user_info)
db.session.commit()
return user_info return user_info
@staticmethod @staticmethod

View File

@ -68,16 +68,15 @@ class TestApprovals(BaseTest):
approval = response[0] approval = response[0]
self.assertEqual(approval['approver']['uid'], approver_uid) self.assertEqual(approval['approver']['uid'], approver_uid)
def test_list_approvals_per_admin(self): def test_list_approvals_as_user(self):
"""All approvals will be returned""" """All approvals as different user"""
rv = self.app.get('/v1.0/approval?everything=true', headers=self.logged_in_headers()) rv = self.app.get('/v1.0/approval?as_user=lb3dp', headers=self.logged_in_headers())
self.assert_success(rv) self.assert_success(rv)
response = json.loads(rv.get_data(as_text=True)) response = json.loads(rv.get_data(as_text=True))
# Returned approvals should match what's in the db, we should get one approval back # Returned approvals should match what's in the db for user ld3dp, we should get one
# per study (2 studies), and that approval should have one related approval. # approval back per study (2 studies), and that approval should have one related approval.
approvals_count = ApprovalModel.query.count()
response_count = len(response) response_count = len(response)
self.assertEqual(2, response_count) self.assertEqual(2, response_count)