Merge branch 'dev' into feature/enum_lookup_data

crc/models/approval.py

@@ -57,23 +57,11 @@ class Approval(object):
 
     @classmethod
     def from_model(cls, model: ApprovalModel):
-        # TODO: Reduce the code by iterating over model's dict keys
+        args = dict((k, v) for k, v in model.__dict__.items() if not k.startswith('_'))
-        instance = cls()
+        instance = cls(**args)
-        instance.id = model.id
-        instance.study_id = model.study_id
-        instance.workflow_id = model.workflow_id
-        instance.version = model.version
-        instance.approver_uid = model.approver_uid
-        instance.status = model.status
-        instance.message = model.message
-        instance.date_created = model.date_created
-        instance.date_approved = model.date_approved
-        instance.version = model.version
-        instance.title = ''
         instance.related_approvals = []
+        instance.title = model.study.title if model.study else ''
 
-        if model.study:
-            instance.title = model.study.title
         try:
             instance.approver = LdapService.user_info(model.approver_uid)
             instance.primary_investigator = LdapService.user_info(model.study.primary_investigator_id)

crc/services/approval_service.py

@@ -287,7 +287,7 @@ class ApprovalService(object):
                     )
                     if mail_result:
                         app.logger.error(mail_result, exc_info=True)
-        # TODO: Log update action by approver_uid - maybe ?
+
         return db_approval
 
     @staticmethod
