diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 778ff8b29..dde08958e 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -17,11 +17,9 @@ from app.dao.jobs_dao import ( dao_update_job, dao_get_job_by_id ) -from app.dao.notifications_dao import dao_create_notification from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count from app.dao.templates_dao import dao_get_template_by_id from app.models import ( - Notification, EMAIL_TYPE, SMS_TYPE, KEY_TYPE_NORMAL @@ -140,6 +138,7 @@ def send_sms(self, notification_type=SMS_TYPE, api_key_id=api_key_id, key_type=key_type, + created_at=created_at, job_id=notification.get('job', None), job_row_number=notification.get('row_number', None), ) @@ -189,8 +188,10 @@ def send_email(self, service_id, notification_type=EMAIL_TYPE, api_key_id=api_key_id, key_type=key_type, + created_at=created_at, job_id=notification.get('job', None), job_row_number=notification.get('row_number', None), + ) provider_tasks.deliver_email.apply_async( diff --git a/app/models.py b/app/models.py index fb6673ab2..fadd057d3 100644 --- a/app/models.py +++ b/app/models.py @@ -538,56 +538,31 @@ class Notification(db.Model): self._personalisation = encryption.encrypt(personalisation) @classmethod - def from_api_request( - cls, - created_at, - notification, - notification_id, - service_id, - notification_type, - api_key_id, - key_type): - return cls( - id=notification_id, - template_id=notification['template'], - template_version=notification['template_version'], - to=notification['to'], - service_id=service_id, - job_id=notification.get('job', None), - job_row_number=notification.get('row_number', None), - status='created', - created_at=datetime.datetime.strptime(created_at, DATETIME_FORMAT), - personalisation=notification.get('personalisation'), - notification_type=notification_type, - api_key_id=api_key_id, - key_type=key_type - ) - - @classmethod - def from_v2_api_request(cls, - template_id, - template_version, - recipient, - service_id, - personalisation, - notification_type, - api_key_id, - key_type, - job_id, - job_row_number): + def from_request(cls, + template_id, + template_version, + recipient, + service_id, + personalisation, + notification_type, + api_key_id, + key_type, + job_id, + job_row_number, + created_at): return cls( template_id=template_id, template_version=template_version, to=recipient, service_id=service_id, status='created', - created_at=datetime.datetime.utcnow(), + created_at=created_at, personalisation=personalisation, notification_type=notification_type, api_key_id=api_key_id, key_type=key_type, job_id=job_id, - job_row_number=job_row_number, + job_row_number=job_row_number ) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 91943d9fb..ea08144c7 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -1,7 +1,10 @@ +from datetime import datetime + from flask import current_app from notifications_utils.renderers import PassThrough from notifications_utils.template import Template +from app import DATETIME_FORMAT from app.celery import provider_tasks from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE @@ -41,19 +44,22 @@ def persist_notification(template_id, notification_type, api_key_id, key_type, + created_at=None, job_id=None, job_row_number=None): - notification = Notification.from_v2_api_request(template_id=template_id, - template_version=template_version, - recipient=recipient, - service_id=service_id, - personalisation=personalisation, - notification_type=notification_type, - api_key_id=api_key_id, - key_type=key_type, - job_id=job_id, - job_row_number=job_row_number - ) + notification = Notification.from_request( + template_id=template_id, + template_version=template_version, + recipient=recipient, + service_id=service_id, + personalisation=personalisation, + notification_type=notification_type, + api_key_id=api_key_id, + key_type=key_type, + created_at=created_at if created_at else datetime.utcnow().strftime(DATETIME_FORMAT), + job_id=job_id, + job_row_number=job_row_number + ) dao_create_notification(notification) return notification diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 307d33132..2b9086a64 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -3,7 +3,6 @@ import pytest from datetime import datetime from freezegun import freeze_time from sqlalchemy.exc import SQLAlchemyError -from sqlalchemy.orm.exc import NoResultFound from app import (encryption, DATETIME_FORMAT) from app.celery import provider_tasks from app.celery import tasks @@ -189,8 +188,6 @@ def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(noti mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email')) mocker.patch('app.celery.tasks.send_email.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") - mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") process_job(job.id) @@ -208,8 +205,6 @@ def test_should_not_process_email_job_if_would_exceed_send_limits(notify_db, not mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.send_email.apply_async') - mocker.patch('app.encryption.encrypt', return_value="something_encrypted") - mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") process_job(job.id) @@ -562,11 +557,12 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_ mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() + now = datetime.utcnow() send_sms( sample_job.service.id, notification_id, encryption.encrypt(notification), - datetime.utcnow().strftime(DATETIME_FORMAT), + now.strftime(DATETIME_FORMAT), api_key_id=str(sample_api_key.id), key_type=KEY_TYPE_NORMAL ) @@ -581,7 +577,7 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_ assert persisted_notification.template_id == sample_job.template.id assert persisted_notification.status == 'created' assert not persisted_notification.sent_at - assert persisted_notification.created_at <= datetime.utcnow() + assert persisted_notification.created_at <= now assert not persisted_notification.sent_by assert persisted_notification.job_row_number == 2 assert persisted_notification.api_key_id == sample_api_key.id @@ -673,7 +669,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh assert persisted_notification.to == 'my_email@my_email.com' assert persisted_notification.template_id == sample_email_template_with_placeholders.id assert persisted_notification.template_version == sample_email_template_with_placeholders.version - assert persisted_notification.created_at >= now + assert persisted_notification.created_at == now assert not persisted_notification.sent_at assert persisted_notification.status == 'created' assert not persisted_notification.sent_by @@ -711,7 +707,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email assert persisted_notification.to == 'my_email@my_email.com' assert persisted_notification.template_id == sample_email_template.id assert persisted_notification.template_version == version_on_notification - assert persisted_notification.created_at >= now + assert persisted_notification.created_at == now assert not persisted_notification.sent_at assert persisted_notification.status == 'created' assert not persisted_notification.sent_by @@ -739,7 +735,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi assert persisted_notification.to == 'my_email@my_email.com' assert persisted_notification.template_id == sample_email_template_with_placeholders.id assert persisted_notification.status == 'created' - assert persisted_notification.created_at >= now + assert persisted_notification.created_at == now assert not persisted_notification.sent_by assert persisted_notification.personalisation == {"name": "Jo"} assert not persisted_notification.reference @@ -766,7 +762,7 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em persisted_notification = Notification.query.all()[0] assert persisted_notification.to == 'my_email@my_email.com' assert persisted_notification.template_id == sample_email_template.id - assert persisted_notification.created_at >= now + assert persisted_notification.created_at == now assert not persisted_notification.sent_at assert persisted_notification.status == 'created' assert not persisted_notification.sent_by diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index ddc559201..851865a6b 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -1,3 +1,5 @@ +import datetime + import pytest from boto3.exceptions import Boto3Error from sqlalchemy.exc import SQLAlchemyError @@ -46,21 +48,44 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api assert NotificationHistory.query.count() == 1 -def test_persist_notification_throws_exception_when_missing_template(sample_template, sample_api_key): +def test_persist_notification_throws_exception_when_missing_template(sample_api_key): assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 with pytest.raises(SQLAlchemyError): persist_notification(template_id=None, template_version=None, recipient='+447111111111', - service_id=sample_template.service.id, - personalisation=None, notification_type='sms', + service_id=sample_api_key.service_id, + personalisation=None, + notification_type='sms', api_key_id=sample_api_key.id, key_type=sample_api_key.key_type) assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 +def test_persist_notification_with_job_and_created(sample_job, sample_api_key): + assert Notification.query.count() == 0 + assert NotificationHistory.query.count() == 0 + created_at = datetime.datetime(2016, 11, 11, 16, 8, 18) + persist_notification(template_id=sample_job.template.id, + template_version=sample_job.template.version, + recipient='+447111111111', + service_id=sample_job.service.id, + personalisation=None, notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + created_at=created_at, + job_id=sample_job.id, + job_row_number=10) + assert Notification.query.count() == 1 + assert NotificationHistory.query.count() == 1 + persisted_notification = Notification.query.all()[0] + assert persisted_notification.job_id == sample_job.id + assert persisted_notification.job_row_number == 10 + assert persisted_notification.created_at == created_at + + @pytest.mark.parametrize('research_mode, queue, notification_type, key_type', [(True, 'research-mode', 'sms', 'normal'), (True, 'research-mode', 'email', 'normal'), diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 78827235a..e6e5877dd 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -1,3 +1,4 @@ +import uuid from datetime import datetime import pytest @@ -12,23 +13,20 @@ from app.models import ( def test_should_build_notification_from_minimal_set_of_api_derived_params(notify_api): now = datetime.utcnow() - notification = { - 'template': 'template', - 'template_version': '1', - 'to': 'someone', - 'personalisation': {} - } - notification = Notification.from_api_request( - created_at=now.strftime(DATETIME_FORMAT), - notification=notification, - notification_id="notification_id", - service_id="service_id", + notification = Notification.from_request( + template_id='template', + template_version='1', + recipient='someone', + service_id='service_id', notification_type='SMS', + created_at=now, api_key_id='api_key_id', - key_type='key_type' + key_type='key_type', + personalisation={}, + job_id=None, + job_row_number=None ) assert notification.created_at == now - assert notification.id == "notification_id" assert notification.template_id == 'template' assert notification.template_version == '1' assert not notification.job_row_number @@ -44,28 +42,22 @@ def test_should_build_notification_from_minimal_set_of_api_derived_params(notify def test_should_build_notification_from_full_set_of_api_derived_params(notify_api): now = datetime.utcnow() - - notification = { - 'template': 'template', - 'template_version': '1', - 'to': 'someone', - 'personalisation': {'key': 'value'}, - 'job': 'job_id', - 'row_number': 100 - } - notification = Notification.from_api_request( - created_at=now.strftime(DATETIME_FORMAT), - notification=notification, - notification_id="notification_id", - service_id="service_id", - notification_type='SMS', - api_key_id='api_key_id', - key_type='key_type' - ) + notification = Notification.from_request(template_id='template', + template_version=1, + recipient='someone', + service_id='service_id', + personalisation={'key': 'value'}, + notification_type='SMS', + api_key_id='api_key_id', + key_type='key_type', + job_id='job_id', + job_row_number=100, + created_at=now + ) assert notification.created_at == now - assert notification.id == "notification_id" + assert notification.id is None assert notification.template_id == 'template' - assert notification.template_version == '1' + assert notification.template_version == 1 assert notification.job_row_number == 100 assert notification.job_id == 'job_id' assert notification.to == 'someone'