Merge branch 'rrt/testing' into rrt/staging

Pipfile

@@ -38,6 +38,7 @@ xlrd = "*"
 ldap3 = "*"
 gunicorn = "*"
 werkzeug = "*"
+sentry-sdk = {extras = ["flask"],version = "==0.14.4"}
 
 [requires]
 python_version = "3.7"

Pipfile.lock

@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "979f996148ee181e3e0af2a3777aa1d00d0fd5d943d49df65963e694b8a88871"
+            "sha256": "54d9d51360f54762138a3acc7696badd1d711e7b1dde9e2d82aa706e40c17102"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -32,10 +32,10 @@
         },
         "amqp": {
             "hashes": [
-                "sha256:6e649ca13a7df3faacdc8bbb280aa9a6602d22fd9d545336077e573a1f4ff3b8",
+                "sha256:24dbaff8ce4f30566bb88976b398e8c4e77637171af3af6f1b9650f48890e60b",
-                "sha256:77f1aef9410698d20eaeac5b73a87817365f457a507d82edf292e12cbb83b08d"
+                "sha256:bb68f8d2bced8f93ccfd07d96c689b716b3227720add971be980accfc2952139"
             ],
-            "version": "==2.5.2"
+            "version": "==2.6.0"
         },
         "aniso8601": {
             "hashes": [
@@ -96,12 +96,18 @@
             ],
             "version": "==3.6.3.0"
         },
