diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index c599313ab..071ba3065 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -48,28 +48,29 @@ def retry_iteration_to_delay(retry=0): @notify_celery.task(bind=True, name="send-sms-to-provider", max_retries=5, default_retry_delay=5) -def send_sms_to_provider(self, service_id, notification_id, encrypted_notification): +def send_sms_to_provider(self, service_id, notification_id, encrypted_notification=None): task_start = monotonic() service = dao_fetch_service_by_id(service_id) provider = provider_to_use('sms', notification_id) notification = get_notification_by_id(notification_id) - notification_json = encryption.decrypt(encrypted_notification) + # notification_json = encryption.decrypt(encrypted_notification) template = Template( dao_get_template_by_id(notification.template_id, notification.template_version).__dict__, - values=notification_json.get('personalisation', {}), + values=notification.personalisation['personalisation'] if notification.personalisation[ + 'personalisation'] else {}, prefix=service.name ) try: if service.research_mode: send_sms_response.apply_async( - (provider.get_name(), str(notification_id), notification_json['to']), queue='research-mode' + (provider.get_name(), str(notification_id), notification.to), queue='research-mode' ) else: provider.send_sms( - to=validate_and_format_phone_number(notification_json['to']), + to=validate_and_format_phone_number(notification.to), content=template.replaced, reference=str(notification_id) ) @@ -84,6 +85,7 @@ def send_sms_to_provider(self, service_id, notification_id, encrypted_notificati notification.sent_at = datetime.utcnow() notification.sent_by = provider.get_name(), notification.content_char_count = template.replaced_content_count + notification.status = 'sending' dao_update_notification(notification) except SmsClientException as e: try: diff --git a/app/models.py b/app/models.py index 6d8ed05e5..cca4b7d32 100644 --- a/app/models.py +++ b/app/models.py @@ -348,7 +348,7 @@ class Notification(db.Model): nullable=True, onupdate=datetime.datetime.utcnow) status = db.Column( - db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type'), nullable=False, default='sending') + db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type'), nullable=False, default='created') reference = db.Column(db.String, nullable=True, index=True) _personalisation = db.Column(db.String, nullable=True) diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index f4a899ffd..767b40413 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -83,7 +83,8 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( notify_db_session, sample_template_with_placeholders, mocker): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders) + db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders, + to_field="+447234123123", personalisation={"name": "Jo"}) notification = _notification_json( sample_template_with_placeholders, @@ -124,13 +125,12 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( call("notifications.sms.total-time", ANY) ]) - notification = notifications_dao.get_notification( - db_notification.service_id, db_notification.id - ) + notification = Notification.query.filter_by(id=db_notification.id).one() assert notification.status == 'sending' assert notification.sent_at > now assert notification.sent_by == 'mmg' + assert notification.content_char_count == 24 def test_should_send_template_to_correct_sms_provider_and_persist( diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 4e6a2e9f3..181af44f0 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -39,7 +39,7 @@ from tests.app.conftest import (sample_notification) def test_should_by_able_to_update_status_by_reference(sample_email_template, ses_provider): - data = _notification_json(sample_email_template) + data = _notification_json(sample_email_template, status='sending') notification = Notification(**data) dao_create_notification( @@ -56,7 +56,7 @@ def test_should_by_able_to_update_status_by_reference(sample_email_template, ses def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_provider): - data = _notification_json(sample_template, job_id=sample_job.id) + data = _notification_json(sample_template, job_id=sample_job.id, status='sending') notification = Notification(**data) dao_create_notification(notification, sample_template.template_type) assert Notification.query.get(notification.id).status == 'sending' @@ -76,7 +76,7 @@ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(n def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_template, sample_job): - data = _notification_json(sample_template, job_id=sample_job.id) + data = _notification_json(sample_template, job_id=sample_job.id, status='sending') notification = Notification(**data) dao_create_notification(notification, sample_template.template_type) assert Notification.query.get(notification.id).status == 'sending' @@ -92,7 +92,7 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_ def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure(sample_template, sample_job): - data = _notification_json(sample_template, job_id=sample_job.id) + data = _notification_json(sample_template, job_id=sample_job.id, status='sending') notification = Notification(**data) dao_create_notification(notification, sample_template.template_type) assert Notification.query.get(notification.id).status == 'sending' @@ -111,7 +111,7 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure def test_should_by_able_to_update_status_by_id_from_sending_to_permanent_failure(sample_template, sample_job): - data = _notification_json(sample_template, job_id=sample_job.id) + data = _notification_json(sample_template, job_id=sample_job.id, status='sending') notification = Notification(**data) dao_create_notification(notification, sample_template.template_type) assert Notification.query.get(notification.id).status == 'sending' @@ -126,15 +126,14 @@ def test_should_by_able_to_update_status_by_id_from_sending_to_permanent_failure _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=1) -def test_should_not_update_status_one_notification_status_is_delivered(sample_email_template, sample_job, ses_provider): - data = _notification_json(sample_email_template, job_id=sample_job.id) - - notification = Notification(**data) - dao_create_notification( - notification, - sample_email_template.template_type) - +def test_should_not_update_status_one_notification_status_is_delivered(notify_db, notify_db_session, + sample_email_template, + ses_provider): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template, + status='sending') assert Notification.query.get(notification.id).status == "sending" + update_provider_stats( notification.id, 'email', @@ -150,16 +149,18 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0) -def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification): - assert Notification.query.get(sample_notification.id).status == 'sending' - assert update_notification_status_by_id(sample_notification.id, 'permanent-failure', 'failure') - assert Notification.query.get(sample_notification.id).status == 'permanent-failure' - _assert_notification_stats(sample_notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=1) - _assert_job_stats(sample_notification.job_id, sent=1, count=1, delivered=0, failed=1) +def test_should_be_able_to_record_statistics_failure_for_sms(notify_db, notify_db_session,): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, status='sending') + assert Notification.query.get(notification.id).status == 'sending' + + assert update_notification_status_by_id(notification.id, 'permanent-failure', 'failure') + assert Notification.query.get(notification.id).status == 'permanent-failure' + _assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=1) + _assert_job_stats(notification.job_id, sent=1, count=1, delivered=0, failed=1) def test_should_be_able_to_record_statistics_failure_for_email(sample_email_template, sample_job, ses_provider): - data = _notification_json(sample_email_template, job_id=sample_job.id) + data = _notification_json(sample_email_template, job_id=sample_job.id, status='sending') notification = Notification(**data) dao_create_notification(notification, sample_email_template.template_type) @@ -295,6 +296,46 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_pre assert stats[1].day == seven_days_ago.date() +def test_create_notification_creates_notification_with_personalisation(notify_db, notify_db_session, + sample_template_with_placeholders, + sample_job, mmg_provider): + + assert Notification.query.count() == 0 + assert NotificationStatistics.query.count() == 0 + assert TemplateStatistics.query.count() == 0 + + data = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_template_with_placeholders, + job=sample_job, + personalisation={'name': 'Jo'}, + status='created') + + assert Notification.query.count() == 1 + notification_from_db = Notification.query.all()[0] + assert notification_from_db.id + assert data.to == notification_from_db.to + assert data.job_id == notification_from_db.job_id + assert data.service == notification_from_db.service + assert data.template == notification_from_db.template + assert data.template_version == notification_from_db.template_version + assert data.created_at == notification_from_db.created_at + assert 'created' == notification_from_db.status + assert {'name': 'Jo'} == notification_from_db.personalisation + _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=0) + + stats = NotificationStatistics.query.filter( + NotificationStatistics.service_id == sample_template_with_placeholders.service.id).first() + assert stats.emails_requested == 0 + assert stats.sms_requested == 1 + + template_stats = TemplateStatistics.query.filter( + TemplateStatistics.service_id == sample_template_with_placeholders.service.id, + TemplateStatistics.template_id == sample_template_with_placeholders.id).first() + assert template_stats.service_id == sample_template_with_placeholders.service.id + assert template_stats.template_id == sample_template_with_placeholders.id + assert template_stats.usage_count == 1 + + def test_save_notification_creates_sms_and_template_stats(sample_template, sample_job, mmg_provider): assert Notification.query.count() == 0 assert NotificationStatistics.query.count() == 0 @@ -314,7 +355,7 @@ def test_save_notification_creates_sms_and_template_stats(sample_template, sampl assert data['template'] == notification_from_db.template assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at - assert 'sending' == notification_from_db.status + assert 'created' == notification_from_db.status _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=0) stats = NotificationStatistics.query.filter(NotificationStatistics.service_id == sample_template.service.id).first() @@ -347,7 +388,7 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp assert data['template'] == notification_from_db.template assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at - assert 'sending' == notification_from_db.status + assert 'created' == notification_from_db.status _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=0) stats = NotificationStatistics.query.filter( @@ -484,7 +525,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr assert data['template'] == notification_from_db.template assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at - assert 'sending' == notification_from_db.status + assert 'created' == notification_from_db.status assert Job.query.get(sample_job.id).notifications_sent == 1 notification_2 = Notification(**data) @@ -535,7 +576,7 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio assert data['template'] == notification_from_db.template assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at - assert 'sending' == notification_from_db.status + assert 'created' == notification_from_db.status assert job_1.id != job_2.id _assert_job_stats(job_id=job_1.id, sent=1, count=1) _assert_job_stats(job_id=job_2.id, sent=0, count=1) @@ -556,7 +597,7 @@ def test_save_notification_with_no_job(sample_template, mmg_provider): assert data['template'] == notification_from_db.template assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at - assert 'sending' == notification_from_db.status + assert 'created' == notification_from_db.status def test_get_notification(sample_notification): @@ -580,7 +621,8 @@ def test_save_notification_no_job_id(sample_template, mmg_provider): assert data['service'] == notification_from_db.service assert data['template'] == notification_from_db.template assert data['template_version'] == notification_from_db.template_version - assert 'sending' == notification_from_db.status + assert 'created' == notification_from_db.status + assert data.get('job_id') is None def test_get_notification_for_job(sample_notification): @@ -917,7 +959,7 @@ def test_sms_fragment_count(char_count, expected_sms_fragment_count): assert get_sms_fragment_count(char_count) == expected_sms_fragment_count -def _notification_json(sample_template, job_id=None, id=None): +def _notification_json(sample_template, job_id=None, id=None, status=None): data = { 'to': '+44709123456', 'service': sample_template.service, @@ -926,12 +968,14 @@ def _notification_json(sample_template, job_id=None, id=None): 'template_id': sample_template.id, 'template_version': sample_template.version, 'created_at': datetime.utcnow(), - 'content_char_count': 160 + 'content_char_count': 160, } if job_id: data.update({'job_id': job_id}) if id: data.update({'id': id}) + if status: + data.update({'status': status}) return data