Added tests for statsd and provider stats outcomes with the new provider stats tasks

- note large change to DAO to remove provider from create notification. Added on send now not creation.

app/celery/provider_tasks.py

@@ -26,7 +26,6 @@ from notifications_utils.template import (
     unlink_govuk_escaped
 )
 
-
 retry_iteration_to_delay = {
     0: 5,  # 5 seconds
     1: 30,  # 30 seconds
@@ -70,6 +69,11 @@ def send_sms_to_provider(self, service_id, notification_id, encrypted_notificati
                 content_char_count=template.replaced_content_count
             )
 
+        notification.sent_at = datetime.utcnow()
+        notification.sent_by = provider.get_name(),
+        notification.content_char_count = template.replaced_content_count
+        dao_update_notification(notification)
+
     except SmsClientException as e:
         try:
             current_app.logger.error(
@@ -79,14 +83,6 @@ def send_sms_to_provider(self, service_id, notification_id, encrypted_notificati
             raise self.retry(queue="retry", countdown=retry_iteration_to_delay[self.request.retries])
         except self.MaxRetriesExceededError:
             notification.status = 'technical-failure'
-    except SQLAlchemyError as e:
-        current_app.logger.exception(e)
-        raise self.retry(queue="retry", exc=e)
-
-    notification.sent_at = datetime.utcnow()
-    notification.sent_by = provider.get_name(),
-    notification.content_char_count = template.replaced_content_count
-    dao_update_notification(notification)
 
     current_app.logger.info(
         "SMS {} created at {} sent at {}".format(notification_id, notification.created_at, notification.sent_at)

app/dao/notifications_dao.py

@@ -197,7 +197,8 @@ def _update_notification_stats_query(notification_type, status):
 
 def _update_statistics(notification, notification_statistics_status):
     if notification.job_id:
-        db.session.query(Job).filter_by(id=notification.job_id
+        db.session.query(Job).filter_by(
+            id=notification.job_id
         ).update(_update_job_stats_query(notification_statistics_status))
 
     db.session.query(NotificationStatistics).filter_by(
@@ -231,17 +232,16 @@ def _update_notification_status(notification, status, notification_statistics_st
     if notification_statistics_status:
         _update_statistics(notification, notification_statistics_status)
 
-    db.session.query(Notification).filter(Notification.id == notification.id
+    db.session.query(Notification).filter(Notification.id == notification.id).update({Notification.status: status})
-                                          ).update({Notification.status: status})
     return True
 
 
 @transactional
 def update_notification_status_by_id(notification_id, status, notification_statistics_status=None):
-    notification = Notification.query.filter(Notification.id == notification_id,
+    notification = Notification.query.filter(
+        Notification.id == notification_id,
         or_(Notification.status == 'sending',
-                                                 Notification.status == 'pending')
+            Notification.status == 'pending')).first()
-                                             ).first()
 
     if not notification:
         return False

tests/app/celery/test_provider_tasks.py

@@ -2,19 +2,21 @@ import uuid
 
 from mock import ANY, call
 
-from app import statsd_client, mmg_client, DATETIME_FORMAT
+from app import statsd_client, mmg_client
 from app.celery.provider_tasks import send_sms_to_provider
 from app.celery.tasks import provider_to_use
-from app.dao import provider_details_dao
+from app.dao import provider_statistics_dao
-from datetime import datetime, timedelta
+from datetime import datetime
 from freezegun import freeze_time
-from app.dao import notifications_dao, jobs_dao, provider_details_dao
+from app.dao import notifications_dao, provider_details_dao
 from notifications_utils.recipients import validate_phone_number, format_phone_number
+from app.celery.research_mode_tasks import send_sms_response
 
 from tests.app.conftest import (
     sample_notification
 )
 
+
 def test_should_return_highest_priority_active_provider(notify_db, notify_db_session):
     providers = provider_details_dao.get_provider_details_by_notification_type('sms')
     first = providers[0]
@@ -79,7 +81,6 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
     )
     freezer.stop()
 
-
     mmg_client.send_sms.assert_called_once_with(
         to=format_phone_number(validate_phone_number("+447234123123")),
         content="Sample service: Hello Jo",
@@ -108,7 +109,6 @@ def test_should_send_template_to_correct_sms_provider_and_persist(
         mocker):
     db_notification = sample_notification(notify_db, notify_db_session, template=sample_template)
 
-
     notification = _notification_json(
         sample_template,
         to="+447234123123"
@@ -140,86 +140,162 @@ def test_should_send_template_to_correct_sms_provider_and_persist(
         content="Sample service: This is a template",
         reference=str(db_notification.id)
     )
-#
+
-# ### FIXME
+
-# def test_send_sms_should_use_template_version_from_job_not_latest(sample_template, mocker):
+def test_send_sms_should_use_template_version_from_notification_not_latest(
-#     notification = _notification_json(sample_template, '+447234123123')
+        notify_db,
-#     mocker.patch('app.encryption.decrypt', return_value=notification)
+        notify_db_session,
-#     mocker.patch('app.mmg_client.send_sms')
+        sample_template,
-#     mocker.patch('app.mmg_client.get_name', return_value="mmg")
+        mocker):
-#     version_on_notification = sample_template.version
+    db_notification = sample_notification(notify_db, notify_db_session, template=sample_template)
-#
+
-#     # Change the template
+    notification = _notification_json(sample_template, '+447234123123')
-#     from app.dao.templates_dao import dao_update_template, dao_get_template_by_id
+    mocker.patch('app.encryption.decrypt', return_value=notification)
-#     sample_template.content = sample_template.content + " another version of the template"
+    mocker.patch('app.mmg_client.send_sms')
-#     dao_update_template(sample_template)
+    mocker.patch('app.mmg_client.get_name', return_value="mmg")
-#     t = dao_get_template_by_id(sample_template.id)
+    version_on_notification = sample_template.version
-#     assert t.version > version_on_notification
+
-#
+    # Change the template
-#     notification_id = uuid.uuid4()
+    from app.dao.templates_dao import dao_update_template, dao_get_template_by_id
-#     now = datetime.utcnow()
+    sample_template.content = sample_template.content + " another version of the template"
-#     send_sms(
+    dao_update_template(sample_template)
-#         sample_template.service_id,
+    t = dao_get_template_by_id(sample_template.id)
-#         notification_id,
+    assert t.version > version_on_notification
-#         "encrypted-in-reality",
+
-#         now.strftime(DATETIME_FORMAT)
+    now = datetime.utcnow()
-#     )
+
-#
+    send_sms_to_provider(
-#     mmg_client.send_sms.assert_called_once_with(
+        db_notification.service_id,
-#         to=format_phone_number(validate_phone_number("+447234123123")),
+        db_notification.id,
-#         content="Sample service: This is a template",
+        "encrypted-in-reality"
-#         reference=str(notification_id)
+    )
-#     )
+
-#
+    mmg_client.send_sms.assert_called_once_with(
-#     persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id)
+        to=format_phone_number(validate_phone_number("+447234123123")),
-#     assert persisted_notification.id == notification_id
+        content="Sample service: This is a template",
-#     assert persisted_notification.to == '+447234123123'
+        reference=str(db_notification.id)
-#     assert persisted_notification.template_id == sample_template.id
+    )
-#     assert persisted_notification.template_version == version_on_notification
+
-#     assert persisted_notification.template_version != sample_template.version
+    persisted_notification = notifications_dao.get_notification(sample_template.service_id, db_notification.id)
-#     assert persisted_notification.created_at == now
+    assert persisted_notification.to == db_notification.to
-#     assert persisted_notification.sent_at > now
+    assert persisted_notification.template_id == sample_template.id
-#     assert persisted_notification.status == 'sending'
+    assert persisted_notification.template_version == version_on_notification
-#     assert persisted_notification.sent_by == 'mmg'
+    assert persisted_notification.template_version != sample_template.version
-#     assert persisted_notification.content_char_count == len("Sample service: This is a template")
+    assert persisted_notification.sent_at > now
-#
+    assert persisted_notification.status == 'sending'
-#
+    assert persisted_notification.sent_by == 'mmg'
-# ### FIXME
+    assert persisted_notification.content_char_count == len("Sample service: This is a template")
-# def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_template, mocker):
+
-#     notification = _notification_json(
+
-#         sample_template,
+def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_notification, mocker):
-#         to="+447234123123"
+    notification = _notification_json(
-#     )
+        sample_notification.template,
-#     mocker.patch('app.encryption.decrypt', return_value=notification)
+        to=sample_notification.to
-#     mocker.patch('app.mmg_client.send_sms')
+    )
-#     mocker.patch('app.mmg_client.get_name', return_value="mmg")
+    mocker.patch('app.encryption.decrypt', return_value=notification)
-#     mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+    mocker.patch('app.mmg_client.send_sms')
-#
+    mocker.patch('app.mmg_client.get_name', return_value="mmg")
-#     sample_service.research_mode = True
+    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
-#     notify_db.session.add(sample_service)
+
-#     notify_db.session.commit()
+    sample_service.research_mode = True
-#
+    notify_db.session.add(sample_service)
-#     notification_id = uuid.uuid4()
+    notify_db.session.commit()
-#     now = datetime.utcnow()
+
-#     send_sms(
+    now = datetime.utcnow()
-#         sample_service.id,
+    send_sms_to_provider(
-#         notification_id,
+        sample_notification.service_id,
-#         "encrypted-in-reality",
+        sample_notification.id,
-#         now.strftime(DATETIME_FORMAT)
+        "encrypted-in-reality"
-#     )
+    )
-#     assert not mmg_client.send_sms.called
+    assert not mmg_client.send_sms.called
-#     send_sms_response.apply_async.assert_called_once_with(
+    send_sms_response.apply_async.assert_called_once_with(
-#         ('mmg', str(notification_id), "+447234123123"), queue='research-mode'
+        ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode'
-#     )
+    )
-#
+
-#     persisted_notification = notifications_dao.get_notification(sample_service.id, notification_id)
+    persisted_notification = notifications_dao.get_notification(sample_service.id, sample_notification.id)
-#     assert persisted_notification.id == notification_id
+    assert persisted_notification.to == sample_notification.to
-#     assert persisted_notification.to == '+447234123123'
+    assert persisted_notification.template_id == sample_notification.template_id
-#     assert persisted_notification.template_id == sample_template.id
+    assert persisted_notification.status == 'sending'
-#     assert persisted_notification.status == 'sending'
+    assert persisted_notification.sent_at > now
-#     assert persisted_notification.sent_at > now
+    assert persisted_notification.sent_by == 'mmg'
-#     assert persisted_notification.created_at == now
+
-#     assert persisted_notification.sent_by == 'mmg'
+
+def test_should_update_provider_stats_on_success(notify_db, sample_service, sample_notification, mocker):
+    provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all()
+    assert len(provider_stats) == 0
+
+    notification = _notification_json(
+        sample_notification.template,
+        to=sample_notification.to
+    )
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.mmg_client.send_sms')
+    mocker.patch('app.mmg_client.get_name', return_value="mmg")
+    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+
+    send_sms_to_provider(
+        sample_notification.service_id,
+        sample_notification.id,
+        "encrypted-in-reality"
+    )
+
+    updated_provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all()
+    assert updated_provider_stats[0].provider.identifier == 'mmg'
+    assert updated_provider_stats[0].unit_count == 1
+
+
+def test_not_should_update_provider_stats_on_success(notify_db, sample_service, sample_notification, mocker):
+    provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all()
+    assert len(provider_stats) == 0
+
+    notification = _notification_json(
+        sample_notification.template,
+        to=sample_notification.to
+    )
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.mmg_client.send_sms')
+    mocker.patch('app.mmg_client.get_name', return_value="mmg")
+    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+
+    sample_service.research_mode = True
+    notify_db.session.add(sample_service)
+    notify_db.session.commit()
+
+    send_sms_to_provider(
+        sample_notification.service_id,
+        sample_notification.id,
+        "encrypted-in-reality"
+    )
+
+    updated_provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all()
+    assert len(updated_provider_stats) == 0
+
+
+def test_statsd_updates(notify_db, notify_db_session, sample_service, sample_notification, mocker):
+    notification = _notification_json(
+        sample_notification.template,
+        to=sample_notification.to
+    )
+
+    mocker.patch('app.statsd_client.incr')
+    mocker.patch('app.statsd_client.timing')
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.mmg_client.send_sms')
+    mocker.patch('app.mmg_client.get_name', return_value="mmg")
+    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+
+    send_sms_to_provider(
+        sample_notification.service_id,
+        sample_notification.id,
+        "encrypted-in-reality"
+    )
+
+    statsd_client.incr.assert_called_once_with("notifications.tasks.send-sms-to-provider")
+    statsd_client.timing.assert_has_calls([
+        call("notifications.tasks.send-sms-to-provider.task-time", ANY),
+        call("notifications.sms.total-time", ANY)
+    ])
 
 
 def _notification_json(template, to, personalisation=None, job_id=None, row_number=None):

tests/app/celery/test_tasks.py

@@ -333,16 +333,15 @@ def test_should_process_all_sms_job(sample_job,
     )
     assert encryption.encrypt.call_args[0][0]['to'] == '+441234123120'
     assert encryption.encrypt.call_args[0][0]['template'] == str(sample_job_with_placeholdered_template.template.id)
-    assert encryption.encrypt.call_args[0][0]['template_version'] == sample_job_with_placeholdered_template.template.version  # noqa
+    assert encryption.encrypt.call_args[0][0][
+               'template_version'] == sample_job_with_placeholdered_template.template.version  # noqa
     assert encryption.encrypt.call_args[0][0]['personalisation'] == {'name': 'chris'}
     tasks.send_sms.apply_async.call_count == 10
     job = jobs_dao.dao_get_job_by_id(sample_job_with_placeholdered_template.id)
     assert job.status == 'finished'
 
 
-### START OF SEND-SMS
+def test_should_send_template_to_correct_sms_task_and_persist(sample_template_with_placeholders, mocker):
-
-def test_should_send_template_to_correct_sms_provider_and_persist(sample_template_with_placeholders, mocker):
     notification = _notification_json(sample_template_with_placeholders,
                                       to="+447234123123", personalisation={"name": "Jo"})
     mocker.patch('app.encryption.decrypt', return_value=notification)
@@ -393,29 +392,6 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
     assert not persisted_notification.job_id
 
 
-def test_should_send_sms_without_personalisation(sample_template, mocker):
-    notification = _notification_json(sample_template, "+447234123123")
-    mocker.patch('app.encryption.decrypt', return_value=notification)
-    mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
-
-    notification_id = uuid.uuid4()
-    now = datetime.utcnow()
-    send_sms(
-        sample_template.service_id,
-        notification_id,
-        "encrypted-in-reality",
-        now.strftime(DATETIME_FORMAT)
-    )
-
-
-    provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with(
-        (sample_template.service_id,
-         notification_id,
-         "encrypted-in-reality"),
-        queue="sms"
-    )
-
-
 def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker):
     user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900890")
     service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
@@ -772,6 +748,7 @@ def test_should_persist_notification_as_failed_if_database_fails(sample_template
         notifications_dao.get_notification(sample_template.service_id, notification_id)
     assert 'No row was found for one' in str(e.value)
 
+
 def test_should_persist_notification_as_failed_if_email_client_fails(sample_email_template, mocker):
     notification = _notification_json(sample_email_template, "my_email@my_email.com")
     mocker.patch('app.encryption.decrypt', return_value=notification)

tests/app/conftest.py

@@ -319,7 +319,6 @@ def sample_notification(notify_db,
                         status='sending',
                         reference=None,
                         created_at=datetime.utcnow(),
-                        provider_name=None,
                         content_char_count=160,
                         create=True):
     if service is None:
@@ -331,9 +330,6 @@ def sample_notification(notify_db,
 
     notification_id = uuid.uuid4()
 
-    if provider_name is None:
-        provider_name = mmg_provider().identifier if template.template_type == 'sms' else ses_provider().identifier
-
     if to_field:
         to = to_field
     else:
@@ -342,6 +338,7 @@ def sample_notification(notify_db,
     data = {
         'id': notification_id,
         'to': to,
+        'job_id': job.id,
         'job': job,
         'service_id': service.id,
         'service': service,

tests/app/dao/test_notification_dao.py

@@ -42,8 +42,7 @@ def test_should_by_able_to_update_status_by_reference(sample_email_template, ses
     notification = Notification(**data)
     dao_create_notification(
         notification,
-        sample_email_template.template_type,
+        sample_email_template.template_type)
-        ses_provider.identifier)
 
     assert Notification.query.get(notification.id).status == "sending"
     notification.reference = 'reference'
@@ -57,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)
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, 'mmg')
+    dao_create_notification(notification, sample_template.template_type)
     assert Notification.query.get(notification.id).status == 'sending'
     assert update_notification_status_by_id(notification.id, 'delivered', 'delivered')
     assert Notification.query.get(notification.id).status == 'delivered'
@@ -77,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)
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, 'mmg')
+    dao_create_notification(notification, sample_template.template_type)
     assert Notification.query.get(notification.id).status == 'sending'
     assert update_notification_status_by_id(notification_id=notification.id, status='pending')
     assert Notification.query.get(notification.id).status == 'pending'
@@ -93,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)
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, 'mmg')
+    dao_create_notification(notification, sample_template.template_type)
     assert Notification.query.get(notification.id).status == 'sending'
     assert update_notification_status_by_id(notification_id=notification.id, status='pending')
     assert Notification.query.get(notification.id).status == 'pending'
@@ -112,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)
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, 'firetext')
+    dao_create_notification(notification, sample_template.template_type)
     assert Notification.query.get(notification.id).status == 'sending'
 
     assert update_notification_status_by_id(
@@ -131,8 +130,7 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em
     notification = Notification(**data)
     dao_create_notification(
         notification,
-        sample_email_template.template_type,
+        sample_email_template.template_type)
-        ses_provider.identifier)
 
     assert Notification.query.get(notification.id).status == "sending"
     update_provider_stats(
@@ -161,7 +159,7 @@ def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification
 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)
     notification = Notification(**data)
-    dao_create_notification(notification, sample_email_template.template_type, ses_provider.identifier)
+    dao_create_notification(notification, sample_email_template.template_type)
 
     update_provider_stats(
         notification.id,
@@ -188,7 +186,7 @@ def test_should_be_able_to_get_statistics_for_a_service(sample_template, mmg_pro
     data = _notification_json(sample_template)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     _assert_notification_stats(notification.service_id, sms_requested=1,
                                notification_created_at=notification.created_at.date())
@@ -198,7 +196,7 @@ def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_templat
     now = datetime.utcnow()
     data = _notification_json(sample_template)
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
     stat = dao_get_notification_statistics_for_service_and_day(
         sample_template.service.id, now.date()
     )
@@ -216,7 +214,7 @@ def test_should_return_none_if_no_statistics_for_a_service_for_a_day(sample_temp
     data = _notification_json(sample_template)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
     assert not dao_get_notification_statistics_for_service_and_day(
         sample_template.service.id, (datetime.utcnow() - timedelta(days=1)).date())
 
@@ -227,9 +225,9 @@ def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
     notification_3 = Notification(**data)
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
-    dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_3, sample_template.template_type)
 
     _assert_notification_stats(sample_template.service.id, sms_requested=3)
 
@@ -247,9 +245,9 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days(sam
     data.update({'created_at': two_days_ago})
     notification_3 = Notification(**data)
 
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
-    dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_3, sample_template.template_type)
 
     stats = dao_get_notification_statistics_for_service(sample_template.service.id)
     assert len(stats) == 3
@@ -281,9 +279,9 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_pre
     notification_2 = Notification(**data)
     data.update({'created_at': eight_days_ago})
     notification_3 = Notification(**data)
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
-    dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_3, sample_template.template_type)
 
     stats = dao_get_notification_statistics_for_service(sample_template.service.id, 7)
     assert len(stats) == 2
@@ -303,7 +301,7 @@ def test_save_notification_creates_sms_and_template_stats(sample_template, sampl
     data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -336,7 +334,7 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp
     data = _notification_json(sample_email_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_email_template.template_type, ses_provider.identifier)
+    dao_create_notification(notification, sample_email_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -370,7 +368,7 @@ def test_save_notification_handles_midnight_properly(sample_template, sample_job
     data = _notification_json(sample_template, sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
 
@@ -386,7 +384,7 @@ def test_save_notification_handles_just_before_midnight_properly(sample_template
     data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
 
@@ -402,7 +400,7 @@ def test_save_notification_and_increment_email_stats(sample_email_template, samp
 
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
-    dao_create_notification(notification_1, sample_email_template.template_type, ses_provider.identifier)
+    dao_create_notification(notification_1, sample_email_template.template_type)
 
     assert Notification.query.count() == 1
 
@@ -412,7 +410,7 @@ def test_save_notification_and_increment_email_stats(sample_email_template, samp
     assert stats1.emails_requested == 1
     assert stats1.sms_requested == 0
 
-    dao_create_notification(notification_2, sample_email_template.template_type, ses_provider.identifier)
+    dao_create_notification(notification_2, sample_email_template.template_type)
 
     assert Notification.query.count() == 2
 
@@ -429,7 +427,7 @@ def test_save_notification_and_increment_sms_stats(sample_template, sample_job,
 
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
 
     assert Notification.query.count() == 1
 
@@ -440,7 +438,7 @@ def test_save_notification_and_increment_sms_stats(sample_template, sample_job,
     assert stats1.emails_requested == 0
     assert stats1.sms_requested == 1
 
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
 
     assert Notification.query.count() == 2
 
@@ -460,7 +458,7 @@ def test_not_save_notification_and_not_create_stats_on_commit_error(sample_templ
 
     notification = Notification(**data)
     with pytest.raises(SQLAlchemyError):
-        dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+        dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 0
     assert Job.query.get(sample_job.id).notifications_sent == 0
@@ -473,7 +471,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr
     data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -488,7 +486,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr
     assert Job.query.get(sample_job.id).notifications_sent == 1
 
     notification_2 = Notification(**data)
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
     assert Notification.query.count() == 2
     # count is off because the count is defaulted to 1 in the sample_job
     _assert_job_stats(sample_job.id, sent=2, count=1)
@@ -500,7 +498,7 @@ def test_should_not_increment_job_if_notification_fails_to_persist(sample_templa
     data = _notification_json(sample_template, job_id=sample_job.id, id=random_id)
 
     notification_1 = Notification(**data)
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
 
     assert Notification.query.count() == 1
     assert Job.query.get(sample_job.id).notifications_sent == 1
@@ -508,7 +506,7 @@ def test_should_not_increment_job_if_notification_fails_to_persist(sample_templa
 
     notification_2 = Notification(**data)
     with pytest.raises(SQLAlchemyError):
-        dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+        dao_create_notification(notification_2, sample_template.template_type)
 
     assert Notification.query.count() == 1
     assert Job.query.get(sample_job.id).notifications_sent == 1
@@ -524,7 +522,7 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio
     data = _notification_json(sample_template, job_id=job_1.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -546,7 +544,7 @@ def test_save_notification_with_no_job(sample_template, mmg_provider):
     data = _notification_json(sample_template)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -571,7 +569,7 @@ def test_save_notification_no_job_id(sample_template, mmg_provider):
     data = _notification_json(sample_template)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -655,7 +653,7 @@ def test_save_new_notification_creates_template_stats(sample_template, sample_jo
     data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert TemplateStatistics.query.count() == 1
     template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id,
@@ -673,7 +671,7 @@ def test_save_new_notification_creates_template_stats_per_day(sample_template, s
     data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert TemplateStatistics.query.count() == 1
     template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id,
@@ -687,7 +685,7 @@ def test_save_new_notification_creates_template_stats_per_day(sample_template, s
     with freeze_time('2016-03-31'):
         assert TemplateStatistics.query.count() == 1
         new_notification = Notification(**data)
-        dao_create_notification(new_notification, sample_template.template_type, mmg_provider.identifier)
+        dao_create_notification(new_notification, sample_template.template_type)
 
     assert TemplateStatistics.query.count() == 2
     first_stats = TemplateStatistics.query.filter(TemplateStatistics.day == datetime(2016, 3, 30)).first()
@@ -710,7 +708,7 @@ def test_save_another_notification_increments_template_stats(sample_template, sa
 
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
 
     assert TemplateStatistics.query.count() == 1
     template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id,
@@ -719,7 +717,7 @@ def test_save_another_notification_increments_template_stats(sample_template, sa
     assert template_stats.template_id == sample_template.id
     assert template_stats.usage_count == 1
 
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
 
     assert TemplateStatistics.query.count() == 1
     template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id,
@@ -739,9 +737,9 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
     notification_3 = Notification(**data)
-    dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_1, sample_template.template_type)
-    dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_2, sample_template.template_type)
-    dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification_3, sample_template.template_type)
 
     _assert_notification_stats(sample_template.service.id, sms_requested=3)
 
@@ -756,32 +754,30 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_
     try:
         # Mess up db in really bad way
         db.session.execute('DROP TABLE TEMPLATE_STATISTICS')
-        dao_create_notification(failing_notification, sample_template.template_type, mmg_provider.identifier)
+        dao_create_notification(failing_notification, sample_template.template_type)
     except Exception as e:
         # There should be no additional notification stats or counts
         _assert_notification_stats(sample_template.service.id, sms_requested=3)
 
 
 @freeze_time("2016-03-30")
-def test_get_template_stats_for_service_returns_stats_in_reverse_date_order(sample_template,
+def test_get_template_stats_for_service_returns_stats_in_reverse_date_order(sample_template, sample_job):
-                                                                            sample_job,
-                                                                            mmg_provider):
     template_stats = dao_get_template_statistics_for_service(sample_template.service.id)
     assert len(template_stats) == 0
     data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
-    dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
+    dao_create_notification(notification, sample_template.template_type)
 
     # move on one day
     with freeze_time('2016-03-31'):
         new_notification = Notification(**data)
-        dao_create_notification(new_notification, sample_template.template_type, mmg_provider.identifier)
+        dao_create_notification(new_notification, sample_template.template_type)
 
     # move on one more day
     with freeze_time('2016-04-01'):
         new_notification = Notification(**data)
-        dao_create_notification(new_notification, sample_template.template_type, mmg_provider.identifier)
+        dao_create_notification(new_notification, sample_template.template_type)
 
     template_stats = dao_get_template_statistics_for_service(sample_template.service_id)
     assert len(template_stats) == 3

tests/app/dao/test_notifications_dao_provider_statistics.py

@@ -44,21 +44,18 @@ def test_should_update_provider_statistics_sms_multi(notify_db,
         notify_db,
         notify_db_session,
         template=sample_template,
-        provider_name=mmg_provider.identifier,
         content_char_count=160)
     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_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_provider_stats(n3.id, 'sms', mmg_provider.identifier)
     provider_stats = get_provider_statistics(
@@ -74,20 +71,17 @@ def test_should_update_provider_statistics_email_multi(notify_db,
     n1 = create_sample_notification(
         notify_db,
         notify_db_session,
-        template=sample_email_template,
+        template=sample_email_template)
-        provider_name=ses_provider.identifier)
     update_provider_stats(n1.id, 'email', ses_provider.identifier)
     n2 = create_sample_notification(
         notify_db,
         notify_db_session,
-        template=sample_email_template,
+        template=sample_email_template)
-        provider_name=ses_provider.identifier)
     update_provider_stats(n2.id, 'email', ses_provider.identifier)
     n3 = create_sample_notification(
         notify_db,
         notify_db_session,
-        template=sample_email_template,
+        template=sample_email_template)
-        provider_name=ses_provider.identifier)
     update_provider_stats(n3.id, 'email', ses_provider.identifier)
     provider_stats = get_provider_statistics(
         sample_email_template.service,