+        "blinker": {
+            "hashes": [
+                "sha256:471aee25f3992bd325afa3772f1063dbdbbca947a041b8b89466dc00d606f8b6"
+            ],
+            "version": "==1.4"
+        },
         "celery": {
             "hashes": [
-                "sha256:108a0bf9018a871620936c33a3ee9f6336a89f8ef0a0f567a9001f4aa361415f",
+                "sha256:5147662e23dc6bc39c17a2cbc9a148debe08ecfb128b0eded14a0d9c81fc5742",
-                "sha256:5b4b37e276033fe47575107a2775469f0b721646a08c96ec2c61531e4fe45f2a"
+                "sha256:df2937b7536a2a9b18024776a3a46fd281721813636c03a5177fa02fe66078f6"
             ],
-            "version": "==4.4.2"
+            "version": "==4.4.3"
         },
         "certifi": {
             "hashes": [
@@ -381,10 +387,10 @@
         },
         "kombu": {
             "hashes": [
-                "sha256:2d1cda774126a044d91a7ff5fa6d09edf99f46924ab332a810760fe6740e9b76",
+                "sha256:ab0afaa5388dd2979cbc439d3623b86a4f7a58d41f621096bef7767c37bc2505",
-                "sha256:598e7e749d6ab54f646b74b2d2df67755dee13894f73ab02a2a9feb8870c7cb2"
+                "sha256:aece08f48706743aaa1b9d607fee300559481eafcc5ee56451aa0ef867a3be07"
             ],
-            "version": "==4.6.8"
+            "version": "==4.6.9"
         },
         "ldap3": {
             "hashes": [
@@ -704,6 +710,17 @@
             "index": "pypi",
             "version": "==2.23.0"
         },
+        "sentry-sdk": {
+            "extras": [
+                "flask"
+            ],
+            "hashes": [
+                "sha256:0e5e947d0f7a969314aa23669a94a9712be5a688ff069ff7b9fc36c66adc160c",
+                "sha256:799a8bf76b012e3030a881be00e97bc0b922ce35dde699c6537122b751d80e2c"
+            ],
+            "index": "pypi",
+            "version": "==0.14.4"
+        },
         "six": {
             "hashes": [
                 "sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259",
@@ -838,10 +855,10 @@
         },
         "waitress": {
             "hashes": [
-                "sha256:045b3efc3d97c93362173ab1dfc159b52cfa22b46c3334ffc805dbdbf0e4309e",
+                "sha256:1bb436508a7487ac6cb097ae7a7fe5413aefca610550baf58f0940e51ecfb261",
-                "sha256:77ff3f3226931a1d7d8624c5371de07c8e90c7e5d80c5cc660d72659aaf23f38"
+                "sha256:3d633e78149eb83b60a07dfabb35579c29aac2d24bb803c18b26fb2ab1a584db"
             ],
-            "version": "==1.4.3"
+            "version": "==1.4.4"
         },
         "webob": {
             "hashes": [
@@ -966,10 +983,10 @@
         },
         "wcwidth": {
             "hashes": [
-                "sha256:cafe2186b3c009a04067022ce1dcd79cb38d8d65ee4f4791b8888d6599d1bbe1",
+                "sha256:3de2e41158cb650b91f9654cbf9a3e053cee0719c9df4ddc11e4b568669e9829",
-                "sha256:ee73862862a156bf77ff92b09034fc4825dd3af9cf81bc5b360668d425f3c5f1"
+                "sha256:b651b6b081476420e4e9ae61239ac4c1b49d0c5ace42b2e81dc2ff49ed50c566"
             ],
-            "version": "==0.1.9"
+            "version": "==0.2.2"
         },
         "zipp": {
             "hashes": [

config/default.py

@@ -13,6 +13,9 @@ DEVELOPMENT = environ.get('DEVELOPMENT', default="true") == "true"
 TESTING = environ.get('TESTING', default="false") == "true"
 PRODUCTION = (environ.get('PRODUCTION', default="false") == "true") or (not DEVELOPMENT and not TESTING)
 
+# Sentry flag
+ENABLE_SENTRY = environ.get('ENABLE_SENTRY', default="false") == "true"
+
 # Add trailing slash to base path
 APPLICATION_ROOT = re.sub(r'//', '/', '/%s/' % environ.get('APPLICATION_ROOT', default="/").strip('/'))
 

crc/__init__.py

@@ -1,11 +1,13 @@
 import logging
 import os
+import sentry_sdk
 
 import connexion
 from flask_cors import CORS
 from flask_marshmallow import Marshmallow
 from flask_migrate import Migrate
 from flask_sqlalchemy import SQLAlchemy
+from sentry_sdk.integrations.flask import FlaskIntegration
 
 logging.basicConfig(level=logging.INFO)
 
@@ -40,6 +42,12 @@ connexion_app.add_api('api.yml', base_path='/v1.0')
 origins_re = [r"^https?:\/\/%s(.*)" % o.replace('.', '\.') for o in app.config['CORS_ALLOW_ORIGINS']]
 cors = CORS(connexion_app.app, origins=origins_re)
 
+if app.config['ENABLE_SENTRY']:
+    sentry_sdk.init(
+        dsn="https://25342ca4e2d443c6a5c49707d68e9f40@o401361.ingest.sentry.io/5260915",
+        integrations=[FlaskIntegration()]
+    )
+
 print('=== USING THESE CONFIG SETTINGS: ===')
 print('DB_HOST = ', )
 print('CORS_ALLOW_ORIGINS = ', app.config['CORS_ALLOW_ORIGINS'])

crc/api.yml

@@ -808,12 +808,12 @@ paths:
                   $ref: "#/components/schemas/Script"
   /approval:
     parameters:
-      - name: approver_uid
+      - name: everything
         in: query
         required: false
-        description: Restrict results to a given approver uid, maybe we restrict the use of this at somepoint.
+        description: If set to true, returns all the approvals known to the system.
         schema:
-          type: string
+          type: boolean
     get:
       operationId: crc.api.approval.get_approvals
       summary: Provides a list of workflows approvals

crc/api/approval.py

@@ -1,3 +1,5 @@
+from flask import g
+
 from crc import app, db, session
 
 from crc.api.common import ApiError, ApiErrorSchema
@@ -5,11 +7,11 @@ from crc.models.approval import Approval, ApprovalModel, ApprovalSchema
 from crc.services.approval_service import ApprovalService
 
 
-def get_approvals(approver_uid=None):
+def get_approvals(everything=False):
-    if not approver_uid:
+    if everything:
         db_approvals = ApprovalService.get_all_approvals()
     else:
-        db_approvals = ApprovalService.get_approvals_per_user(approver_uid)
+        db_approvals = ApprovalService.get_approvals_per_user(g.user.uid)
     approvals = [Approval.from_model(approval_model) for approval_model in db_approvals]
 
     results = ApprovalSchema(many=True).dump(approvals)
@@ -32,6 +34,9 @@ def update_approval(approval_id, body):
         raise ApiError('unknown_approval', 'The approval "' + str(approval_id) + '" is not recognized.')
 
     approval: Approval = ApprovalSchema().load(body)
+    if approval_model.approver_uid != g.user.uid:
+        raise ApiError("not_your_approval", "You may not modify this approval. It belongs to another user.")
+
     approval.update_model(approval_model)
     session.commit()
 

crc/api/user.py

@@ -1,8 +1,5 @@
-import json
-
-import connexion
 import flask
-from flask import redirect, g, request
+from flask import g, request
 
 from crc import app, db
 from crc.api.common import ApiError

crc/models/approval.py

@@ -68,24 +68,33 @@ class Approval(object):
         if model.study:
             instance.title = model.study.title
 
-        principal_investigator_id = model.study.primary_investigator_id
         instance.approver = {}
         try:
             ldap_service = LdapService()
-            user_info = ldap_service.user_info(principal_investigator_id)
+            user_info = ldap_service.user_info(model.approver_uid)
-        except (ApiError, LDAPSocketOpenError) as exception:
-            user_info = None
-            instance.approver['display_name'] = 'Primary Investigator details'
-            instance.approver['department'] = 'currently not available'
-
-        if user_info:
-            # TODO: Rename approver to primary investigator
             instance.approver['uid'] = model.approver_uid
             instance.approver['display_name'] = user_info.display_name
             instance.approver['title'] = user_info.title
             instance.approver['department'] = user_info.department
+        except (ApiError, LDAPSocketOpenError) as exception:
+            user_info = None
+            instance.approver['display_name'] = 'Unknown'
+            instance.approver['department'] = 'currently not available'
+
+        instance.primary_investigator = {}
+        try:
+            ldap_service = LdapService()
+            user_info = ldap_service.user_info(model.study.primary_investigator_id)
+            instance.primary_investigator['uid'] = model.approver_uid
+            instance.primary_investigator['display_name'] = user_info.display_name
+            instance.primary_investigator['title'] = user_info.title
+            instance.primary_investigator['department'] = user_info.department
+        except (ApiError, LDAPSocketOpenError) as exception:
+            user_info = None
+            instance.primary_investigator['display_name'] = 'Primary Investigator details'
+            instance.primary_investigator['department'] = 'currently not available'
+
 
-        # TODO: Organize it properly, move it to services
         doc_dictionary = FileService.get_reference_data(FileService.DOCUMENT_LIST, 'code', ['id'])
 
         instance.associated_files = []
@@ -98,12 +107,13 @@ class Approval(object):
             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((irb_doc_code, approval_file.file_data.file_model.name))
+                associated_file['name'] = '_'.join((extra_info['category1'],
+                                                    approval_file.file_data.file_model.name))
                 associated_file['description'] = extra_info['description']
             else:
                 associated_file['name'] = approval_file.file_data.file_model.name
                 associated_file['description'] = 'No description available'
-            associated_file['name'] = '(' + principal_investigator_id + ')' + associated_file['name']
+            associated_file['name'] = '(' +  model.study.primary_investigator_id + ')' + associated_file['name']
             associated_file['content_type'] = approval_file.file_data.file_model.content_type
             instance.associated_files.append(associated_file)
 
@@ -118,7 +128,8 @@ class ApprovalSchema(ma.Schema):
     class Meta:
         model = Approval
         fields = ["id", "study_id", "workflow_id", "version", "title",
-            "version", "status", "message", "approver", "associated_files"]
+                  "status", "message", "approver", "primary_investigator",
+                  "associated_files", "date_created"]
         unknown = INCLUDE
 
     @marshmallow.post_load

crc/scripts/request_approval.py

@@ -26,7 +26,8 @@ RequestApproval approver1 "dhf8r"
             ApprovalService.add_approval(study_id, workflow_id, args)
         elif isinstance(uids, list):
             for id in uids:
-                ApprovalService.add_approval(study_id, workflow_id, id)
+                if id: ## Assure it's not empty or null
+                    ApprovalService.add_approval(study_id, workflow_id, id)
 
     def get_uids(self, task, args):
         if len(args) < 1:

package-lock.json

@@ -0,0 +1,3 @@
+{
+  "lockfileVersion": 1
+}

tests/test_approvals_api.py

@@ -5,35 +5,6 @@ from crc import app, db, session
 from crc.models.approval import ApprovalModel, ApprovalSchema, ApprovalStatus
 
 
-APPROVAL_PAYLOAD = {
-    'id': None,
-    'approver': {
-      'uid': 'bgb22',
-      'display_name': 'Billy Bob (bgb22)',
-      'title': 'E42:He\'s a hoopy frood',
-      'department': 'E0:EN-Eng Study of Parallel Universes'
-    },
-    'title': 'El Study',
-    'status': 'DECLINED',
-    'version': 1,
-    'message': 'Incorrect documents',
-    'associated_files': [
-      {
-        'id': 42,
-        'name': 'File 1',
-        'content_type': 'document'
-      },
-      {
-        'id': 43,
-        'name': 'File 2',
-        'content_type': 'document'
-      }
-    ],
-    'workflow_id': 1,
-    'study_id': 1
-}
-
-
 class TestApprovals(BaseTest):
     def setUp(self):
         """Initial setup shared by all TestApprovals tests"""
@@ -64,7 +35,7 @@ class TestApprovals(BaseTest):
     def test_list_approvals_per_approver(self):
         """Only approvals associated with approver should be returned"""
         approver_uid = self.approval_2.approver_uid
-        rv = self.app.get(f'/v1.0/approval?approver_uid={approver_uid}', headers=self.logged_in_headers())
+        rv = self.app.get(f'/v1.0/approval', headers=self.logged_in_headers())
         self.assert_success(rv)
 
         response = json.loads(rv.get_data(as_text=True))
@@ -82,7 +53,7 @@ class TestApprovals(BaseTest):
 
     def test_list_approvals_per_admin(self):
         """All approvals will be returned"""
-        rv = self.app.get('/v1.0/approval', headers=self.logged_in_headers())
+        rv = self.app.get('/v1.0/approval?everything=true', headers=self.logged_in_headers())
         self.assert_success(rv)
 
         response = json.loads(rv.get_data(as_text=True))
@@ -90,24 +61,67 @@ class TestApprovals(BaseTest):
         # Returned approvals should match what's in the db
         approvals_count = ApprovalModel.query.count()
         response_count = len(response)
-        self.assertEqual(approvals_count, response_count)
+        self.assertEqual(2, response_count)
 
-    def test_update_approval(self):
+        rv = self.app.get('/v1.0/approval', headers=self.logged_in_headers())
-        """Approval status will be updated"""
+        self.assert_success(rv)
-        approval_id = self.approval.id
+        response = json.loads(rv.get_data(as_text=True))
-        data = dict(APPROVAL_PAYLOAD)
+        response_count = len(response)
-        data['id'] = approval_id
+        self.assertEqual(1, response_count)
 
-        self.assertEqual(self.approval.status, ApprovalStatus.PENDING.value)
+    def test_update_approval_fails_if_not_the_approver(self):
+        approval = session.query(ApprovalModel).filter_by(approver_uid='arc93').first()
+        data = {'id': approval.id,
+                "approver_uid": "dhf8r",
+                'message': "Approved.  I like the cut of your jib.",
+                'status': ApprovalStatus.APPROVED.value}
 
-        rv = self.app.put(f'/v1.0/approval/{approval_id}',
+        self.assertEqual(approval.status, ApprovalStatus.PENDING.value)
+
+        rv = self.app.put(f'/v1.0/approval/{approval.id}',
                           content_type="application/json",
-                          headers=self.logged_in_headers(),
+                          headers=self.logged_in_headers(),  # As dhf8r
+                          data=json.dumps(data))
+        self.assert_failure(rv)
+
+    def test_accept_approval(self):
+        approval = session.query(ApprovalModel).filter_by(approver_uid='dhf8r').first()
+        data = {'id': approval.id,
+                "approver_uid": "dhf8r",
+                'message': "Approved.  I like the cut of your jib.",
+                'status': ApprovalStatus.APPROVED.value}
+
+        self.assertEqual(approval.status, ApprovalStatus.PENDING.value)
+
+        rv = self.app.put(f'/v1.0/approval/{approval.id}',
+                          content_type="application/json",
+                          headers=self.logged_in_headers(),  # As dhf8r
                           data=json.dumps(data))
         self.assert_success(rv)
 
-        session.refresh(self.approval)
+        session.refresh(approval)
 
         # Updated record should now have the data sent to the endpoint
-        self.assertEqual(self.approval.message, data['message'])
+        self.assertEqual(approval.message, data['message'])
-        self.assertEqual(self.approval.status, ApprovalStatus.DECLINED.value)
+        self.assertEqual(approval.status, ApprovalStatus.APPROVED.value)
+
+    def test_decline_approval(self):
+        approval = session.query(ApprovalModel).filter_by(approver_uid='dhf8r').first()
+        data = {'id': approval.id,
+                "approver_uid": "dhf8r",
+                'message': "Approved.  I find the cut of your jib lacking.",
+                'status': ApprovalStatus.DECLINED.value}
+
+        self.assertEqual(approval.status, ApprovalStatus.PENDING.value)
+
+        rv = self.app.put(f'/v1.0/approval/{approval.id}',
+                          content_type="application/json",
+                          headers=self.logged_in_headers(),  # As dhf8r
+                          data=json.dumps(data))
+        self.assert_success(rv)
+
+        session.refresh(approval)
+
+        # Updated record should now have the data sent to the endpoint
+        self.assertEqual(approval.message, data['message'])
+        self.assertEqual(approval.status, ApprovalStatus.DECLINED.value)

tests/test_request_approval_script.py

@@ -1,6 +1,6 @@
-from crc.services.file_service import FileService
 from tests.base_test import BaseTest
 
+from crc.services.file_service import FileService
 from crc.scripts.request_approval import RequestApproval
 from crc.services.workflow_processor import WorkflowProcessor
 from crc.api.common import ApiError
@@ -26,6 +26,22 @@ class TestRequestApprovalScript(BaseTest):
         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_blank_second_approver(self):
+        self.load_example_data()
+        self.create_reference_document()
+        workflow = self.create_workflow('empty_workflow')
+        processor = WorkflowProcessor(workflow)
+        task = processor.next_task()
+        task.data = {"study": {"approval1": "dhf8r", 'approval2':''}}
+        FileService.add_workflow_file(workflow_id=workflow.id,
+                                      irb_doc_code="UVACompl_PRCAppr",
+                                      name="anything.png", content_type="text",
+                                      binary_data=b'1234')
+        script = RequestApproval()
+        script.do_task(task, workflow.study_id, workflow.id, "study.approval1", "study.approval2")
+        self.assertEquals(1, db.session.query(ApprovalModel).count())
+
+
     def test_do_task_with_incorrect_argument(self):
         """This script should raise an error if it can't figure out the approvers."""
         self.load_example_data()

tests/test_tasks_api.py

@@ -10,7 +10,6 @@ from crc.models.api_models import WorkflowApiSchema, MultiInstanceType, TaskSche
 from crc.models.file import FileModelSchema
 from crc.models.stats import TaskEventModel
 from crc.models.workflow import WorkflowStatus
-from crc.services.protocol_builder import ProtocolBuilderService
 from crc.services.workflow_service import WorkflowService