Provider stats only updated if the provider successfully sends the message.

app/celery/tasks.py

@@ -36,7 +36,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 (
@@ -262,6 +262,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)
@@ -331,7 +337,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)

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,28 @@ def dao_update_notification(notification):
     db.session.commit()
 
 
-def update_notification_reference_by_id(id, reference):
+@transactional
-    count = db.session.query(Notification).filter_by(
+def update_notification_after_sent_to_provider(id_, notification_type, provider_name, reference=None):
-        id=id
+    provider = ProviderDetails.query.filter_by(identifier=provider_name).one()
-    ).update({
+    notification = Notification.query.filter(Notification.id == id_).one()
-        Notification.reference: reference
+    if reference:
-    })
+        notification.reference = reference
-    db.session.commit()
+        db.session.add(notification)
-    return 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 + (
+        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)
 
 
 def get_notification_for_job(service_id, job_id, notification_id):

tests/app/conftest.py

@@ -328,7 +328,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
@@ -352,7 +352,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
 
 

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')
+    update_notification_after_sent_to_provider(
-    assert count == 1
+        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'

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)
+        template=sample_email_template,
-    create_sample_notification(
+        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)
+        template=sample_email_template,
-    create_sample_notification(
+        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()