Merge pull request #141 from alphagov/capture-aggregate-data

Capture aggregate data

app/__init__.py

@@ -13,6 +13,8 @@ from app.clients.sms.firetext import FiretextClient
 from app.clients.email.aws_ses import AwsSesClient
 from app.encryption import Encryption
 
+DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%f"
+
 db = SQLAlchemy()
 ma = Marshmallow()
 notify_celery = NotifyCelery()
@@ -49,6 +51,7 @@ def create_app():
     from app.invite.rest import invite as invite_blueprint
     from app.permission.rest import permission as permission_blueprint
     from app.accept_invite.rest import accept_invite
+    from app.notifications_statistics.rest import notifications_statistics as notifications_statistics_blueprint
 
     application.register_blueprint(service_blueprint, url_prefix='/service')
     application.register_blueprint(user_blueprint, url_prefix='/user')
@@ -59,6 +62,7 @@ def create_app():
     application.register_blueprint(invite_blueprint)
     application.register_blueprint(permission_blueprint, url_prefix='/permission')
     application.register_blueprint(accept_invite, url_prefix='/invite')
+    application.register_blueprint(notifications_statistics_blueprint)
 
     return application
 

app/celery/tasks.py

@@ -1,4 +1,4 @@
-from app import create_uuid
+from app import create_uuid, DATETIME_FORMAT
 from app import notify_celery, encryption, firetext_client, aws_ses_client
 from app.clients.email.aws_ses import AwsSesClientException
 from app.clients.sms.firetext import FiretextClientException
@@ -6,7 +6,7 @@ from app.dao.services_dao import dao_fetch_service_by_id
 from app.dao.templates_dao import dao_get_template_by_id
 from app.dao.notifications_dao import dao_create_notification, dao_update_notification
 from app.dao.jobs_dao import dao_update_job, dao_get_job_by_id
-from app.models import Notification
+from app.models import Notification, TEMPLATE_TYPE_EMAIL, TEMPLATE_TYPE_SMS
 from flask import current_app
 from sqlalchemy.exc import SQLAlchemyError
 from app.aws import s3
@@ -39,7 +39,7 @@ def process_job(job_id):
                 str(job.service_id),
                 str(create_uuid()),
                 encrypted,
-                str(datetime.utcnow())),
+                datetime.utcnow().strftime(DATETIME_FORMAT)),
                 queue='bulk-sms'
             )
 
@@ -50,7 +50,7 @@ def process_job(job_id):
                 job.template.subject,
                 "{}@{}".format(job.service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']),
                 encrypted,
-                str(datetime.utcnow())),
+                datetime.utcnow().strftime(DATETIME_FORMAT)),
                 queue='bulk-email')
 
     finished = datetime.utcnow()
@@ -89,12 +89,12 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
             service_id=service_id,
             job_id=notification.get('job', None),
             status=status,
-            created_at=created_at,
+            created_at=datetime.strptime(created_at, DATETIME_FORMAT),
             sent_at=sent_at,
             sent_by=client.get_name()
         )
 
-        dao_create_notification(notification_db_object)
+        dao_create_notification(notification_db_object, TEMPLATE_TYPE_SMS)
 
         if can_send:
             try:
@@ -159,11 +159,11 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not
             service_id=service_id,
             job_id=notification.get('job', None),
             status=status,
-            created_at=created_at,
+            created_at=datetime.strptime(created_at, DATETIME_FORMAT),
             sent_at=sent_at,
             sent_by=client.get_name()
         )
-        dao_create_notification(notification_db_object)
+        dao_create_notification(notification_db_object, TEMPLATE_TYPE_EMAIL)
 
         if can_send:
             try:

app/dao/notifications_dao.py

@@ -1,16 +1,59 @@
 from flask import current_app
 from app import db
-from app.models import Notification, Job
+from app.models import Notification, Job, NotificationStatistics, TEMPLATE_TYPE_SMS, TEMPLATE_TYPE_EMAIL
 from sqlalchemy import desc
