diff --git a/crc/api/approval.py b/crc/api/approval.py index 5d1feea6..c51634e4 100644 --- a/crc/api/approval.py +++ b/crc/api/approval.py @@ -1,10 +1,15 @@ +from crc import app, db, session + from crc.api.common import ApiError, ApiErrorSchema from crc.models.approval import Approval, ApprovalModel, ApprovalSchema from crc.services.approval_service import ApprovalService def get_approvals(approver_uid = None): - db_approvals = ApprovalService.get_all_approvals() + if not approver_uid: + db_approvals = ApprovalService.get_all_approvals() + else: + db_approvals = ApprovalService.get_approvals_per_user(approver_uid) approvals = [Approval.from_model(approval_model) for approval_model in db_approvals] results = ApprovalSchema(many=True).dump(approvals) return results @@ -13,18 +18,14 @@ def update_approval(approval_id, body): if approval_id is None: raise ApiError('unknown_approval', 'Please provide a valid Approval ID.') - approver_uid = body.get('approver_uid') - status = body.get('status') - - if approver_uid is None: - raise ApiError('bad_formed_approval', 'Please provide a valid Approver UID') - if status is None: - raise ApiError('bad_formed_approval', 'Please provide a valid status for approval update') - - db_approval = ApprovalService.update_approval(approval_id, approver_uid, status) - if db_approval is None: + approval_model = session.query(ApprovalModel).get(approval_id) + if approval_model is None: raise ApiError('unknown_approval', 'The approval "' + str(approval_id) + '" is not recognized.') - approval = Approval.from_model(db_approval) + approval: Approval = ApprovalSchema().load(body) + approval.update_model(approval_model) + session.add(approval_model) + session.commit() + result = ApprovalSchema().dump(approval) return result diff --git a/crc/models/approval.py b/crc/models/approval.py index 37262e54..90fa14e2 100644 --- a/crc/models/approval.py +++ b/crc/models/approval.py @@ -1,12 +1,17 @@ import enum +import marshmallow from marshmallow import INCLUDE from sqlalchemy import func +from ldap3.core.exceptions import LDAPSocketOpenError + from crc import db, ma +from crc.api.common import ApiError from crc.models.file import FileModel from crc.models.study import StudyModel from crc.models.workflow import WorkflowModel +from crc.services.ldap_service import LdapService class ApprovalStatus(enum.Enum): @@ -45,8 +50,12 @@ class ApprovalModel(db.Model): class Approval(object): + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + @classmethod def from_model(cls, model: ApprovalModel): + # TODO: Reduce the code by iterating over model's dict keys instance = cls() instance.id = model.id instance.study_id = model.study_id @@ -62,12 +71,18 @@ class Approval(object): if model.study: instance.title = model.study.title - # TODO: Use ldap lookup - instance.approver = {} - instance.approver['uid'] = 'bgb22' - instance.approver['display_name'] = 'Billy Bob (bgb22)' - instance.approver['title'] = 'E42:He\'s a hoopy frood' - instance.approver['department'] = 'E0:EN-Eng Study of Parallel Universes' + try: + ldap_service = LdapService() + user_info = ldap_service.user_info(model.approver_uid) + except (ApiError, LDAPSocketOpenError) as exception: + user_info = None + + if user_info: + instance.approver = {} + 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 instance.associated_files = [] for approval_file in model.approval_files: @@ -79,6 +94,11 @@ class Approval(object): return instance + def update_model(self, study_model: StudyModel): + for k,v in self.__dict__.items(): + if not k.startswith('_'): + study_model.__dict__[k] = v + class ApprovalSchema(ma.Schema): class Meta: @@ -87,6 +107,10 @@ class ApprovalSchema(ma.Schema): "version", "status", "approver", "associated_files"] unknown = INCLUDE + @marshmallow.post_load + def make_approval(self, data, **kwargs): + """Loads the basic approval data for updates to the database""" + return Approval(**data) # Carlos: Here is the data structure I was trying to imagine. # If I were to continue down my current traing of thought, I'd create diff --git a/tests/test_approvals_api.py b/tests/test_approvals_api.py index b0453732..8cc14d14 100644 --- a/tests/test_approvals_api.py +++ b/tests/test_approvals_api.py @@ -2,7 +2,35 @@ import json from tests.base_test import BaseTest from crc import app, db, session -from crc.models.approval import ApprovalModel +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, + '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): @@ -15,32 +43,62 @@ class TestApprovals(BaseTest): self.approval = ApprovalModel( study=self.study, workflow=self.workflow, - approver_uid='bgb22', - status='WAITING', # TODO: Use enumerate options + approver_uid='arc93', + status=ApprovalStatus.WAITING.value, version=1 ) session.add(self.approval) + + self.approval_2 = ApprovalModel( + study=self.study, + workflow=self.workflow, + approver_uid='dhf8r', + status=ApprovalStatus.WAITING.value, + version=1 + ) + session.add(self.approval_2) + session.commit() def test_list_approvals_per_approver(self): """Only approvals associated with approver should be returned""" - rv = self.app.get('/v1.0/approval', headers=self.logged_in_headers()) + approver_uid = self.approval_2.approver_uid + rv = self.app.get(f'/v1.0/approval?approver_uid={approver_uid}', headers=self.logged_in_headers()) self.assert_success(rv) + response = json.loads(rv.get_data(as_text=True)) + + # Stored approvals are 2 + approvals_count = ApprovalModel.query.count() + self.assertEqual(approvals_count, 2) + + # but Dan's approvals should be only 1 + self.assertEqual(len(response), 1) + + # Confirm approver UID matches returned payload + approval = ApprovalSchema().load(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', 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 + approvals_count = ApprovalModel.query.count() + response_count = len(response) + self.assertEqual(approvals_count, response_count) + def test_update_approval(self): """Approval status will be updated""" - body = { - 'approver_uid': 'rvr98', - 'status': 'DECLINED' - } approval_id = self.approval.id + data = dict(APPROVAL_PAYLOAD) + data['id'] = approval_id + data = json.dumps(data) rv = self.app.put(f'/v1.0/approval/{approval_id}', content_type="application/json", headers=self.logged_in_headers(), - data=json.dumps(body)) + data=data) self.assert_success(rv)