Merge pull request #138 from sartography/fix/cleanup_todos
Cleanup pending TODOs
This commit is contained in:
commit
ff7c30b879
|
@ -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
|
||||
instance = cls()
|
||||
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 = ''
|
||||
args = dict((k, v) for k, v in model.__dict__.items() if not k.startswith('_'))
|
||||
instance = cls(**args)
|
||||
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)
|
||||
|
|
|
@ -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.
|
||||
latest_approval_request = db.session.query(ApprovalModel). \
|
||||
# Find any existing approvals for this workflow.
|
||||
latest_approval_requests = db.session.query(ApprovalModel). \
|
||||
filter(ApprovalModel.workflow_id == workflow_id). \
|
||||
filter(ApprovalModel.approver_uid == approver_uid). \
|
||||
order_by(desc(ApprovalModel.version)).first()
|
||||
order_by(desc(ApprovalModel.version))
|
||||
|
||||
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:
|
||||
request_file_ids = list(file.file_data_id for file in latest_approval_request.approval_files)
|
||||
if latest_approver_request:
|
||||
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
|
||||
db.session.add(latest_approval_request)
|
||||
version = latest_approval_request.version + 1
|
||||
for approval_request in latest_approval_requests:
|
||||
if (approval_request.version == latest_approver_request.version and
|
||||
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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -384,7 +384,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):
|
||||
|
|
|
@ -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()]
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
pass
|
||||
self.load_example_data()
|
||||
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()
|
||||
|
|
Loading…
Reference in New Issue