@@ -299,11 +299,12 @@ class ApprovalService(object):
             pending approvals and create a new approval for the latest version
             of the workflow."""
 
-        # Find any existing approvals for this workflow and approver.
+        # Find any existing approvals for this workflow.
-        latest_approval_request = db.session.query(ApprovalModel). \
+        latest_approval_requests = db.session.query(ApprovalModel). \
             filter(ApprovalModel.workflow_id == workflow_id). \
-            filter(ApprovalModel.approver_uid == approver_uid). \
+            order_by(desc(ApprovalModel.version))
-            order_by(desc(ApprovalModel.version)).first()
+
+        latest_approver_request = latest_approval_requests.filter(ApprovalModel.approver_uid == approver_uid).first()
 
         # Construct as hash of the latest files to see if things have changed since
         # the last approval.
@@ -318,16 +319,20 @@ class ApprovalService(object):
         # If an existing approval request exists and no changes were made, do nothing.
         # If there is an existing approval request for a previous version of the workflow
         # then add a new request, and cancel any waiting/pending requests.
-        if latest_approval_request:
+        if latest_approver_request:
-            request_file_ids = list(file.file_data_id for file in latest_approval_request.approval_files)
+            request_file_ids = list(file.file_data_id for file in latest_approver_request.approval_files)
             current_data_file_ids.sort()
             request_file_ids.sort()
+            other_approver = latest_approval_requests.filter(ApprovalModel.approver_uid != approver_uid).first()
             if current_data_file_ids == request_file_ids:
-                return  # This approval already exists.
+                return  # This approval already exists or we're updating other approver.
             else:
-                latest_approval_request.status = ApprovalStatus.CANCELED.value
+                for approval_request in latest_approval_requests:
-                db.session.add(latest_approval_request)
+                    if (approval_request.version == latest_approver_request.version and
-                version = latest_approval_request.version + 1
+                        approval_request.status != ApprovalStatus.CANCELED.value):
+                        approval_request.status = ApprovalStatus.CANCELED.value
+                        db.session.add(approval_request)
+                version = latest_approver_request.version + 1
         else:
             version = 1
 

crc/services/email_service.py

@@ -13,7 +13,7 @@ class EmailService(object):
     """Provides common tools for working with an Email"""
 
     @staticmethod
-    def add_email(subject, sender, recipients, content, content_html, study_id):
+    def add_email(subject, sender, recipients, content, content_html, study_id=None):
         """We will receive all data related to an email and store it"""
 
         # Find corresponding study - if any

crc/services/study_service.py

@@ -1,3 +1,4 @@
+from copy import copy
 from datetime import datetime
 import json
 from typing import List
@@ -185,6 +186,7 @@ class StudyService(object):
 
     @staticmethod
     def get_investigators(study_id, all=False):
+        """Convert array of investigators from protocol builder into a dictionary keyed on the type. """
 
         # Loop through all known investigator types as set in the reference file
         inv_dictionary = FileService.get_reference_data(FileService.INVESTIGATOR_LIST, 'code')
@@ -192,18 +194,26 @@ class StudyService(object):
         # Get PB required docs
         pb_investigators = ProtocolBuilderService.get_investigators(study_id=study_id)
 
-        """Convert array of investigators from protocol builder into a dictionary keyed on the type"""
+        # It is possible for the same type to show up more than once in some circumstances, in those events
+        # append a counter to the name.
+        investigators = {}
         for i_type in inv_dictionary:
-            pb_data = next((item for item in pb_investigators if item['INVESTIGATORTYPE'] == i_type), None)
+            pb_data_entries = list(item for item in pb_investigators if item['INVESTIGATORTYPE'] == i_type)
-            if pb_data:
+            entry_count = 0
-                inv_dictionary[i_type]['user_id'] = pb_data["NETBADGEID"]
+            investigators[i_type] = copy(inv_dictionary[i_type])
-                inv_dictionary[i_type].update(StudyService.get_ldap_dict_if_available(pb_data["NETBADGEID"]))
+            investigators[i_type]['user_id'] = None
-            else:
+            for pb_data in pb_data_entries:
-                inv_dictionary[i_type]['user_id'] = None
+                entry_count += 1
-
+                if entry_count == 1:
+                    t = i_type
+                else:
+                    t = i_type + "_" + str(entry_count)
+                investigators[t] = copy(inv_dictionary[i_type])
+                investigators[t]['user_id'] = pb_data["NETBADGEID"]
+                investigators[t].update(StudyService.get_ldap_dict_if_available(pb_data["NETBADGEID"]))
         if not all:
-            inv_dictionary = dict(filter(lambda elem: elem[1]['user_id'] is not None, inv_dictionary.items()))
+            investigators = dict(filter(lambda elem: elem[1]['user_id'] is not None, investigators.items()))
-        return inv_dictionary
+        return investigators
 
     @staticmethod
     def get_ldap_dict_if_available(user_id):

crc/services/workflow_service.py

@@ -385,7 +385,8 @@ class WorkflowService(object):
         except TypeError as te:
             raise ApiError.from_task(code="template_error", message="Error processing template for task %s: %s" %
                                                           (spiff_task.task_spec.name, str(te)), task=spiff_task)
-        # TODO:  Catch additional errors and report back.
+        except Exception as e:
+            app.logger.error(str(e), exc_info=True)
 
     @staticmethod
     def process_options(spiff_task, field):

tests/approval/test_approvals_api.py

@@ -217,27 +217,6 @@ class TestApprovals(BaseTest):
         total_counts = sum(counts[status] for status in statuses)
         self.assertEqual(total_counts, len(approvals), 'Total approval counts for user should match number of approvals for user')
 
-    def _create_study_workflow_approvals(self, user_uid, title, primary_investigator_id, approver_uids, statuses,
-                                         workflow_spec_name="random_fact"):
-        study = self.create_study(uid=user_uid, title=title, primary_investigator_id=primary_investigator_id)
-        workflow = self.create_workflow(workflow_name=workflow_spec_name, study=study)
-        approvals = []
-
-        for i in range(len(approver_uids)):
-            approvals.append(self.create_approval(
-                study=study,
-                workflow=workflow,
-                approver_uid=approver_uids[i],
-                status=statuses[i],
-                version=1
-            ))
-
-        return {
-            'study': study,
-            'workflow': workflow,
-            'approvals': approvals,
-        }
-
     def _add_lots_of_random_approvals(self, n=100, workflow_spec_name="random_fact"):
         num_studies_before = db.session.query(StudyModel).count()
         statuses = [name for name, value in ApprovalStatus.__members__.items()]

tests/approval/test_approvals_service.py

@@ -1,7 +1,7 @@
 from tests.base_test import BaseTest
 from crc import db
 from crc.models.approval import ApprovalModel
-from crc.services.approval_service import ApprovalService
+from crc.services.approval_service import ApprovalService, ApprovalStatus
 from crc.services.file_service import FileService
 from crc.services.workflow_processor import WorkflowProcessor
 
@@ -83,6 +83,34 @@ class TestApprovalsService(BaseTest):
 
         self.assertEqual(len(records), 2)
 
+    def test_new_approval_cancels_all_previous_approvals(self):
+        self.create_reference_document()
+        workflow = self.create_workflow("empty_workflow")
+        FileService.add_workflow_file(workflow_id=workflow.id,
+                                      name="anything.png", content_type="text",
+                                      binary_data=b'5678', irb_doc_code="UVACompl_PRCAppr" )
+        ApprovalService.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+        ApprovalService.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="lb3dp")
+
+        current_count = ApprovalModel.query.count()
+        self.assertTrue(current_count, 2)
+
+        FileService.add_workflow_file(workflow_id=workflow.id,
+                                      name="borderline.png", content_type="text",
+                                      binary_data=b'906090', irb_doc_code="AD_CoCAppr" )
+
+        ApprovalService.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+
+        current_count = ApprovalModel.query.count()
+        canceled_count = ApprovalModel.query.filter(ApprovalModel.status == ApprovalStatus.CANCELED.value)
+        self.assertTrue(current_count, 2)
+        self.assertTrue(current_count, 3)
+
+        ApprovalService.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="lb3dp")
+
+        current_count = ApprovalModel.query.count()
+        self.assertTrue(current_count, 4)
+
     def test_new_approval_sends_proper_emails(self):
         self.assertEqual(1, 1)
 

tests/base_test.py

@@ -240,6 +240,29 @@ class BaseTest(unittest.TestCase):
             db.session.commit()
         return study
 
+    def _create_study_workflow_approvals(self, user_uid, title, primary_investigator_id, approver_uids, statuses,
+                                         workflow_spec_name="random_fact"):
+        study = self.create_study(uid=user_uid, title=title, primary_investigator_id=primary_investigator_id)
+        workflow = self.create_workflow(workflow_name=workflow_spec_name, study=study)
+        approvals = []
+
+        for i in range(len(approver_uids)):
+            approvals.append(self.create_approval(
+                study=study,
+                workflow=workflow,
+                approver_uid=approver_uids[i],
+                status=statuses[i],
+                version=1
+            ))
+
+        full_study = {
+            'study': study,
+            'workflow': workflow,
+            'approvals': approvals,
+        }
+
+        return full_study
+
     def create_workflow(self, workflow_name, study=None, category_id=None):
         db.session.flush()
         spec = db.session.query(WorkflowSpecModel).filter(WorkflowSpecModel.name == workflow_name).first()

tests/data/pb_responses/investigators.json

@@ -13,5 +13,15 @@
     "INVESTIGATORTYPE": "PI",
     "INVESTIGATORTYPEFULL": "Primary Investigator",
     "NETBADGEID": "dhf8r"
+  },
+    {
+    "INVESTIGATORTYPE": "SI",
+    "INVESTIGATORTYPEFULL": "Sub Investigator",
+    "NETBADGEID": "ajl2j"
+  },
+  {
+    "INVESTIGATORTYPE": "SI",
+    "INVESTIGATORTYPEFULL": "Sub Investigator",
+    "NETBADGEID": "cah3us"
   }
-]
+]

tests/emails/test_email_service.py

@@ -31,4 +31,15 @@ class TestEmailService(BaseTest):
         self.assertEqual(email_model.content_html, content_html)
         self.assertEqual(email_model.study, study)
 
-        # TODO: Create email model without study
+        subject = 'Email Subject - Empty study'
+        EmailService.add_email(subject=subject, sender=sender, recipients=recipients,
+                               content=content, content_html=content_html)
+
+        email_model = EmailModel.query.order_by(EmailModel.id.desc()).first()
+
+        self.assertEqual(email_model.subject, subject)
+        self.assertEqual(email_model.sender, sender)
+        self.assertEqual(email_model.recipients, str(recipients))
+        self.assertEqual(email_model.content, content)
+        self.assertEqual(email_model.content_html, content_html)
+        self.assertEqual(email_model.study, None)

tests/study/test_study_api.py

@@ -7,6 +7,7 @@ from unittest.mock import patch
 from crc import session, app
 from crc.models.protocol_builder import ProtocolBuilderStatus, \
     ProtocolBuilderStudySchema
+from crc.models.approval import ApprovalStatus
 from crc.models.stats import TaskEventModel
 from crc.models.study import StudyModel, StudySchema
 from crc.models.workflow import WorkflowSpecModel, WorkflowModel
@@ -95,8 +96,21 @@ class TestStudyApi(BaseTest):
         # TODO: WRITE A TEST FOR STUDY FILES
 
     def test_get_study_has_details_about_approvals(self):
-        # TODO: WRITE A TEST FOR STUDY APPROVALS
+        self.load_example_data()
-        pass
+        full_study = self._create_study_workflow_approvals(
+            user_uid="dhf8r", title="first study", primary_investigator_id="lb3dp",
+            approver_uids=["lb3dp", "dhf8r"], statuses=[ApprovalStatus.PENDING.value, ApprovalStatus.PENDING.value]
+        )
+
+        api_response = self.app.get('/v1.0/study/%i' % full_study['study'].id,
+                                    headers=self.logged_in_headers(), content_type="application/json")
+        self.assert_success(api_response)
+        study = StudySchema().loads(api_response.get_data(as_text=True))
+
+        self.assertEqual(len(study.approvals), 2)
+
+        for approval in study.approvals:
+            self.assertEqual(full_study['study'].title, approval['title'])
 
     def test_add_study(self):
         self.load_example_data()

tests/study/test_study_service.py

@@ -193,7 +193,7 @@ class TestStudyService(BaseTest):
         workflow = self.create_workflow('docx') # The workflow really doesnt matter in this case.
         investigators = StudyService().get_investigators(workflow.study_id, all=True)
 
-        self.assertEqual(9, len(investigators))
+        self.assertEqual(10, len(investigators))
 
         # dhf8r is in the ldap mock data.
         self.assertEqual("dhf8r", investigators['PI']['user_id'])
@@ -219,10 +219,14 @@ class TestStudyService(BaseTest):
         workflow = self.create_workflow('docx') # The workflow really doesnt matter in this case.
         investigators = StudyService().get_investigators(workflow.study_id, all=False)
 
-        self.assertEqual(3, len(investigators))
+        self.assertEqual(5, len(investigators))
 
         # dhf8r is in the ldap mock data.
         self.assertEqual("dhf8r", investigators['PI']['user_id'])
         self.assertEqual("Dan Funk", investigators['PI']['display_name']) # Data from ldap
         self.assertEqual("Primary Investigator", investigators['PI']['label']) # Data from xls file.
         self.assertEqual("Always", investigators['PI']['display']) # Data from xls file.
+
+        # Both Alex and Aaron are SI, and both should be returned.
+        self.assertEqual("ajl2j", investigators['SI']['user_id'])
+        self.assertEqual("cah3us", investigators['SI_2']['user_id'])

tests/test_protocol_builder.py

@@ -24,7 +24,7 @@ class TestProtocolBuilder(BaseTest):
         mock_get.return_value.text = self.protocol_builder_response('investigators.json')
         response = ProtocolBuilderService.get_investigators(self.test_study_id)
         self.assertIsNotNone(response)
-        self.assertEqual(3, len(response))
+        self.assertEqual(5, len(response))
         self.assertEqual("DC", response[0]["INVESTIGATORTYPE"])
         self.assertEqual("Department Contact", response[0]["INVESTIGATORTYPEFULL"])
         self.assertEqual("asd3v", response[0]["NETBADGEID"])

tests/test_tasks_api.py

@@ -322,7 +322,7 @@ class TestTasksApi(BaseTest):
         self.assertEqual(4, len(navigation)) # Start task, form_task, multi_task, end task
         self.assertEqual("UserTask", workflow.next_task.type)
         self.assertEqual(MultiInstanceType.sequential.value, workflow.next_task.multi_instance_type)
-        self.assertEqual(3, workflow.next_task.multi_instance_count)
+        self.assertEqual(5, workflow.next_task.multi_instance_count)
 
         # Assure that the names for each task are properly updated, so they aren't all the same.
         self.assertEqual("Primary Investigator", workflow.next_task.properties['display_name'])
@@ -480,15 +480,15 @@ class TestTasksApi(BaseTest):
         workflow = self.create_workflow('multi_instance_parallel')
 
         workflow_api = self.get_workflow_api(workflow)
-        self.assertEqual(6, len(workflow_api.navigation))
+        self.assertEqual(8, len(workflow_api.navigation))
         ready_items = [nav for nav in workflow_api.navigation if nav['state'] == "READY"]
-        self.assertEqual(3, len(ready_items))
+        self.assertEqual(5, len(ready_items))
 
         self.assertEqual("UserTask", workflow_api.next_task.type)
         self.assertEqual("MultiInstanceTask",workflow_api.next_task.name)
         self.assertEqual("Primary Investigator", workflow_api.next_task.title)
 
-        for i in random.sample(range(3), 3):
+        for i in random.sample(range(5), 5):
             task = TaskSchema().load(ready_items[i]['task'])
             rv = self.app.put('/v1.0/workflow/%i/task/%s/set_token' % (workflow.id, task.id),
                               headers=self.logged_in_headers(),