Merge pull request #1024 from alphagov/job-stats-for-dashboard

Change the job_statistics table structure

app/dao/statistics_dao.py

@@ -60,7 +60,10 @@ def timeout_job_counts(notifications_type, timeout_start):
         ).update({
             sent: sent_count,
             failed: failed_count,
-            delivered: delivered_count
+            delivered: delivered_count,
+            'sent': sent_count,
+            'delivered': delivered_count,
+            'failed': failed_count
         }, synchronize_session=False)
     return total_updated
 
@@ -87,11 +90,13 @@ def create_or_update_job_sending_statistics(notification):
 @transactional
 def __update_job_stats_sent_count(notification):
     column = columns(notification.notification_type, 'sent')
+    new_column = 'sent'
 
     return db.session.query(JobStatistics).filter_by(
         job_id=notification.job_id,
     ).update({
-        column: column + 1
+        column: column + 1,
+        new_column: column + 1
     })
 
 
@@ -102,7 +107,8 @@ def __insert_job_stats(notification):
         emails_sent=1 if notification.notification_type == EMAIL_TYPE else 0,
         sms_sent=1 if notification.notification_type == SMS_TYPE else 0,
         letters_sent=1 if notification.notification_type == LETTER_TYPE else 0,
-        updated_at=datetime.utcnow()
+        updated_at=datetime.utcnow(),
+        sent=1
     )
     db.session.add(stats)
 
@@ -131,10 +137,12 @@ def columns(notification_type, status):
 def update_job_stats_outcome_count(notification):
     if notification.status in NOTIFICATION_STATUS_TYPES_FAILED:
         column = columns(notification.notification_type, 'failed')
+        new_column = 'failed'
 
     elif notification.status in [NOTIFICATION_DELIVERED,
                                  NOTIFICATION_SENT] and notification.notification_type != LETTER_TYPE:
         column = columns(notification.notification_type, 'delivered')
+        new_column = 'delivered'
 
     else:
         column = None
@@ -143,7 +151,8 @@ def update_job_stats_outcome_count(notification):
         return db.session.query(JobStatistics).filter_by(
             job_id=notification.job_id,
         ).update({
-            column: column + 1
+            column: column + 1,
+            new_column: column + 1
         })
     else:
         return 0

app/models.py

@@ -1126,6 +1126,9 @@ class JobStatistics(db.Model):
     sms_failed = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0)
     letters_sent = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0)
     letters_failed = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0)