+from datetime import datetime
 
 
-def dao_create_notification(notification):
+def dao_get_notification_statistics_for_service(service_id):
-    if notification.job_id:
+    return NotificationStatistics.query.filter_by(
-        db.session.query(Job).filter_by(
+        service_id=service_id
-            id=notification.job_id
+    ).order_by(desc(NotificationStatistics.day)).all()
-        ).update({Job.notifications_sent: Job.notifications_sent + 1})
+
-    db.session.add(notification)
+
-    db.session.commit()
+def dao_create_notification(notification, notification_type):
+    try:
+        if notification.job_id:
+            update_job_sent_count(notification)
+
+        if update_notification_stats(notification, notification_type) == 0:
+            stats = NotificationStatistics(
+                day=notification.created_at.strftime('%Y-%m-%d'),
+                service_id=notification.service_id,
+                sms_requested=1 if notification_type == TEMPLATE_TYPE_SMS else 0,
+                emails_requested=1 if notification_type == TEMPLATE_TYPE_EMAIL else 0
+            )
+            db.session.add(stats)
+        db.session.add(notification)
+        db.session.commit()
+    except:
+        db.session.rollback()
+        raise
+
+
+def update_notification_stats(notification, notification_type):
+    if notification_type == TEMPLATE_TYPE_SMS:
+        update = {
+            NotificationStatistics.sms_requested: NotificationStatistics.sms_requested + 1
+        }
+    else:
+        update = {
+            NotificationStatistics.emails_requested: NotificationStatistics.emails_requested + 1
+        }
+
+    return db.session.query(NotificationStatistics).filter_by(
+        day=notification.created_at.strftime('%Y-%m-%d'),
+        service_id=notification.service_id
+    ).update(update)
+
+
+def update_job_sent_count(notification):
+    db.session.query(Job).filter_by(
+        id=notification.job_id
+    ).update({
+        Job.notifications_sent: Job.notifications_sent + 1,
+        Job.updated_at: datetime.utcnow()
+    })
 
 
 def dao_update_notification(notification):

app/models.py

@@ -107,7 +107,30 @@ class ApiKey(db.Model):
     )
 
 
-TEMPLATE_TYPES = ['sms', 'email', 'letter']
+class NotificationStatistics(db.Model):
+    __tablename__ = 'notification_statistics'
+
+    id = db.Column(db.Integer, primary_key=True)
+    day = db.Column(db.String(255), nullable=False)
+    service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False)
+    service = db.relationship('Service', backref=db.backref('service_notification_stats', lazy='dynamic'))
+    emails_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False)
+    emails_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=True)
+    emails_error = db.Column(db.BigInteger, index=False, unique=False, nullable=True)
+    sms_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False)
+    sms_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=True)
+    sms_error = db.Column(db.BigInteger, index=False, unique=False, nullable=True)
+
+    __table_args__ = (
+        UniqueConstraint('service_id', 'day', name='uix_service_to_day'),
+    )
+
+
+TEMPLATE_TYPE_SMS = 'sms'
+TEMPLATE_TYPE_EMAIL = 'email'
+TEMPLATE_TYPE_LETTER = 'letter'
+
+TEMPLATE_TYPES = [TEMPLATE_TYPE_SMS, TEMPLATE_TYPE_EMAIL, TEMPLATE_TYPE_LETTER]
 
 
 class Template(db.Model):

app/notifications/rest.py

@@ -10,7 +10,7 @@ from flask import (
 
 from utils.template import Template, NeededByTemplateError, NoPlaceholderForDataError
 
-from app import api_user, encryption, create_uuid
+from app import api_user, encryption, create_uuid, DATETIME_FORMAT
 from app.authentication.auth import require_admin
 from app.dao import (
     templates_dao,
@@ -178,7 +178,7 @@ def send_notification(notification_type):
             service_id,
             notification_id,
             encryption.encrypt(notification),
-            str(datetime.utcnow())
+            datetime.utcnow().strftime(DATETIME_FORMAT)
         ), queue='sms')
     else:
         if service.restricted and notification['to'] not in [user.email_address for user in service.users]:
@@ -190,6 +190,6 @@ def send_notification(notification_type):
             template.subject,
             "{}@{}".format(service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']),
             encryption.encrypt(notification),
-            str(datetime.utcnow())
+            datetime.utcnow().strftime(DATETIME_FORMAT)
         ), queue='email')
     return jsonify({'notification_id': notification_id}), 201

app/notifications_statistics/rest.py

