diff --git a/app/celery/tasks.py b/app/celery/tasks.py index b20fe59c2..80925b5af 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_after_sent_to_provider + update_provider_stats ) from app.dao.jobs_dao import ( @@ -268,7 +268,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): reference=str(notification_id) ) - update_notification_after_sent_to_provider( + update_provider_stats( notification_id, 'sms', provider.get_name() @@ -349,13 +349,15 @@ def send_email(service_id, notification_id, from_address, encrypted_notification reply_to_addresses=reply_to_addresses, ) - update_notification_after_sent_to_provider( + update_provider_stats( notification_id, 'email', - provider.get_name(), - reference=reference + provider.get_name() ) + notification_db_object.reference = reference + dao_update_notification(notification_db_object) + except EmailClientException as e: current_app.logger.exception(e) notification_db_object.status = 'technical-failure' diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index c5db52329..ac9d597d9 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -278,9 +278,13 @@ def dao_update_notification(notification): @transactional -def update_notification_after_sent_to_provider(id_, notification_type, provider_name, reference=None): - provider = ProviderDetails.query.filter_by(identifier=provider_name).one() +def update_provider_stats( + id_, + notification_type, + provider_name): + notification = Notification.query.filter(Notification.id == id_).one() + provider = ProviderDetails.query.filter_by(identifier=provider_name).one() def unit_count(): if notification_type == TEMPLATE_TYPE_EMAIL: @@ -304,10 +308,6 @@ def update_notification_after_sent_to_provider(id_, notification_type, provider_ 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): return Notification.query.filter_by(service_id=service_id, job_id=job_id, id=notification_id).one() diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index e73c0d391..e0b8c7166 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -518,7 +518,7 @@ def test_should_send_email_if_restricted_service_and_valid_email(notify_db, noti notification = _notification_json(template, "test@restricted.com") mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.send_email', return_value="1234") notification_id = uuid.uuid4() now = datetime.utcnow() @@ -667,7 +667,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker): notification = _notification_json(sample_email_template, 'my_email@my_email.com') mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.send_email', return_value="1234") mocker.patch('app.aws_ses_client.get_name', return_value='ses') version_on_notification = sample_email_template.version # Change the template @@ -710,7 +710,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi notification = _notification_json(sample_email_template_with_placeholders, "my_email@my_email.com", {"name": "Jo"}) mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.send_email', return_value="1234") mocker.patch('app.aws_ses_client.get_name', return_value='ses') notification_id = uuid.uuid4() @@ -1046,7 +1046,6 @@ def test_should_call_send_email_response_task_if_research_mode( sample_email_template, to="john@smith.com" ) - reference = uuid.uuid4() mocker.patch('app.uuid.uuid4', return_value=reference) @@ -1060,6 +1059,7 @@ def test_should_call_send_email_response_task_if_research_mode( notify_db.session.commit() notification_id = uuid.uuid4() + now = datetime.utcnow() send_email( sample_service.id, diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index a26dbbaa6..94991d63b 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_after_sent_to_provider, + update_provider_stats, update_notification_status_by_reference, dao_get_template_statistics_for_service, get_notifications_for_service @@ -38,7 +38,7 @@ from tests.app.conftest import (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 - update_notification_after_sent_to_provider( + update_provider_stats( sample_notification.id, 'sms', mmg_provider.identifier, @@ -56,7 +56,7 @@ 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_after_sent_to_provider( + update_provider_stats( notification.id, 'email', ses_provider.identifier, @@ -147,7 +147,7 @@ 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_after_sent_to_provider( + update_provider_stats( notification.id, 'email', ses_provider.identifier, @@ -174,7 +174,7 @@ 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_after_sent_to_provider( + update_provider_stats( notification.id, 'email', ses_provider.identifier, diff --git a/tests/app/dao/test_notifications_dao_provider_statistics.py b/tests/app/dao/test_notifications_dao_provider_statistics.py index 24d2fc6dd..5a096c114 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.notifications_dao import update_provider_stats from app.dao.provider_statistics_dao import ( get_provider_statistics, get_fragment_count) from tests.app.conftest import sample_notification as create_sample_notification @@ -14,7 +14,7 @@ def test_should_update_provider_statistics_sms(notify_db, notify_db, notify_db_session, template=sample_template) - update_notification_after_sent_to_provider(n1.id, 'sms', mmg_provider.identifier) + update_provider_stats(n1.id, 'sms', mmg_provider.identifier) provider_stats = get_provider_statistics( sample_template.service, providers=[mmg_provider.identifier]).one() @@ -29,7 +29,7 @@ def test_should_update_provider_statistics_email(notify_db, notify_db, notify_db_session, template=sample_email_template) - update_notification_after_sent_to_provider(n1.id, 'email', ses_provider.identifier, reference="reference") + update_provider_stats(n1.id, 'email', ses_provider.identifier, reference="reference") provider_stats = get_provider_statistics( sample_email_template.service, providers=[ses_provider.identifier]).one() @@ -46,21 +46,21 @@ def test_should_update_provider_statistics_sms_multi(notify_db, template=sample_template, provider_name=mmg_provider.identifier, content_char_count=160) - update_notification_after_sent_to_provider(n1.id, 'sms', mmg_provider.identifier) + update_provider_stats(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) - update_notification_after_sent_to_provider(n2.id, 'sms', mmg_provider.identifier) + update_provider_stats(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) + update_provider_stats(n3.id, 'sms', mmg_provider.identifier) provider_stats = get_provider_statistics( sample_template.service, providers=[mmg_provider.identifier]).one() @@ -76,19 +76,19 @@ def test_should_update_provider_statistics_email_multi(notify_db, notify_db_session, template=sample_email_template, provider_name=ses_provider.identifier) - update_notification_after_sent_to_provider(n1.id, 'email', ses_provider.identifier, reference="reference") + update_provider_stats(n1.id, 'email', ses_provider.identifier, reference="reference") n2 = create_sample_notification( notify_db, notify_db_session, template=sample_email_template, provider_name=ses_provider.identifier) - update_notification_after_sent_to_provider(n2.id, 'email', ses_provider.identifier, reference="reference") + update_provider_stats(n2.id, 'email', ses_provider.identifier, reference="reference") n3 = create_sample_notification( notify_db, notify_db_session, template=sample_email_template, provider_name=ses_provider.identifier) - update_notification_after_sent_to_provider(n3.id, 'email', ses_provider.identifier, reference="reference") + update_provider_stats(n3.id, 'email', ses_provider.identifier, reference="reference") provider_stats = get_provider_statistics( sample_email_template.service, providers=[ses_provider.identifier]).one()