Removed update reference from updating the provider stats

- single focus method
- allows not to pollute DAO with research mode

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'

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()

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,

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,

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()