New notification stats table

- to capture the counts of things that we do
- initial commit captures when we create an email or sms

DOES NOT know about ultimate success only that we asked our partners to ship the notification

Requires some updates when we retry sending in event of error.

app/celery/tasks.py

@@ -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
@@ -93,7 +93,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_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:
@@ -162,7 +162,7 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not
             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:

app/dao/notifications_dao.py

@@ -1,16 +1,53 @@
 from flask import current_app
 from app import db
-from app.models import Notification, Job
+from app.models import Notification, Job, ServiceNotificationStats, TEMPLATE_TYPE_SMS, TEMPLATE_TYPE_EMAIL
 from sqlalchemy import desc
+from datetime import datetime
 
 
-def dao_create_notification(notification):
+def dao_create_notification(notification, notification_type):
-    if notification.job_id:
+    try:
-        db.session.query(Job).filter_by(
+        if notification.job_id:
-            id=notification.job_id
+            update_job_sent_count(notification)
-        ).update({Job.notifications_sent: Job.notifications_sent + 1})
+
-    db.session.add(notification)
+        day = datetime.utcnow().strftime('%Y-%m-%d')
-    db.session.commit()
+
+        if notification_type == TEMPLATE_TYPE_SMS:
+            update = {
+                ServiceNotificationStats.sms_requested: ServiceNotificationStats.sms_requested + 1
+            }
+        else:
+            update = {
+                ServiceNotificationStats.emails_requested: ServiceNotificationStats.emails_requested + 1
+            }
+
+        result = db.session.query(ServiceNotificationStats).filter_by(
+            day=day,
+            service_id=notification.service_id
+        ).update(update)
+
+        if result == 0:
+            stats = ServiceNotificationStats(
+                day=day,
+                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_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 ServiceNotificationStats(db.Model):
+    __tablename__ = 'service_notification_stats'
+
+    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):

migrations/versions/0036_notification_stats.py

@@ -0,0 +1,41 @@
+"""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():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.create_table('service_notification_stats',
+    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'), 'service_notification_stats', ['service_id'], unique=False)
+    ### end Alembic commands ###
+
+
+def downgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.drop_index(op.f('ix_service_notification_stats_service_id'), table_name='service_notification_stats')
+    op.drop_table('service_notification_stats')
+    ### end Alembic commands ###

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,4 +1,9 @@
-from app.models import Notification, Job
+import pytest
+import uuid
+from freezegun import freeze_time
+import random
+from sqlalchemy.exc import SQLAlchemyError, IntegrityError
+from app.models import Notification, Job, ServiceNotificationStats
 from datetime import datetime
 from app.dao.notifications_dao import (
     dao_create_notification,
@@ -10,19 +15,19 @@ from app.dao.notifications_dao import (
 from tests.app.conftest import sample_job
 
 
-def test_save_notification_and_increment_job(sample_template, sample_job):
+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)
+    dao_create_notification(notification, sample_template.template_type)
 
     assert Notification.query.count() == 1
     notification_from_db = Notification.query.all()[0]
@@ -35,9 +40,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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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 ServiceNotificationStats.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 +292,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 +315,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 +345,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 +376,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 = ServiceNotificationStats.query.filter(
+        ServiceNotificationStats.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/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