@@ -0,0 +1,25 @@
+from flask import (
+    Blueprint,
+    jsonify,
+)
+
+from app.dao.notifications_dao import (
+    dao_get_notification_statistics_for_service
+)
+from app.schemas import notifications_statistics_schema
+
+notifications_statistics = Blueprint(
+    'notifications-statistics',
+    __name__, url_prefix='/service/<service_id>/notifications-statistics'
+)
+
+from app.errors import register_errors
+
+register_errors(notifications_statistics)
+
+
+@notifications_statistics.route('', methods=['GET'])
+def get_all_templates_for_service(service_id):
+    templates = dao_get_notification_statistics_for_service(service_id=service_id)
+    data, errors = notifications_statistics_schema.dump(templates, many=True)
+    return jsonify(data=data)

app/schemas.py

@@ -35,19 +35,6 @@ class BaseSchema(ma.ModelSchema):
         assert key is not None, "Envelope key undefined"
         return key
 
-    # Code to envelope the input and response.
-    # TOBE added soon.
-
-    # @pre_load(pass_many=True)
-    # def unwrap_envelope(self, data, many):
-    #     key = self.get_envelope_key(many)
-    #     return data[key]
-
-    # @post_dump(pass_many=True)
-    # def wrap_with_envelope(self, data, many):
-    #     key = self.get_envelope_key(many)
-    #     return {key: data}
-
     @post_load
     def make_instance(self, data):
         """Deserialize data to an instance of the model. Update an existing row
@@ -92,6 +79,11 @@ class TemplateSchema(BaseSchema):
         exclude = ("updated_at", "created_at", "service_id", "jobs")
 
 
+class NotificationsStatisticsSchema(BaseSchema):
+    class Meta:
+        model = models.NotificationStatistics
+
+
 class ApiKeySchema(BaseSchema):
     class Meta:
         model = models.ApiKey
@@ -230,3 +222,4 @@ notification_status_schema = NotificationStatusSchema()
 notification_status_schema_load_json = NotificationStatusSchema(load_json=True)
 invited_user_schema = InvitedUserSchema()
 permission_schema = PermissionSchema()
+notifications_statistics_schema = NotificationsStatisticsSchema()

migrations/versions/0036_notification_stats.py

@@ -0,0 +1,37 @@
+"""empty message
+
+Revision ID: 0036_notification_stats
+Revises: 0035_default_sent_count
+Create Date: 2016-03-08 11:16:25.659463
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0036_notification_stats'
+down_revision = '0035_default_sent_count'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+def upgrade():
+    op.create_table('notification_statistics',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('day', sa.String(length=255), nullable=False),
+    sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False),
+    sa.Column('emails_requested', sa.BigInteger(), nullable=False),
+    sa.Column('emails_delivered', sa.BigInteger(), nullable=True),
+    sa.Column('emails_error', sa.BigInteger(), nullable=True),
+    sa.Column('sms_requested', sa.BigInteger(), nullable=False),
+    sa.Column('sms_delivered', sa.BigInteger(), nullable=True),
+    sa.Column('sms_error', sa.BigInteger(), nullable=True),
+    sa.ForeignKeyConstraint(['service_id'], ['services.id'], ),
+    sa.PrimaryKeyConstraint('id'),
+    sa.UniqueConstraint('service_id', 'day', name='uix_service_to_day')
+    )
+    op.create_index(op.f('ix_service_notification_stats_service_id'), 'notification_statistics', ['service_id'], unique=False)
+
+
+def downgrade():
+    op.drop_index(op.f('ix_service_notification_stats_service_id'), table_name='notification_statistics')
+    op.drop_table('notification_statistics')

tests/app/celery/test_tasks.py

@@ -2,7 +2,7 @@ import uuid
 import pytest
 from flask import current_app
 from app.celery.tasks import (send_sms, send_sms_code, send_email_code, send_email, process_job, email_invited_user)
-from app import (firetext_client, aws_ses_client, encryption)
+from app import (firetext_client, aws_ses_client, encryption, DATETIME_FORMAT)
 from app.clients.email.aws_ses import AwsSesClientException
 from app.clients.sms.firetext import FiretextClientException
 from app.dao import notifications_dao, jobs_dao
@@ -34,7 +34,7 @@ def test_should_process_sms_job(sample_job, mocker):
         (str(sample_job.service_id),
          "uuid",
          "something_encrypted",
-         "2016-01-01 11:09:00.061258"),
+         "2016-01-01T11:09:00.061258"),
         queue="bulk-sms"
     )
     job = jobs_dao.dao_get_job_by_id(sample_job.id)
