diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 1cd97dd98..761e01543 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -37,7 +37,7 @@ from app.dao.notifications_dao import ( dao_update_notification, delete_notifications_created_more_than_a_week_ago, dao_get_notification_statistics_for_service_and_day, - update_notification_reference_by_id + update_notification_after_sent_to_provider ) from app.dao.jobs_dao import ( @@ -268,6 +268,12 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): reference=str(notification_id) ) + update_notification_after_sent_to_provider( + notification_id, + 'sms', + provider.get_name() + ) + except SmsClientException as e: current_app.logger.error( "SMS notification {} failed".format(notification_id) @@ -343,7 +349,12 @@ def send_email(service_id, notification_id, from_address, encrypted_notification reply_to_addresses=reply_to_addresses, ) - update_notification_reference_by_id(notification_id, reference) + update_notification_after_sent_to_provider( + notification_id, + 'email', + provider.get_name(), + reference=reference + ) except EmailClientException as e: current_app.logger.exception(e) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index f93e062ad..c5db52329 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -176,22 +176,6 @@ def dao_create_notification(notification, notification_type, provider_identifier service_id=notification.service_id) db.session.add(template_stats) - update_count = db.session.query(ProviderStatistics).filter_by( - day=date.today(), - service_id=notification.service_id, - provider_id=provider.id - ).update({'unit_count': ProviderStatistics.unit_count + ( - 1 if notification_type == TEMPLATE_TYPE_EMAIL else get_sms_fragment_count(notification.content_char_count))}) - - if update_count == 0: - provider_stats = ProviderStatistics( - day=notification.created_at.date(), - service_id=notification.service_id, - provider_id=provider.id, - unit_count=1 if notification_type == TEMPLATE_TYPE_EMAIL else get_sms_fragment_count( - notification.content_char_count)) - db.session.add(provider_stats) - db.session.add(notification) @@ -293,14 +277,36 @@ def dao_update_notification(notification): db.session.commit() -def update_notification_reference_by_id(id, reference): - count = db.session.query(Notification).filter_by( - id=id - ).update({ - Notification.reference: reference - }) - db.session.commit() - return count +@transactional +def update_notification_after_sent_to_provider(id_, notification_type, provider_name, reference=None): + provider = ProviderDetails.query.filter_by(identifier=provider_name).one() + notification = Notification.query.filter(Notification.id == id_).one() + + def unit_count(): + if notification_type == TEMPLATE_TYPE_EMAIL: + return 1 + else: + return get_sms_fragment_count(notification.content_char_count) + + update_count = db.session.query(ProviderStatistics).filter_by( + day=date.today(), + service_id=notification.service_id, + provider_id=provider.id + ).update({'unit_count': ProviderStatistics.unit_count + unit_count()}) + + if update_count == 0: + provider_stats = ProviderStatistics( + day=notification.created_at.date(), + service_id=notification.service_id, + provider_id=provider.id, + unit_count=unit_count() + ) + + db.session.add(provider_stats) + + if reference: + notification.reference = reference + db.session.add(notification) def get_notification_for_job(service_id, job_id, notification_id): diff --git a/tests/app/conftest.py b/tests/app/conftest.py index fc3d31dea..a0e002f17 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -335,7 +335,7 @@ def sample_notification(notify_db, notification_id = uuid.uuid4() if provider_name is None: - provider = mmg_provider() if template.template_type == 'sms' else ses_provider() + provider_name = mmg_provider().identifier if template.template_type == 'sms' else ses_provider().identifier if to_field: to = to_field @@ -359,7 +359,7 @@ def sample_notification(notify_db, data['job_row_number'] = job_row_number notification = Notification(**data) if create: - dao_create_notification(notification, template.template_type, provider.identifier) + dao_create_notification(notification, template.template_type, provider_name) return notification diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 1a55e3849..a26dbbaa6 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -25,7 +25,7 @@ from app.dao.notifications_dao import ( delete_notifications_created_more_than_a_week_ago, dao_get_notification_statistics_for_service_and_day, update_notification_status_by_id, - update_notification_reference_by_id, + update_notification_after_sent_to_provider, update_notification_status_by_reference, dao_get_template_statistics_for_service, get_notifications_for_service @@ -36,10 +36,13 @@ from notifications_utils.template import get_sms_fragment_count from tests.app.conftest import (sample_notification) -def test_should_by_able_to_update_reference_by_id(sample_notification): +def test_should_by_able_to_update_reference_by_notification_id(sample_notification, mmg_provider): assert not Notification.query.get(sample_notification.id).reference - count = update_notification_reference_by_id(sample_notification.id, 'reference') - assert count == 1 + update_notification_after_sent_to_provider( + sample_notification.id, + 'sms', + mmg_provider.identifier, + reference='reference') assert Notification.query.get(sample_notification.id).reference == 'reference' @@ -53,7 +56,11 @@ def test_should_by_able_to_update_status_by_reference(sample_email_template, ses ses_provider.identifier) assert Notification.query.get(notification.id).status == "sending" - update_notification_reference_by_id(notification.id, 'reference') + update_notification_after_sent_to_provider( + notification.id, + 'email', + ses_provider.identifier, + reference="reference") update_notification_status_by_reference('reference', 'delivered', 'delivered') assert Notification.query.get(notification.id).status == 'delivered' _assert_notification_stats(notification.service_id, emails_delivered=1, emails_requested=1, emails_failed=0) @@ -140,7 +147,11 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em ses_provider.identifier) assert Notification.query.get(notification.id).status == "sending" - update_notification_reference_by_id(notification.id, 'reference') + update_notification_after_sent_to_provider( + notification.id, + 'email', + ses_provider.identifier, + reference='reference') update_notification_status_by_reference('reference', 'delivered', 'delivered') assert Notification.query.get(notification.id).status == 'delivered' @@ -163,7 +174,11 @@ def test_should_be_able_to_record_statistics_failure_for_email(sample_email_temp notification = Notification(**data) dao_create_notification(notification, sample_email_template.template_type, ses_provider.identifier) - update_notification_reference_by_id(notification.id, 'reference') + update_notification_after_sent_to_provider( + notification.id, + 'email', + ses_provider.identifier, + reference='reference') count = update_notification_status_by_reference('reference', 'failed', 'failure') assert count == 1 assert Notification.query.get(notification.id).status == 'failed' diff --git a/tests/app/dao/test_notifications_dao_provider_statistics.py b/tests/app/dao/test_notifications_dao_provider_statistics.py index cc166edd1..24d2fc6dd 100644 --- a/tests/app/dao/test_notifications_dao_provider_statistics.py +++ b/tests/app/dao/test_notifications_dao_provider_statistics.py @@ -1,6 +1,6 @@ from datetime import (date, timedelta) from app.models import ProviderStatistics - +from app.dao.notifications_dao import update_notification_after_sent_to_provider from app.dao.provider_statistics_dao import ( get_provider_statistics, get_fragment_count) from tests.app.conftest import sample_notification as create_sample_notification @@ -10,10 +10,11 @@ def test_should_update_provider_statistics_sms(notify_db, notify_db_session, sample_template, mmg_provider): - create_sample_notification( + n1 = create_sample_notification( notify_db, notify_db_session, template=sample_template) + update_notification_after_sent_to_provider(n1.id, 'sms', mmg_provider.identifier) provider_stats = get_provider_statistics( sample_template.service, providers=[mmg_provider.identifier]).one() @@ -24,10 +25,11 @@ def test_should_update_provider_statistics_email(notify_db, notify_db_session, sample_email_template, ses_provider): - create_sample_notification( + n1 = create_sample_notification( notify_db, notify_db_session, template=sample_email_template) + update_notification_after_sent_to_provider(n1.id, 'email', ses_provider.identifier, reference="reference") provider_stats = get_provider_statistics( sample_email_template.service, providers=[ses_provider.identifier]).one() @@ -38,21 +40,27 @@ def test_should_update_provider_statistics_sms_multi(notify_db, notify_db_session, sample_template, mmg_provider): - create_sample_notification( + n1 = create_sample_notification( notify_db, notify_db_session, template=sample_template, + provider_name=mmg_provider.identifier, content_char_count=160) - create_sample_notification( + update_notification_after_sent_to_provider(n1.id, 'sms', mmg_provider.identifier) + n2 = create_sample_notification( notify_db, notify_db_session, template=sample_template, + provider_name=mmg_provider.identifier, content_char_count=161) - create_sample_notification( + update_notification_after_sent_to_provider(n2.id, 'sms', mmg_provider.identifier) + n3 = create_sample_notification( notify_db, notify_db_session, template=sample_template, + provider_name=mmg_provider.identifier, content_char_count=307) + update_notification_after_sent_to_provider(n3.id, 'sms', mmg_provider.identifier) provider_stats = get_provider_statistics( sample_template.service, providers=[mmg_provider.identifier]).one() @@ -63,18 +71,24 @@ def test_should_update_provider_statistics_email_multi(notify_db, notify_db_session, sample_email_template, ses_provider): - create_sample_notification( + n1 = create_sample_notification( notify_db, notify_db_session, - template=sample_email_template) - create_sample_notification( + template=sample_email_template, + provider_name=ses_provider.identifier) + update_notification_after_sent_to_provider(n1.id, 'email', ses_provider.identifier, reference="reference") + n2 = create_sample_notification( notify_db, notify_db_session, - template=sample_email_template) - create_sample_notification( + template=sample_email_template, + provider_name=ses_provider.identifier) + update_notification_after_sent_to_provider(n2.id, 'email', ses_provider.identifier, reference="reference") + n3 = create_sample_notification( notify_db, notify_db_session, - template=sample_email_template) + template=sample_email_template, + provider_name=ses_provider.identifier) + update_notification_after_sent_to_provider(n3.id, 'email', ses_provider.identifier, reference="reference") provider_stats = get_provider_statistics( sample_email_template.service, providers=[ses_provider.identifier]).one()