diff --git a/crc/__init__.py b/crc/__init__.py index dcb6cde8..7fde7de0 100644 --- a/crc/__init__.py +++ b/crc/__init__.py @@ -47,6 +47,7 @@ ma = Marshmallow(app) from crc import models from crc import api from crc.api import admin +from crc.services.file_service import FileService from crc.services.workflow_service import WorkflowService connexion_app.add_api('api.yml', base_path='/v1.0') @@ -57,6 +58,7 @@ def process_waiting_tasks(): WorkflowService.do_waiting() scheduler.add_job(process_waiting_tasks,'interval',minutes=1) +scheduler.add_job(FileService.cleanup_file_data, 'interval', minutes=1440) # once a day scheduler.start() diff --git a/crc/api/tools.py b/crc/api/tools.py index 684b1fee..55b599a5 100644 --- a/crc/api/tools.py +++ b/crc/api/tools.py @@ -102,11 +102,12 @@ def evaluate_python_expression(body): except Exception as e: return {"result": False, "expression": body['expression'], "key": body['key'], "error": str(e)} + def send_test_email(subject, address, message, data=None): - rendered, wrapped = EmailService().get_rendered_content(message, data) + content, content_html = EmailService().get_rendered_content(message, data) EmailService.add_email( subject=subject, sender=DEFAULT_SENDER, recipients=[address], - content=rendered, - content_html=wrapped) + content=content, + content_html=content_html) diff --git a/crc/models/email.py b/crc/models/email.py index a549cfdb..cf2bb543 100644 --- a/crc/models/email.py +++ b/crc/models/email.py @@ -23,9 +23,8 @@ class EmailModel(db.Model): class EmailModelSchema(ma.Schema): - # TODO: clean this us. Do we need load_instance and unknown? + class Meta: model = EmailModel - load_instance = True - additional = ["id", "subject", "sender", "recipients", "timestamp"] - unknown = INCLUDE + fields = ["id", "subject", "sender", "recipients", "cc", "bcc", "content", "content_html", + "study_id", "timestamp", "workflow_spec_id"] diff --git a/crc/scripts/email.py b/crc/scripts/email.py index bca72827..2e5a7105 100644 --- a/crc/scripts/email.py +++ b/crc/scripts/email.py @@ -3,6 +3,7 @@ import traceback from crc import app, session from crc.api.common import ApiError +from crc.models.email import EmailModelSchema from crc.models.file import FileModel, CONTENT_TYPES from crc.models.workflow import WorkflowModel from crc.services.document_service import DocumentService @@ -58,7 +59,8 @@ email(subject="My Subject", recipients="user@example.com", attachments=['Study_A bcc = self.get_email_addresses(kwargs['bcc'], study_id) if 'reply_to' in kwargs: reply_to = kwargs['reply_to'] - if 'attachments' in kwargs: + # Don't process if attachments is None or '' + if 'attachments' in kwargs and kwargs['attachments'] is not None and kwargs['attachments'] != '': files = self.get_files(kwargs['attachments'], study_id) else: @@ -92,7 +94,7 @@ email(subject="My Subject", recipients="user@example.com", attachments=['Study_A print(repr(traceback.format_exception(exc_type, exc_value, exc_traceback))) raise e - return email_model.id + return EmailModelSchema().dump(email_model) def get_email_addresses(self, users, study_id): emails = [] diff --git a/crc/scripts/get_localtime.py b/crc/scripts/get_localtime.py new file mode 100644 index 00000000..83fab223 --- /dev/null +++ b/crc/scripts/get_localtime.py @@ -0,0 +1,34 @@ +from crc.api.common import ApiError +from crc.scripts.script import Script + +import dateparser +import pytz + + +class GetLocaltime(Script): + + def get_description(self): + return """Converts a UTC Datetime object into a Datetime object with a different timezone. + Defaults to US/Eastern""" + + def do_task_validate_only(self, task, study_id, workflow_id, *args, **kwargs): + if 'timestamp' in kwargs: + return True + raise ApiError(code='missing_timestamp', + message='You must include a timestamp to convert.') + + def do_task(self, task, study_id, workflow_id, *args, **kwargs): + if 'timestamp' in kwargs: + timestamp = kwargs['timestamp'] + if 'timezone' in kwargs: + timezone = kwargs['timezone'] + else: + timezone = 'US/Eastern' + parsed_timestamp = dateparser.parse(timestamp) + localtime = parsed_timestamp.astimezone(pytz.timezone(timezone)) + return str(localtime) + + else: + raise ApiError(code='missing_timestamp', + message='You must include a timestamp to convert.') + diff --git a/crc/services/email_service.py b/crc/services/email_service.py index 97c35226..3d3ef120 100644 --- a/crc/services/email_service.py +++ b/crc/services/email_service.py @@ -49,9 +49,11 @@ class EmailService(object): msg.attach(file['name'], file['type'], file_data.data) mail.send(msg) + except Exception as e: app.logger.error('An exception happened in EmailService', exc_info=True) app.logger.error(str(e)) + raise e db.session.add(email_model) db.session.commit() diff --git a/crc/services/file_service.py b/crc/services/file_service.py index 4e9ae6d9..aea6ba61 100644 --- a/crc/services/file_service.py +++ b/crc/services/file_service.py @@ -545,3 +545,49 @@ class FileService(object): dmn_file = prefix + etree.tostring(root) return dmn_file + + @staticmethod + def cleanup_file_data(copies_to_keep=1): + if isinstance(copies_to_keep, int) and copies_to_keep > 0: + + deleted_models = [] + saved_models = [] + current_models = [] + + session.flush() + + workflow_spec_models = session.query(WorkflowSpecModel).all() + + for wf_spec_model in workflow_spec_models: + file_models = session.query(FileModel)\ + .filter(FileModel.workflow_spec_id == wf_spec_model.id)\ + .all() + + for file_model in file_models: + file_data_models = session.query(FileDataModel)\ + .filter(FileDataModel.file_model_id == file_model.id)\ + .order_by(desc(FileDataModel.date_created))\ + .all() + current_models.append(file_data_models[:copies_to_keep]) + for fd_model in file_data_models[copies_to_keep:]: + dependencies = session.query(WorkflowSpecDependencyFile)\ + .filter(WorkflowSpecDependencyFile.file_data_id == fd_model.id)\ + .all() + if len(dependencies) > 0: + saved_models.append(fd_model) + continue + lookups = session.query(LookupFileModel)\ + .filter(LookupFileModel.file_data_model_id == fd_model.id)\ + .all() + if len(lookups) > 0: + saved_models.append(fd_model) + continue + deleted_models.append(fd_model) + session.delete(fd_model) + + session.commit() + return current_models, saved_models, deleted_models + + else: + raise ApiError(code='bad_keep', + message='You must keep at least 1 version') diff --git a/migrations/versions/ba6df7e560a1_modify_email_timestamp.py b/migrations/versions/ba6df7e560a1_modify_email_timestamp.py new file mode 100644 index 00000000..58f870de --- /dev/null +++ b/migrations/versions/ba6df7e560a1_modify_email_timestamp.py @@ -0,0 +1,24 @@ +"""modify email timestamp + +Revision ID: ba6df7e560a1 +Revises: 6d8ceb1c18cb +Create Date: 2021-10-13 10:54:23.894034 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'ba6df7e560a1' +down_revision = '6d8ceb1c18cb' +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute("alter table email alter column timestamp type timestamp with time zone") + + +def downgrade(): + op.execute("alter table email alter column timestamp type timestamp without time zone") diff --git a/tests/data/email/email.bpmn b/tests/data/email/email.bpmn index 40489409..389ba06d 100644 --- a/tests/data/email/email.bpmn +++ b/tests/data/email/email.bpmn @@ -19,15 +19,12 @@ Email content to be delivered to {{ ApprvlApprvr1 }} --- **Test Some Formatting** - _UVA Tracking Number:_ {{ 321 }} - - + _UVA Tracking Number:_ {{ 321 }} Flow_08n2npe Flow_1xlrgne subject="Camunda Email Subject" recipients=[ApprvlApprvr1,PIComputingID] -email_id = email(subject=subject,recipients=recipients) - +email_model = email(subject=subject,recipients=recipients) diff --git a/tests/data/email_attachment_empty_string/email_attachment_empty_string.bpmn b/tests/data/email_attachment_empty_string/email_attachment_empty_string.bpmn new file mode 100644 index 00000000..e93e34a6 --- /dev/null +++ b/tests/data/email_attachment_empty_string/email_attachment_empty_string.bpmn @@ -0,0 +1,84 @@ + + + + + Flow_1synsig + + + # Dear Approver +## you have been requested for approval + + +--- +New request submitted by {{ PIComputingID }} + +Email content to be delivered to {{ ApprvlApprvr1 }} + +--- +**Test Some Formatting** + _UVA Tracking Number:_ {{ 321 }} + Flow_08n2npe + Flow_1ch3gt4 + attachments = '' +email_id = email(subject=subject,recipients=recipients, attachments=attachments) + + + + + + + + + + + Flow_1synsig + Flow_08n2npe + + + + Flow_1gei5cf + + + + # Email +{{ email_id }} + Flow_1ch3gt4 + Flow_1gei5cf + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/email_none_attachment/email_none_attachment.bpmn b/tests/data/email_none_attachment/email_none_attachment.bpmn new file mode 100644 index 00000000..ebfbc897 --- /dev/null +++ b/tests/data/email_none_attachment/email_none_attachment.bpmn @@ -0,0 +1,84 @@ + + + + + Flow_1synsig + + + # Dear Approver +## you have been requested for approval + + +--- +New request submitted by {{ PIComputingID }} + +Email content to be delivered to {{ ApprvlApprvr1 }} + +--- +**Test Some Formatting** + _UVA Tracking Number:_ {{ 321 }} + Flow_08n2npe + Flow_1ch3gt4 + attachments = None +email_id = email(subject=subject,recipients=recipients, attachments=attachments) + + + + + + + + + + + Flow_1synsig + Flow_08n2npe + + + + Flow_1gei5cf + + + + # Email +{{ email_id }} + Flow_1ch3gt4 + Flow_1gei5cf + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/get_localtime/get_localtime.bpmn b/tests/data/get_localtime/get_localtime.bpmn new file mode 100644 index 00000000..43a136d5 --- /dev/null +++ b/tests/data/get_localtime/get_localtime.bpmn @@ -0,0 +1,74 @@ + + + + + Flow_0lnc9x0 + + + + + + Flow_0kgtoh1 + + + + This is my email + Flow_0lnc9x0 + Flow_0gtgzcf + email_model = email(subject='My Email Subject', recipients='user@example.com') + + + timestamp = email_model.timestamp +localtime = get_localtime(str(timestamp)) + Flow_0gtgzcf + Flow_0k1hbif + timestamp=email_model.timestamp +localtime = get_localtime(timestamp=timestamp) + + + # Timestamp +{{ timestamp }} + + +# Localtime +{{ localtime }} + Flow_0k1hbif + Flow_0kgtoh1 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/emails/test_email_script.py b/tests/emails/test_email_script.py index 0b1ec499..0cb1d558 100644 --- a/tests/emails/test_email_script.py +++ b/tests/emails/test_email_script.py @@ -1,6 +1,8 @@ from tests.base_test import BaseTest from crc import mail from crc.models.email import EmailModel +import datetime +from unittest.mock import patch class TestEmailScript(BaseTest): @@ -35,5 +37,78 @@ class TestEmailScript(BaseTest): # Correct From field self.assertEqual('uvacrconnect@virginia.edu', outbox[0].sender) - db_emails = EmailModel.query.count() - self.assertEqual(db_emails, 1) + # Make sure we log the email + # Grab model for comparison below + db_emails = EmailModel.query.all() + self.assertEqual(len(db_emails), 1) + + # Check whether we get a good email model back from the script + self.assertIn('email_model', workflow_api.next_task.data) + self.assertEqual(db_emails[0].recipients, workflow_api.next_task.data['email_model']['recipients']) + self.assertEqual(db_emails[0].sender, workflow_api.next_task.data['email_model']['sender']) + self.assertEqual(db_emails[0].subject, workflow_api.next_task.data['email_model']['subject']) + + # Make sure timestamp is UTC + self.assertEqual(db_emails[0].timestamp.tzinfo, datetime.timezone.utc) + + @patch('crc.services.email_service.EmailService.add_email') + def test_email_raises_exception(self, mock_response): + self.load_example_data() + mock_response.return_value.ok = True + mock_response.side_effect = Exception("This is my exception!") + + workflow = self.create_workflow('email') + + task_data = { + 'PIComputingID': 'dhf8r@virginia.edu', + 'ApprvlApprvr1': 'lb3dp@virginia.edu' + } + task = self.get_workflow_api(workflow).next_task + + with mail.record_messages() as outbox: + with self.assertRaises(Exception) as e: + self.complete_form(workflow, task, task_data) + + def test_email_with_none_attachment(self): + # This workflow specifically sends `attachments = None` as a parameter + # to the email script + workflow = self.create_workflow('email_none_attachment') + workflow_api = self.get_workflow_api(workflow) + task = workflow_api.next_task + + with mail.record_messages() as outbox: + workflow_api = self.complete_form(workflow, task, {'subject': 'My Test Email', + 'recipients': 'user@example.com'}) + task = workflow_api.next_task + # Make sure 'attachments' is in task.data, and is None + self.assertIn('attachments', task.data) + self.assertEqual(task.data['attachments'], None) + + # Make sure we still send an email + self.assertIn('email_id', task.data) + + self.assertEqual(len(outbox), 1) + self.assertEqual(outbox[0].subject, "My Test Email") + self.assertEqual(outbox[0].recipients, ['user@example.com']) + + def test_email_attachment_empty_string(self): + # This workflow specifically sends `attachments = ''` as a parameter + # to the email script + workflow = self.create_workflow('email_attachment_empty_string') + workflow_api = self.get_workflow_api(workflow) + task = workflow_api.next_task + + with mail.record_messages() as outbox: + workflow_api = self.complete_form(workflow, task, {'subject': 'My Test Email', + 'recipients': 'user@example.com'}) + task = workflow_api.next_task + # Make sure 'attachments' is in task.data, and is None + self.assertIn('attachments', task.data) + self.assertEqual(task.data['attachments'], '') + + # Make sure we still send an email + self.assertIn('email_id', task.data) + + self.assertEqual(len(outbox), 1) + self.assertEqual(outbox[0].subject, "My Test Email") + self.assertEqual(outbox[0].recipients, ['user@example.com']) diff --git a/tests/emails/test_email_service.py b/tests/emails/test_email_service.py index 1cb36525..b045118f 100644 --- a/tests/emails/test_email_service.py +++ b/tests/emails/test_email_service.py @@ -3,6 +3,7 @@ from tests.base_test import BaseTest from crc import session from crc.models.email import EmailModel from crc.services.email_service import EmailService +from unittest.mock import patch class TestEmailService(BaseTest): @@ -10,7 +11,6 @@ class TestEmailService(BaseTest): def test_add_email(self): self.load_example_data() study = self.create_study() - workflow = self.create_workflow('random_fact') subject = 'Email Subject' sender = 'sender@sartography.com' @@ -42,3 +42,25 @@ class TestEmailService(BaseTest): self.assertEqual(email_model.content, content) self.assertEqual(email_model.content_html, content_html) self.assertEqual(email_model.study, None) + + @patch('crc.services.email_service.EmailService.add_email') + def test_add_email_with_error(self, mock_response): + + self.load_example_data() + mock_response.return_value.ok = True + mock_response.side_effect = Exception("This is my exception!") + + study = self.create_study() + + subject = 'Email Subject' + sender = 'sender@sartography.com' + recipients = ['recipient@sartography.com', 'back@sartography.com'] + content = 'Content for this email' + content_html = '