@@ -69,7 +69,7 @@ def test_should_process_email_job(sample_email_job, mocker):
          sample_email_job.template.subject,
          "{}@{}".format(sample_email_job.service.email_from, "test.notify.com"),
          "something_encrypted",
-         "2016-01-01 11:09:00.061258"),
+         "2016-01-01T11:09:00.061258"),
         queue="bulk-email"
     )
     job = jobs_dao.dao_get_job_by_id(sample_email_job.id)
@@ -106,7 +106,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
         sample_template_with_placeholders.service_id,
         notification_id,
         "encrypted-in-reality",
-        now
+        now.strftime(DATETIME_FORMAT)
     )
 
     firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: Hello Jo")
@@ -138,7 +138,7 @@ def test_should_send_sms_without_personalisation(sample_template, mocker):
         sample_template.service_id,
         notification_id,
         "encrypted-in-reality",
-        now
+        now.strftime(DATETIME_FORMAT)
     )
 
     firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template")
@@ -164,7 +164,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
         service.id,
         notification_id,
         "encrypted-in-reality",
-        now
+        now.strftime(DATETIME_FORMAT)
     )
 
     firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template")
@@ -190,7 +190,7 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db,
         service.id,
         notification_id,
         "encrypted-in-reality",
-        now
+        now.strftime(DATETIME_FORMAT)
     )
 
     firetext_client.send_sms.assert_not_called()