+    sent = db.Column(db.BigInteger, index=False, unique=False, nullable=True, default=0)
+    delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=True, default=0)
+    failed = db.Column(db.BigInteger, index=False, unique=False, nullable=True, default=0)
     created_at = db.Column(
         db.DateTime,
         index=False,

migrations/versions/0094_job_stats_update.py

@@ -0,0 +1,25 @@
+"""empty message
+
+Revision ID: 0094_job_stats_update
+Revises: 0093_data_gov_uk
+Create Date: 2017-06-06 14:37:30.051647
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = '0094_job_stats_update'
+down_revision = '0093_data_gov_uk'
+
+
+def upgrade():
+    op.add_column('job_statistics', sa.Column('sent', sa.BigInteger(), nullable=True))
+    op.add_column('job_statistics', sa.Column('delivered', sa.BigInteger(), nullable=True))
+    op.add_column('job_statistics', sa.Column('failed', sa.BigInteger(), nullable=True))
+
+
+def downgrade():
+    op.drop_column('job_statistics', 'sent')
+    op.drop_column('job_statistics', 'failed')
+    op.drop_column('job_statistics', 'delivered')

tests/app/dao/test_statistics_dao.py

@@ -199,6 +199,10 @@ def test_should_update_a_stats_entry_with_its_success_outcome_for_a_job(
     assert stat.sms_failed == 0
     assert stat.letters_failed == 0
 
+    assert stat.sent == email_count + sms_count + letter_count
+    assert stat.delivered == email_count + sms_count
+    assert stat.failed == 0
+
 
 @pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [
     (SMS_TYPE, 1, 0, 0, NOTIFICATION_TECHNICAL_FAILURE),
@@ -264,6 +268,10 @@ def test_should_update_a_stats_entry_with_its_error_outcomes_for_a_job(
     assert stat.emails_delivered == 0
     assert stat.sms_delivered == 0
 
+    assert stat.sent == email_count + sms_count + letter_count
+    assert stat.delivered == 0
+    assert stat.failed == email_count + sms_count + letter_count
+
 
 @pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [
     (SMS_TYPE, 1, 0, 0, NOTIFICATION_DELIVERED),
@@ -326,6 +334,10 @@ def test_should_update_a_stats_entry_with_its_success_outcomes_for_a_job(
     assert stat.emails_delivered == email_count
     assert stat.sms_delivered == sms_count
 
+    assert stat.sent == email_count + sms_count + letter_count
+    assert stat.delivered == 0 if notification_type == LETTER_TYPE else 1
+    assert stat.failed == 0
+
 
 @pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [
     (SMS_TYPE, 1, 0, 0, NOTIFICATION_PENDING),
@@ -394,6 +406,10 @@ def test_should_not_update_job_stats_if_irrelevant_status(
     assert stat.emails_delivered == 0
     assert stat.sms_delivered == 0
 
+    assert stat.sent == email_count + sms_count + letter_count
+    assert stat.delivered == 0
+    assert stat.failed == 0
+
 
 @pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count', [
     (SMS_TYPE, 2, 1, 1),
@@ -480,41 +496,52 @@ def test_inserting_one_type_of_notification_maintains_other_counts(
     assert updated_stats[0].sms_sent == sms_count
     assert updated_stats[0].letters_sent == letter_count
 
+    if notification_type == EMAIL_TYPE:
+        assert updated_stats[0].sent == email_count
+    elif notification_type == SMS_TYPE:
+        assert updated_stats[0].sent == sms_count
+    elif notification_type == LETTER_TYPE:
+        assert updated_stats[0].sent == letter_count
+
 
 def test_updating_one_type_of_notification_to_success_maintains_other_counts(
         notify_db,
         notify_db_session,
-        sample_job,
+        sample_service,
         sample_letter_template
 ):
-    sms_template = sample_template(notify_db, notify_db_session, service=sample_job.service)
+    job_1 = sample_job(notify_db, notify_db_session, service=sample_service)
-    email_template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
+    job_2 = sample_job(notify_db, notify_db_session, service=sample_service)
+    job_3 = sample_job(notify_db, notify_db_session, service=sample_service)
+
+    sms_template = sample_template(notify_db, notify_db_session, service=sample_service)
+    email_template = sample_email_template(notify_db, notify_db_session, service=sample_service)
     letter_template = sample_letter_template
 
     letter = sample_notification(
         notify_db,
         notify_db_session,
-        service=sample_job.service,
+        service=sample_service,
         template=letter_template,
-        job=sample_job,
+        job=job_1,
         status=NOTIFICATION_CREATED
     )
 
     email = sample_notification(
         notify_db,
         notify_db_session,
-        service=sample_job.service,
+        service=sample_service,
         template=email_template,
-        job=sample_job,
+        job=job_2,
         status=NOTIFICATION_CREATED
     )
 
     sms = sample_notification(
         notify_db,
         notify_db_session,
-        service=sample_job.service,
+        service=sample_service,
         template=sms_template,
-        job=sample_job,
+        job=job_3,
         status=NOTIFICATION_CREATED
     )
 
@@ -530,49 +557,76 @@ def test_updating_one_type_of_notification_to_success_maintains_other_counts(
     update_job_stats_outcome_count(email)
     update_job_stats_outcome_count(sms)
 
-    stats = JobStatistics.query.all()
+    stats = JobStatistics.query.order_by(JobStatistics.created_at).all()
-    assert len(stats) == 1
+    assert len(stats) == 3
-    assert stats[0].emails_sent == 1
-    assert stats[0].sms_sent == 1
     assert stats[0].letters_sent == 1
-    assert stats[0].emails_delivered == 1
+    assert stats[0].emails_sent == 0
-    assert stats[0].sms_delivered == 1
+    assert stats[0].sms_sent == 0
+    assert stats[0].emails_delivered == 0
+    assert stats[0].sms_delivered == 0
+
+    assert stats[1].letters_sent == 0
+    assert stats[1].emails_sent == 1
+    assert stats[1].sms_sent == 0
+    assert stats[1].emails_delivered == 1
+    assert stats[1].sms_delivered == 0
+
+    assert stats[2].letters_sent == 0
+    assert stats[2].emails_sent == 0
+    assert stats[2].sms_sent == 1
+    assert stats[2].emails_delivered == 0
+    assert stats[2].sms_delivered == 1
+
+    assert stats[0].sent == 1
+    assert stats[0].delivered == 0
+    assert stats[0].failed == 0
+
+    assert stats[1].sent == 1
+    assert stats[1].delivered == 1
+    assert stats[1].failed == 0
+
+    assert stats[2].sent == 1
+    assert stats[2].delivered == 1
+    assert stats[2].failed == 0
 
 
 def test_updating_one_type_of_notification_to_error_maintains_other_counts(
         notify_db,
         notify_db_session,
-        sample_job,
+        sample_service,
         sample_letter_template
 ):
-    sms_template = sample_template(notify_db, notify_db_session, service=sample_job.service)
+    job_1 = sample_job(notify_db, notify_db_session, service=sample_service)
-    email_template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
+    job_2 = sample_job(notify_db, notify_db_session, service=sample_service)
+    job_3 = sample_job(notify_db, notify_db_session, service=sample_service)
+    sms_template = sample_template(notify_db, notify_db_session, service=sample_service)
+    email_template = sample_email_template(notify_db, notify_db_session, service=sample_service)
     letter_template = sample_letter_template
 
     letter = sample_notification(
         notify_db,
         notify_db_session,
-        service=sample_job.service,
+        service=sample_service,
         template=letter_template,
-        job=sample_job,
+        job=job_1,
         status=NOTIFICATION_CREATED
     )
 
     email = sample_notification(
         notify_db,
         notify_db_session,
-        service=sample_job.service,
+        service=sample_service,
         template=email_template,
-        job=sample_job,
+        job=job_2,
         status=NOTIFICATION_CREATED
     )
 
     sms = sample_notification(
         notify_db,
         notify_db_session,
-        service=sample_job.service,
+        service=sample_service,
         template=sms_template,
-        job=sample_job,
+        job=job_3,
         status=NOTIFICATION_CREATED
     )
 
@@ -588,20 +642,50 @@ def test_updating_one_type_of_notification_to_error_maintains_other_counts(
     update_job_stats_outcome_count(email)
     update_job_stats_outcome_count(sms)
 
-    stats = JobStatistics.query.all()
+    stats = JobStatistics.query.order_by(JobStatistics.created_at).all()
-    assert len(stats) == 1
+    assert len(stats) == 3
-    assert stats[0].emails_sent == 1
+    assert stats[0].emails_sent == 0
-    assert stats[0].sms_sent == 1
+    assert stats[0].sms_sent == 0
     assert stats[0].letters_sent == 1
     assert stats[0].emails_delivered == 0
     assert stats[0].sms_delivered == 0
-    assert stats[0].sms_failed == 1
+    assert stats[0].sms_failed == 0
-    assert stats[0].emails_failed == 1
+    assert stats[0].emails_failed == 0
+    assert stats[0].letters_failed == 1
+
+    assert stats[1].emails_sent == 1
+    assert stats[1].sms_sent == 0
+    assert stats[1].letters_sent == 0
+    assert stats[1].emails_delivered == 0
+    assert stats[1].sms_delivered == 0
+    assert stats[1].sms_failed == 0
+    assert stats[1].emails_failed == 1
+    assert stats[1].letters_failed == 0
+
+    assert stats[2].emails_sent == 0
+    assert stats[2].sms_sent == 1
+    assert stats[2].letters_sent == 0
+    assert stats[2].emails_delivered == 0
+    assert stats[2].sms_delivered == 0
+    assert stats[2].sms_failed == 1
+    assert stats[2].emails_failed == 0
+    assert stats[1].letters_failed == 0
+
+    assert stats[0].sent == 1
+    assert stats[0].delivered == 0
+    assert stats[0].failed == 1
+
+    assert stats[1].sent == 1
+    assert stats[1].delivered == 0
+    assert stats[1].failed == 1
+
+    assert stats[2].sent == 1
+    assert stats[2].delivered == 0
+    assert stats[2].failed == 1
 
 
-def test_will_not_timeout_job_counts_before_notification_timeouts(notify_db, notify_db_session, sample_job):
+def test_will_not_timeout_job_counts_before_notification_timeouts(notify_db, notify_db_session,
-    sms_template = sample_template(notify_db, notify_db_session, service=sample_job.service)
+                                                                  sample_job, sample_template):
-    email_template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
 
     one_minute_ago = datetime.utcnow() - timedelta(minutes=1)
 
@@ -609,43 +693,51 @@ def test_will_not_timeout_job_counts_before_notification_timeouts(notify_db, not
         notify_db,
         notify_db_session,
         service=sample_job.service,
-        template=sms_template,
+        template=sample_template,
         job=sample_job,
         status=NOTIFICATION_CREATED
     )
 
-    email = sample_notification(
+    sms_2 = sample_notification(
         notify_db,
         notify_db_session,
         service=sample_job.service,
-        template=email_template,
+        template=sample_template,
         job=sample_job,
         status=NOTIFICATION_CREATED
     )
 
-    create_or_update_job_sending_statistics(email)
     create_or_update_job_sending_statistics(sms)
+    create_or_update_job_sending_statistics(sms_2)
 
     JobStatistics.query.update({JobStatistics.created_at: one_minute_ago})
 
-    intial_stats = JobStatistics.query.all()
+    initial_stats = JobStatistics.query.all()
 
-    assert intial_stats[0].emails_sent == 1
+    assert initial_stats[0].emails_sent == 0
-    assert intial_stats[0].sms_sent == 1
+    assert initial_stats[0].sms_sent == 2
-    assert intial_stats[0].emails_delivered == 0
+    assert initial_stats[0].emails_delivered == 0
-    assert intial_stats[0].sms_delivered == 0
+    assert initial_stats[0].sms_delivered == 0
-    assert intial_stats[0].sms_failed == 0
+    assert initial_stats[0].sms_failed == 0
-    assert intial_stats[0].emails_failed == 0
+    assert initial_stats[0].emails_failed == 0
+
+    assert initial_stats[0].sent == 2
+    assert initial_stats[0].delivered == 0
+    assert initial_stats[0].failed == 0
 
     dao_timeout_job_statistics(61)
     updated_stats = JobStatistics.query.all()
-    assert updated_stats[0].emails_sent == 1
+    assert updated_stats[0].emails_sent == 0
-    assert updated_stats[0].sms_sent == 1
+    assert updated_stats[0].sms_sent == 2
     assert updated_stats[0].emails_delivered == 0
     assert updated_stats[0].sms_delivered == 0
     assert updated_stats[0].sms_failed == 0
     assert updated_stats[0].emails_failed == 0
 
+    assert initial_stats[0].sent == 2
+    assert initial_stats[0].delivered == 0
+    assert initial_stats[0].failed == 0
+
 
 @pytest.mark.parametrize('notification_type, sms_count, email_count', [
     (SMS_TYPE, 3, 0),
@@ -688,6 +780,9 @@ def test_timeout_job_counts_timesout_multiple_jobs(
         assert stats.sms_delivered == 0
         assert stats.sms_failed == 0
         assert stats.emails_failed == 0
+        assert stats.sent == email_count + sms_count
+        assert stats.delivered == 0
+        assert stats.failed == 0
 
     dao_timeout_job_statistics(1)
     updated_stats = JobStatistics.query.all()
@@ -698,6 +793,9 @@ def test_timeout_job_counts_timesout_multiple_jobs(
         assert stats.sms_delivered == 0
         assert stats.sms_failed == sms_count
         assert stats.emails_failed == email_count
+        assert stats.sent == email_count + sms_count
+        assert stats.delivered == 0
+        assert stats.failed == email_count + sms_count
 
 
 count_notifications = len(NOTIFICATION_STATUS_TYPES)
@@ -754,17 +852,13 @@ def test_timeout_job_sets_all_non_delivered_emails_to_error_and_doesnt_affect_sm
     assert initial_stats[0].sms_failed == 0
     assert initial_stats[0].emails_failed == 0
 
-    all = JobStatistics.query.all()
+    assert initial_stats[0].sent == count_notifications
-    for a in all:
+    assert initial_stats[0].delivered == 0
-        print(a)
+    assert initial_stats[0].failed == 0
 
     # timeout the notifications
     dao_timeout_job_statistics(1)
 
-    all = JobStatistics.query.all()
-    for a in all:
-        print(a)
-
     # after timeout all delivered states are success and ALL other states are failed
     updated_stats = JobStatistics.query.filter_by(job_id=email_job.id).all()
     assert updated_stats[0].emails_sent == count_notifications
@@ -774,6 +868,10 @@ def test_timeout_job_sets_all_non_delivered_emails_to_error_and_doesnt_affect_sm
     assert updated_stats[0].sms_failed == 0
     assert updated_stats[0].emails_failed == count_error_notifications
 
+    assert initial_stats[0].sent == count_notifications
+    assert initial_stats[0].delivered == count_success_notifications
+    assert initial_stats[0].failed == count_error_notifications
+
     sms_stats = JobStatistics.query.filter_by(job_id=sms_job.id).all()
     assert sms_stats[0].emails_sent == 0
     assert sms_stats[0].sms_sent == 1
@@ -781,6 +879,9 @@ def test_timeout_job_sets_all_non_delivered_emails_to_error_and_doesnt_affect_sm
     assert sms_stats[0].sms_delivered == 0
     assert sms_stats[0].sms_failed == 1
     assert sms_stats[0].emails_failed == 0
+    assert sms_stats[0].sent == 1
+    assert sms_stats[0].delivered == 0
+    assert sms_stats[0].failed == 1
 
 
 # this test is as above, but for SMS not email
@@ -810,6 +911,10 @@ def test_timeout_job_sets_all_non_delivered_states_to_error(
         assert stats.sms_failed == 0
         assert stats.emails_failed == 0
 
+        assert stats.sent == count_notifications
+        assert stats.delivered == 0
+        assert stats.failed == 0
+
     dao_timeout_job_statistics(1)
     updated_stats = JobStatistics.query.all()
 
@@ -820,3 +925,7 @@ def test_timeout_job_sets_all_non_delivered_states_to_error(
         assert stats.sms_delivered == count_success_notifications
         assert stats.sms_failed == count_error_notifications
         assert stats.emails_failed == 0
+
+        assert stats.sent == count_notifications
+        assert stats.delivered == count_success_notifications
+        assert stats.failed == count_error_notifications