use ft_notification_status and notifications for job statistics

we previously always read from NotificationHistory to get the
notification status stats for a job. Now, if the job is more than three
days old read from ft_notification_status table, otherwise read from
the notifications table (to keep live updates).
This commit is contained in:
Leo Hemsted
2018-12-12 12:57:33 +00:00
parent e555a7595b
commit b80beab76c
6 changed files with 120 additions and 20 deletions

View File

@@ -186,3 +186,14 @@ def fetch_notification_status_totals_for_all_services(start_date, end_date):
else: else:
query = stats query = stats
return query.all() return query.all()
def fetch_notification_statuses_for_job(job_id):
return db.session.query(
FactNotificationStatus.notification_status.label('status'),
func.sum(FactNotificationStatus.notification_count).label('count'),
).filter(
FactNotificationStatus.job_id == job_id,
).group_by(
FactNotificationStatus.notification_status
).all()

View File

@@ -16,7 +16,7 @@ from app.models import (
JOB_STATUS_PENDING, JOB_STATUS_PENDING,
JOB_STATUS_SCHEDULED, JOB_STATUS_SCHEDULED,
LETTER_TYPE, LETTER_TYPE,
NotificationHistory, Notification,
Template, Template,
ServiceDataRetention ServiceDataRetention
) )
@@ -25,19 +25,14 @@ from app.variables import LETTER_TEST_API_FILENAME
@statsd(namespace="dao") @statsd(namespace="dao")
def dao_get_notification_outcomes_for_job(service_id, job_id): def dao_get_notification_outcomes_for_job(service_id, job_id):
query = db.session.query( return db.session.query(
func.count(NotificationHistory.status).label('count'), func.count(Notification.status).label('count'),
NotificationHistory.status Notification.status
)
return query.filter(
NotificationHistory.service_id == service_id
).filter( ).filter(
NotificationHistory.job_id == job_id Notification.service_id == service_id,
Notification.job_id == job_id
).group_by( ).group_by(
NotificationHistory.status Notification.status
).order_by(
asc(NotificationHistory.status)
).all() ).all()

View File

@@ -1,3 +1,4 @@
import dateutil
from flask import ( from flask import (
Blueprint, Blueprint,
jsonify, jsonify,
@@ -13,6 +14,7 @@ from app.dao.jobs_dao import (
dao_get_jobs_by_service_id, dao_get_jobs_by_service_id,
dao_get_future_scheduled_job_by_id_and_service_id, dao_get_future_scheduled_job_by_id_and_service_id,
dao_get_notification_outcomes_for_job) dao_get_notification_outcomes_for_job)
from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job
from app.dao.services_dao import dao_fetch_service_by_id 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.templates_dao import dao_get_template_by_id
from app.dao.notifications_dao import get_notifications_for_job from app.dao.notifications_dao import get_notifications_for_job
@@ -24,7 +26,7 @@ from app.schemas import (
) )
from app.celery.tasks import process_job from app.celery.tasks import process_job
from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANCELLED, LETTER_TYPE from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANCELLED, LETTER_TYPE
from app.utils import pagination_links from app.utils import pagination_links, midnight_n_days_ago
from app.config import QueueNames from app.config import QueueNames
from app.errors import ( from app.errors import (
register_errors, register_errors,
@@ -171,8 +173,14 @@ def get_paginated_jobs(service_id, limit_days, statuses, page):
) )
data = job_schema.dump(pagination.items, many=True).data data = job_schema.dump(pagination.items, many=True).data
for job_data in data: for job_data in data:
statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id']) created_at = dateutil.parser.parse(job_data['created_at']).replace(tzinfo=None)
job_data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics] if created_at < midnight_n_days_ago(3):
# ft_notification_status table
statistics = fetch_notification_statuses_for_job(job_data['id'])
else:
# notifications table
statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id'])
job_data['statistics'] = [{'status': statistic.status, 'count': statistic.count} for statistic in statistics]
return { return {
'data': data, 'data': data,

View File

@@ -9,11 +9,12 @@ from app.dao.fact_notification_status_dao import (
fetch_notification_status_for_service_by_month, fetch_notification_status_for_service_by_month,
fetch_notification_status_for_service_for_day, fetch_notification_status_for_service_for_day,
fetch_notification_status_for_service_for_today_and_7_previous_days, fetch_notification_status_for_service_for_today_and_7_previous_days,
fetch_notification_status_totals_for_all_services fetch_notification_status_totals_for_all_services,
fetch_notification_statuses_for_job,
) )
from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
from freezegun import freeze_time from freezegun import freeze_time
from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job
def test_update_fact_notification_status(notify_db_session): def test_update_fact_notification_status(notify_db_session):
@@ -288,3 +289,18 @@ def set_up_data():
create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0)) create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))
create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered') create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered')
create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered') create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered')
def test_fetch_notification_statuses_for_job(sample_template):
j1 = create_job(sample_template)
j2 = create_job(sample_template)
create_ft_notification_status(date(2018, 10, 1), job=j1, notification_status='created', count=1)
create_ft_notification_status(date(2018, 10, 1), job=j1, notification_status='delivered', count=2)
create_ft_notification_status(date(2018, 10, 2), job=j1, notification_status='created', count=4)
create_ft_notification_status(date(2018, 10, 1), job=j2, notification_status='created', count=8)
assert {x.status: x.count for x in fetch_notification_statuses_for_job(j1.id)} == {
'created': 5,
'delivered': 2
}