@@ -217,7 +217,8 @@ def test_should_send_email_if_restricted_service_and_valid_email(notify_db, noti
         'subject',
         'email_from',
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
+    )
 
     aws_ses_client.send_email.assert_called_once_with(
         "email_from",
@@ -243,8 +244,8 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa
         sample_job.service.id,
         notification_id,
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template")
     persisted_notification = notifications_dao.get_notification(sample_job.template.service_id, notification_id)
     assert persisted_notification.id == notification_id
@@ -275,8 +276,8 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
         'subject',
         'email_from',
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     aws_ses_client.send_email.assert_called_once_with(
         "email_from",
         "my_email@my_email.com",
@@ -313,8 +314,8 @@ def test_should_use_email_template_and_persist_without_personalisation(
         'subject',
         'email_from',
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     aws_ses_client.send_email.assert_called_once_with(
         "email_from",
         "my_email@my_email.com",
@@ -339,8 +340,8 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa
         sample_template.service_id,
         notification_id,
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template")
     persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id)
     assert persisted_notification.id == notification_id
@@ -371,8 +372,8 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai
         'subject',
         'email_from',
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     aws_ses_client.send_email.assert_called_once_with(
         "email_from",
         "my_email@my_email.com",
@@ -405,8 +406,8 @@ def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
         sample_template.service_id,
         notification_id,
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     firetext_client.send_sms.assert_not_called()
     with pytest.raises(NoResultFound) as e:
         notifications_dao.get_notification(sample_template.service_id, notification_id)
@@ -431,8 +432,8 @@ def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mo
         'subject',
         'email_from',
         "encrypted-in-reality",
-        now)
+        now.strftime(DATETIME_FORMAT)
-
+    )
     aws_ses_client.send_email.assert_not_called()
     with pytest.raises(NoResultFound) as e:
         notifications_dao.get_notification(sample_email_template.service_id, notification_id)

tests/app/conftest.py

@@ -283,12 +283,13 @@ def sample_notification(notify_db,
         'id': notification_id,
         'to': to,
         'job': job,
+        'service_id': service.id,
         'service': service,
         'template': template,
         'created_at': datetime.utcnow()
     }
     notification = Notification(**data)
-    dao_create_notification(notification)
+    dao_create_notification(notification, template.template_type)
     return notification
 
 

tests/app/dao/test_notification_dao.py

@@ -1,28 +1,119 @@
-from app.models import Notification, Job
+import pytest
-from datetime import datetime
+import uuid
+from freezegun import freeze_time
+from sqlalchemy.exc import SQLAlchemyError, IntegrityError
+from app.models import Notification, Job, NotificationStatistics
+from datetime import datetime, timedelta
 from app.dao.notifications_dao import (
     dao_create_notification,
     dao_update_notification,
     get_notification,
     get_notification_for_job,
-    get_notifications_for_job
+    get_notifications_for_job,
+    dao_get_notification_statistics_for_service
 )
 from tests.app.conftest import sample_job
 
 
-def test_save_notification_and_increment_job(sample_template, sample_job):
+def test_should_be_able_to_get_statistics_for_a_service(sample_template):
-
-    assert Notification.query.count() == 0
     data = {
         'to': '+44709123456',
-        'job_id': sample_job.id,
         'service': sample_template.service,
+        'service_id': sample_template.service.id,
         'template': sample_template,
         'created_at': datetime.utcnow()
     }
 
     notification = Notification(**data)
-    dao_create_notification(notification)
+    dao_create_notification(notification, sample_template.template_type)
+
+    stats = dao_get_notification_statistics_for_service(sample_template.service.id)
+    assert len(stats) == 1
+    assert stats[0].emails_requested == 0
+    assert stats[0].sms_requested == 1
+    assert stats[0].day == notification.created_at.strftime('%Y-%m-%d')
+    assert stats[0].service_id == notification.service_id
+
+
+def test_should_be_able_to_get_all_statistics_for_a_service(sample_template):
+    data = {
+        'to': '+44709123456',
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification_1 = Notification(**data)
+    notification_2 = Notification(**data)
+    notification_3 = Notification(**data)
+    dao_create_notification(notification_1, sample_template.template_type)
+    dao_create_notification(notification_2, sample_template.template_type)
+    dao_create_notification(notification_3, sample_template.template_type)
+
+    stats = dao_get_notification_statistics_for_service(sample_template.service.id)
+    assert len(stats) == 1
+    assert stats[0].emails_requested == 0
+    assert stats[0].sms_requested == 3
+
+
+def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days(sample_template):
+    data = {
+        'to': '+44709123456',
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template
+    }
+
+    today = datetime.utcnow()
+    yesterday = datetime.utcnow() - timedelta(days=1)
+    two_days_ago = datetime.utcnow() - timedelta(days=2)
+    data.update({
+        'created_at': today
+    })
+    notification_1 = Notification(**data)
+    data.update({
+        'created_at': yesterday
+    })
+    notification_2 = Notification(**data)
+    data.update({
+        'created_at': two_days_ago
+    })
+    notification_3 = Notification(**data)
+    dao_create_notification(notification_1, sample_template.template_type)
+    dao_create_notification(notification_2, sample_template.template_type)
+    dao_create_notification(notification_3, sample_template.template_type)
+
+    stats = dao_get_notification_statistics_for_service(sample_template.service.id)
+    assert len(stats) == 3
+    assert stats[0].emails_requested == 0
+    assert stats[0].sms_requested == 1
+    assert stats[0].day == today.strftime('%Y-%m-%d')
+    assert stats[1].emails_requested == 0
+    assert stats[1].sms_requested == 1
+    assert stats[1].day == yesterday.strftime('%Y-%m-%d')
+    assert stats[2].emails_requested == 0
+    assert stats[2].sms_requested == 1
+    assert stats[2].day == two_days_ago.strftime('%Y-%m-%d')
+
+
+def test_should_be_empty_list_if_no_statistics_for_a_service(sample_service):
+    assert len(dao_get_notification_statistics_for_service(sample_service.id)) == 0
+
+
+def test_save_notification_and_create_sms_stats(sample_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -35,9 +126,251 @@ def test_save_notification_and_increment_job(sample_template, sample_job):
     assert 'sent' == notification_from_db.status
     assert Job.query.get(sample_job.id).notifications_sent == 1
 
+    stats = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_template.service.id
+    ).first()
+
+    assert stats.emails_requested == 0
+    assert stats.sms_requested == 1
+
+
+def test_save_notification_and_create_email_stats(sample_email_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_email_template.service,
+        'service_id': sample_email_template.service.id,
+        'template': sample_email_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    dao_create_notification(notification, sample_email_template.template_type)
+
+    assert Notification.query.count() == 1
+    notification_from_db = Notification.query.all()[0]
+    assert notification_from_db.id
+    assert data['to'] == notification_from_db.to
+    assert data['job_id'] == notification_from_db.job_id
+    assert data['service'] == notification_from_db.service
+    assert data['template'] == notification_from_db.template
+    assert data['created_at'] == notification_from_db.created_at
+    assert 'sent' == notification_from_db.status
+    assert Job.query.get(sample_job.id).notifications_sent == 1
+
+    stats = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_email_template.service.id
+    ).first()
+
+    assert stats.emails_requested == 1
+    assert stats.sms_requested == 0
+
+
+@freeze_time("2016-01-01 00:00:00.000000")
+def test_save_notification_handles_midnight_properly(sample_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    dao_create_notification(notification, sample_template.template_type)
+
+    assert Notification.query.count() == 1
+
+    stats = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_template.service.id
+    ).first()
+
+    assert stats.day == '2016-01-01'
+
+
+@freeze_time("2016-01-01 23:59:59.999999")
+def test_save_notification_handles_just_before_midnight_properly(sample_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    dao_create_notification(notification, sample_template.template_type)
+
+    assert Notification.query.count() == 1
+
+    stats = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_template.service.id
+    ).first()
+
+    assert stats.day == '2016-01-01'
+
+
+def test_save_notification_and_increment_email_stats(sample_email_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_email_template.service,
+        'service_id': sample_email_template.service.id,
+        'template': sample_email_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification_1 = Notification(**data)
+    notification_2 = Notification(**data)
+    dao_create_notification(notification_1, sample_email_template.template_type)
+
+    assert Notification.query.count() == 1
+
+    stats1 = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_email_template.service.id
+    ).first()
+
+    assert stats1.emails_requested == 1
+    assert stats1.sms_requested == 0
+
+    dao_create_notification(notification_2, sample_email_template)
+
+    assert Notification.query.count() == 2
+
+    stats2 = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_email_template.service.id
+    ).first()
+
+    assert stats2.emails_requested == 2
+    assert stats2.sms_requested == 0
+
+
+def test_save_notification_and_increment_sms_stats(sample_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification_1 = Notification(**data)
+    notification_2 = Notification(**data)
+    dao_create_notification(notification_1, sample_template.template_type)
+
+    assert Notification.query.count() == 1
+
+    stats1 = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_template.service.id
+    ).first()
+
+    assert stats1.emails_requested == 0
+    assert stats1.sms_requested == 1
+
+    dao_create_notification(notification_2, sample_template.template_type)
+
+    assert Notification.query.count() == 2
+
+    stats2 = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_template.service.id
+    ).first()
+
+    assert stats2.emails_requested == 0
+    assert stats2.sms_requested == 2
+
+
+def test_not_save_notification_and_not_create_stats_on_commit_error(sample_template, sample_job):
+    random_id = str(uuid.uuid4())
+
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': random_id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    with pytest.raises(SQLAlchemyError):
+        dao_create_notification(notification, sample_template.template_type)
+
+    assert Notification.query.count() == 0
+    assert Job.query.get(sample_job.id).notifications_sent == 0
+    assert NotificationStatistics.query.count() == 0
+
+
+def test_save_notification_and_increment_job(sample_template, sample_job):
+    assert Notification.query.count() == 0
+    data = {
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service': sample_template.service,
+        'service_id': sample_template.service.id,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification = Notification(**data)
+    dao_create_notification(notification, sample_template.template_type)
+
+    assert Notification.query.count() == 1
+    notification_from_db = Notification.query.all()[0]
+    assert notification_from_db.id
+    assert data['to'] == notification_from_db.to
+    assert data['job_id'] == notification_from_db.job_id
+    assert data['service'] == notification_from_db.service
+    assert data['template'] == notification_from_db.template
+    assert data['created_at'] == notification_from_db.created_at
+    assert 'sent' == notification_from_db.status
+    assert Job.query.get(sample_job.id).notifications_sent == 1
+
+    notification_2 = Notification(**data)
+    dao_create_notification(notification_2, sample_template)
+    assert Notification.query.count() == 2
+    assert Job.query.get(sample_job.id).notifications_sent == 2
+
+
+def test_should_not_increment_job_if_notification_fails_to_persist(sample_template, sample_job):
+    random_id = str(uuid.uuid4())
+
+    assert Notification.query.count() == 0
+    data = {
+        'id': random_id,
+        'to': '+44709123456',
+        'job_id': sample_job.id,
+        'service_id': sample_template.service.id,
+        'service': sample_template.service,
+        'template': sample_template,
+        'created_at': datetime.utcnow()
+    }
+
+    notification_1 = Notification(**data)
+    dao_create_notification(notification_1, sample_template.template_type)
+
+    assert Notification.query.count() == 1
+    assert Job.query.get(sample_job.id).notifications_sent == 1
+    job_last_updated_at = Job.query.get(sample_job.id).updated_at
+
+    notification_2 = Notification(**data)
+    with pytest.raises(SQLAlchemyError):
+        dao_create_notification(notification_2, sample_template.template_type)
+
+    assert Notification.query.count() == 1
+    assert Job.query.get(sample_job.id).notifications_sent == 1
+    assert Job.query.get(sample_job.id).updated_at == job_last_updated_at
+
 
 def test_save_notification_and_increment_correct_job(notify_db, notify_db_session, sample_template):
-
     job_1 = sample_job(notify_db, notify_db_session, sample_template.service)
     job_2 = sample_job(notify_db, notify_db_session, sample_template.service)
 
@@ -45,13 +378,14 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio
     data = {
         'to': '+44709123456',
         'job_id': job_1.id,
+        'service_id': sample_template.service.id,
         'service': sample_template.service,
         'template': sample_template,
         'created_at': datetime.utcnow()
     }
 
     notification = Notification(**data)
-    dao_create_notification(notification)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -67,17 +401,17 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio
 
 
 def test_save_notification_with_no_job(sample_template):
-
     assert Notification.query.count() == 0
     data = {
         'to': '+44709123456',
+        'service_id': sample_template.service.id,
         'service': sample_template.service,
         'template': sample_template,
         'created_at': datetime.utcnow()
     }
 
     notification = Notification(**data)
-    dao_create_notification(notification)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -97,18 +431,18 @@ def test_get_notification(sample_notification):
 
 
 def test_save_notification_no_job_id(sample_template):
-
     assert Notification.query.count() == 0
     to = '+44709123456'
     data = {
         'to': to,
+        'service_id': sample_template.service.id,
         'service': sample_template.service,
         'template': sample_template,
         'created_at': datetime.utcnow()
     }
 
     notification = Notification(**data)
-    dao_create_notification(notification)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -128,20 +462,28 @@ def test_get_notification_for_job(sample_notification):
 
 
 def test_get_all_notifications_for_job(notify_db, notify_db_session, sample_job):
-
     from tests.app.conftest import sample_notification
     for i in range(0, 5):
-        sample_notification(notify_db,
+        try:
-                            notify_db_session,
+            sample_notification(notify_db,
-                            service=sample_job.service,
+                                notify_db_session,
-                            template=sample_job.template,
+                                service=sample_job.service,
-                            job=sample_job)
+                                template=sample_job.template,
+                                job=sample_job)
+        except IntegrityError:
+            pass
 
     notifcations_from_db = get_notifications_for_job(sample_job.service.id, sample_job.id).items
     assert len(notifcations_from_db) == 5
+    stats = NotificationStatistics.query.filter(
+        NotificationStatistics.service_id == sample_job.service.id
+    ).first()
+
+    assert stats.emails_requested == 0
+    assert stats.sms_requested == 5
 
 
-def test_update_notification(sample_notification):
+def test_update_notification(sample_notification, sample_template):
     assert sample_notification.status == 'sent'
     sample_notification.status = 'failed'
     dao_update_notification(sample_notification)

tests/app/notifications/test_rest.py

@@ -359,7 +359,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_templat
                 (str(sample_template_with_placeholders.service.id),
                  notification_id,
                  "something_encrypted",
-                 "2016-01-01 11:09:00.061258"),
+                 "2016-01-01T11:09:00.061258"),
                 queue="sms"
             )
             assert response.status_code == 201
@@ -526,7 +526,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
                 (str(sample_template.service_id),
                  notification_id,
                  "something_encrypted",
-                 "2016-01-01 11:09:00.061258"),
+                 "2016-01-01T11:09:00.061258"),
                 queue="sms"
             )
             assert response.status_code == 201
@@ -748,7 +748,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
                  "Email Subject",
                  "sample.service@test.notify.com",
                  "something_encrypted",
-                 "2016-01-01 11:09:00.061258"),
+                 "2016-01-01T11:09:00.061258"),
                 queue="email"
             )
             assert response.status_code == 201

tests/app/template/test_rest.py

@@ -348,7 +348,7 @@ def test_should_get_only_templates_for_that_servcie(notify_api, service_factory)
                 method='POST',
                 request_body=data
             )
-            client.post(
+            resp = client.post(
                 '/service/{}/template'.format(service_1.id),
                 headers=[('Content-Type', 'application/json'), create_auth_header],
                 data=data