Hypertext Markup Language content for this email

' + + # Make sure we generate an error + with self.assertRaises(Exception) as e: + EmailService.add_email(subject=subject, sender=sender, recipients=recipients, + content=content, content_html=content_html, study_id=study.id) + # Make sure it's the error we want + self.assertEqual('This is my exception!', e.exception.args[0]) diff --git a/tests/files/test_file_data_cleanup.py b/tests/files/test_file_data_cleanup.py new file mode 100644 index 00000000..96110d09 --- /dev/null +++ b/tests/files/test_file_data_cleanup.py @@ -0,0 +1,149 @@ +from tests.base_test import BaseTest + +from crc import session +from crc.models.file import FileModel, FileDataModel, LookupFileModel +from crc.models.workflow import WorkflowSpecModel, WorkflowSpecDependencyFile +from crc.services.file_service import FileService + +from sqlalchemy import desc + +import io +import json + + +class TestFileDataCleanup(BaseTest): + + xml_str_one = b""" + + + + + + + + + + + + """ + + xml_str_two = b""" + + + + Flow_1v0s5ht + + + # Hello + Flow_1v0s5ht + Flow_12k5ua1 + + + + Flow_12k5ua1 + + + + + + + + + + + + + + + + + + + + + + + + + + """ + + def test_file_data_cleanup(self): + """Update a file twice. Make sure we clean up the correct files""" + + self.load_example_data() + workflow = self.create_workflow('empty_workflow') + file_data_model_count = session.query(FileDataModel).count() + + # Use for comparison after cleanup + replaced_models = [] + + # Get `empty_workflow` workflow spec + workflow_spec_model = session.query(WorkflowSpecModel)\ + .filter(WorkflowSpecModel.name == 'empty_workflow')\ + .first() + + # Get file model for empty_workflow spec + file_model = session.query(FileModel)\ + .filter(FileModel.workflow_spec_id == workflow_spec_model.id)\ + .first() + + # Grab the file data model for empty_workflow file_model + original_file_data_model = session.query(FileDataModel)\ + .filter(FileDataModel.file_model_id == file_model.id)\ + .order_by(desc(FileDataModel.date_created))\ + .first() + + # Add file to dependencies + # It should not get deleted + wf_spec_depend_model = WorkflowSpecDependencyFile(file_data_id=original_file_data_model.id, + workflow_id=workflow.id) + session.add(wf_spec_depend_model) + session.commit() + + # Update first time + replaced_models.append(original_file_data_model) + data = {'file': (io.BytesIO(self.xml_str_one), file_model.name)} + rv = self.app.put('/v1.0/file/%i/data' % file_model.id, data=data, follow_redirects=True, + content_type='multipart/form-data', headers=self.logged_in_headers()) + self.assert_success(rv) + file_json_first = json.loads(rv.get_data(as_text=True)) + + # Update second time + # replaced_models.append(old_file_data_model) + data = {'file': (io.BytesIO(self.xml_str_two), file_model.name)} + rv = self.app.put('/v1.0/file/%i/data' % file_model.id, data=data, follow_redirects=True, + content_type='multipart/form-data', headers=self.logged_in_headers()) + self.assert_success(rv) + file_json_second = json.loads(rv.get_data(as_text=True)) + + # Add lookup file + data = {'file': (io.BytesIO(b'asdf'), 'lookup_1.xlsx')} + rv = self.app.post('/v1.0/file?workflow_spec_id=%s' % workflow_spec_model.id, data=data, follow_redirects=True, + content_type='multipart/form-data', headers=self.logged_in_headers()) + self.assert_success(rv) + file_json = json.loads(rv.get_data(as_text=True)) + lookup_file_id = file_json['id'] + lookup_data_model = session.query(FileDataModel).filter(FileDataModel.file_model_id == lookup_file_id).first() + lookup_model = LookupFileModel(file_data_model_id=lookup_data_model.id, + workflow_spec_id=workflow_spec_model.id) + session.add(lookup_model) + session.commit() + + # Update lookup file + data = {'file': (io.BytesIO(b'1234'), 'lookup_1.xlsx')} + rv = self.app.put('/v1.0/file/%i/data' % lookup_file_id, data=data, follow_redirects=True, + content_type='multipart/form-data', headers=self.logged_in_headers()) + self.assert_success(rv) + + # Run the cleanup files process + current_models, saved_models, deleted_models = FileService.cleanup_file_data() + + # assert correct versions are removed + new_count = session.query(FileDataModel).count() + self.assertEqual(8, new_count) + self.assertEqual(4, len(current_models)) + self.assertEqual(2, len(saved_models)) + self.assertEqual(1, len(deleted_models)) + + print('test_file_data_cleanup') diff --git a/tests/scripts/test_get_localtime.py b/tests/scripts/test_get_localtime.py new file mode 100644 index 00000000..7e5b489b --- /dev/null +++ b/tests/scripts/test_get_localtime.py @@ -0,0 +1,17 @@ +from tests.base_test import BaseTest +from crc.scripts.get_localtime import GetLocaltime + + +class TestGetLocaltime(BaseTest): + + def test_get_localtime(self): + self.load_example_data() + + workflow = self.create_workflow('get_localtime') + workflow_api = self.get_workflow_api(workflow) + task = workflow_api.next_task + + timestamp = task.data['timestamp'] + localtime = task.data['localtime'] + + self.assertEqual(localtime, GetLocaltime().do_task(None, None, None, timestamp=timestamp))