View File

@@ -19,13 +19,54 @@ from app.models import (
Job, Job,
EMAIL_TYPE, SMS_TYPE, LETTER_TYPE EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
) )
from tests.app.db import create_job, create_service, create_template from tests.app.db import create_job, create_service, create_template, create_notification
def test_should_have_decorated_notifications_dao_functions(): def test_should_have_decorated_notifications_dao_functions():
assert dao_get_notification_outcomes_for_job.__wrapped__.__name__ == 'dao_get_notification_outcomes_for_job' # noqa assert dao_get_notification_outcomes_for_job.__wrapped__.__name__ == 'dao_get_notification_outcomes_for_job' # noqa
def test_should_count_of_statuses_for_notifications_associated_with_job(sample_template, sample_job):
create_notification(sample_template, job=sample_job, status='created')
create_notification(sample_template, job=sample_job, status='created')
create_notification(sample_template, job=sample_job, status='created')
create_notification(sample_template, job=sample_job, status='sending')
create_notification(sample_template, job=sample_job, status='delivered')
results = dao_get_notification_outcomes_for_job(sample_template.service_id, sample_job.id)
assert {row.status: row.count for row in results} == {
'created': 3,
'sending': 1,
'delivered': 1,
}
def test_should_return_zero_length_array_if_no_notifications_for_job(sample_service, sample_job):
assert len(dao_get_notification_outcomes_for_job(sample_job.id, sample_service.id)) == 0
def test_should_return_notifications_only_for_this_job(sample_template):
job_1 = create_job(sample_template)
job_2 = create_job(sample_template)
create_notification(sample_template, job=job_1, status='created')
create_notification(sample_template, job=job_2, status='sent')
results = dao_get_notification_outcomes_for_job(sample_template.service_id, job_1.id)
assert {row.status: row.count for row in results} == {'created': 1}
def test_should_return_notifications_only_for_this_service(sample_notification_with_job):
other_service = create_service(service_name='one')
other_template = create_template(service=other_service)
other_job = create_job(other_template)
create_notification(other_template, job=other_job)
assert len(dao_get_notification_outcomes_for_job(sample_notification_with_job.service_id, other_job.id)) == 0
assert len(dao_get_notification_outcomes_for_job(other_service.id, sample_notification_with_job.id)) == 0
def test_create_sample_job(sample_template): def test_create_sample_job(sample_template):
assert Job.query.count() == 0 assert Job.query.count() == 0

View File

@@ -1,6 +1,6 @@
import json import json
import uuid import uuid
from datetime import datetime, timedelta from datetime import datetime, timedelta, date
from freezegun import freeze_time from freezegun import freeze_time
import pytest import pytest
@@ -12,7 +12,7 @@ from app.models import JOB_STATUS_TYPES, JOB_STATUS_PENDING
from tests import create_authorization_header from tests import create_authorization_header
from tests.conftest import set_config from tests.conftest import set_config
from tests.app.db import create_job, create_notification from tests.app.db import create_ft_notification_status, create_job, create_notification
def test_get_job_with_invalid_service_id_returns404(client, sample_service): def test_get_job_with_invalid_service_id_returns404(client, sample_service):
@@ -690,3 +690,32 @@ def test_get_all_notifications_for_job_returns_csv_format(admin_request, sample_
'row_number', 'row_number',
'recipient' 'recipient'
} }
@freeze_time('2017-06-10 12:00')
def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template):
# it's the 10th today, so 3 days should include all of 7th, 8th, 9th, and some of 10th.
just_three_days_ago = datetime(2017, 6, 6, 22, 59, 59)
not_quite_three_days_ago = just_three_days_ago + timedelta(seconds=1)
job_1 = create_job(sample_template, created_at=just_three_days_ago)
job_2 = create_job(sample_template, created_at=not_quite_three_days_ago)
# some notifications created more than three days ago, some created after the midnight cutoff
create_ft_notification_status(date(2017, 6, 6), job=job_1, notification_status='delivered', count=2)
create_ft_notification_status(date(2017, 6, 7), job=job_1, notification_status='delivered', count=4)
# job2's new enough
create_notification(job=job_2, status='created', created_at=not_quite_three_days_ago)
# this isn't picked up because the job is too new
create_ft_notification_status(date(2017, 6, 7), job=job_2, notification_status='delivered', count=8)
# this isn't picked up because we're using the ft status table for job_1 as it's old
create_notification(job=job_1, status='created', created_at=not_quite_three_days_ago)
resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id)
assert resp_json['data'][0]['id'] == str(job_2.id)
assert resp_json['data'][0]['statistics'] == [{'status': 'created', 'count': 1}]
assert resp_json['data'][1]['id'] == str(job_1.id)
assert resp_json['data'][1]['statistics'] == [{'status': 'delivered', 'count': 6}]