Add an option to group notification stats for 7 days by template

Currently, admin app requests service statistics (with notification
counts grouped by status) and template statistics (with counts by
template) in order to display the service dashboard.

Service statistics are gathered from FactNotificationStatus table
(counts for the last 7 days) combined with Notification (counts for
today).

Template statistics are currently gathered from redis cache, which
contains a separate counter per template per day. It's hard for us
to maintain consistency between redis and DB counts. Currently it
doesn't update the count for cancelled letters, counter resets in
the middle of the day might produce a wrong result for the rest of
the week and cleared redis cache can't be repopulated for services
with low data retention periods).

Since FactNotificationStatus already contains separate counts for
each template_id we can use the existing logic with some additional
filters to get separate counts for each template and status combination,
which would allow us to populate the service dashboard page from one
query response.
This commit is contained in:
Alexey Bezhan
2019-01-14 16:58:57 +00:00
parent 79f49ebdc2
commit 876346f469
2 changed files with 60 additions and 4 deletions

View File

@@ -107,12 +107,13 @@ def fetch_notification_status_for_service_for_day(bst_day, service_id):
).all()
def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, limit_days=7):
def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, by_template=False, limit_days=7):
start_date = midnight_n_days_ago(limit_days)
now = datetime.utcnow()
stats_for_7_days = db.session.query(
FactNotificationStatus.notification_type.label('notification_type'),
FactNotificationStatus.notification_status.label('status'),
*([FactNotificationStatus.template_id.label('template_id')] if by_template else []),
FactNotificationStatus.notification_count.label('count')
).filter(
FactNotificationStatus.service_id == service_id,
@@ -123,6 +124,7 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
stats_for_today = db.session.query(
Notification.notification_type.cast(db.Text),
Notification.status,
*([Notification.template_id] if by_template else []),
func.count().label('count')
).filter(
Notification.created_at >= get_london_midnight_in_utc(now),
@@ -130,14 +132,24 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
Notification.key_type != KEY_TYPE_TEST
).group_by(
Notification.notification_type,
*([Notification.template_id] if by_template else []),
Notification.status
)
all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery()
return db.session.query(
query = db.session.query(
*([Template.name, Template.is_precompiled_letter, all_stats_table.c.template_id] if by_template else []),
all_stats_table.c.notification_type,
all_stats_table.c.status,
func.cast(func.sum(all_stats_table.c.count), Integer).label('count'),
).group_by(
)
if by_template:
query = query.filter(all_stats_table.c.template_id == Template.id)
return query.group_by(
*([Template.name, Template.is_precompiled_letter, all_stats_table.c.template_id] if by_template else []),
all_stats_table.c.notification_type,
all_stats_table.c.status,
).all()

View File

@@ -2,6 +2,7 @@ from datetime import timedelta, datetime, date
from uuid import UUID
import pytest
import mock
from app.dao.fact_notification_status_dao import (
update_fact_notification_status,
@@ -188,6 +189,7 @@ def test_fetch_notification_status_for_service_for_day(notify_db_session):
def test_fetch_notification_status_for_service_for_today_and_7_previous_days(notify_db_session):
service_1 = create_service(service_name='service_1')
sms_template = create_template(service=service_1, template_type=SMS_TYPE)
sms_template_2 = create_template(service=service_1, template_type=SMS_TYPE)
email_template = create_template(service=service_1, template_type=EMAIL_TYPE)
create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=10)
@@ -197,6 +199,7 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days(not
create_ft_notification_status(date(2018, 10, 26), 'letter', service_1, count=5)
create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))
create_notification(sms_template_2, 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(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered')
@@ -220,13 +223,54 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days(not
assert results[2].notification_type == 'sms'
assert results[2].status == 'created'
assert results[2].count == 2
assert results[2].count == 3
assert results[3].notification_type == 'sms'
assert results[3].status == 'delivered'
assert results[3].count == 19
@freeze_time('2018-10-31T18:00:00')
def test_fetch_notification_status_by_template_for_service_for_today_and_7_previous_days(notify_db_session):
service_1 = create_service(service_name='service_1')
sms_template = create_template(template_name='sms Template 1', service=service_1, template_type=SMS_TYPE)
sms_template_2 = create_template(template_name='sms Template 2', service=service_1, template_type=SMS_TYPE)
email_template = create_template(service=service_1, template_type=EMAIL_TYPE)
# create unused email template
create_template(service=service_1, template_type=EMAIL_TYPE)
create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=10)
create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=11)
create_ft_notification_status(date(2018, 10, 24), 'sms', service_1, count=8)
create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, notification_status='created')
create_ft_notification_status(date(2018, 10, 29), 'email', service_1, count=3)
create_ft_notification_status(date(2018, 10, 26), 'letter', service_1, count=5)
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_2, 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')
# too early, shouldn't be included
create_notification(service_1.templates[0], created_at=datetime(2018, 10, 30, 12, 0, 0), status='delivered')
results = fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id, by_template=True)
assert [
('email Template Name', False, mock.ANY, 'email', 'delivered', 1),
('email Template Name', False, mock.ANY, 'email', 'delivered', 3),
('letter Template Name', False, mock.ANY, 'letter', 'delivered', 5),
('sms Template 1', False, mock.ANY, 'sms', 'created', 1),
('sms Template Name', False, mock.ANY, 'sms', 'created', 1),
('sms Template 1', False, mock.ANY, 'sms', 'delivered', 1),
('sms Template 2', False, mock.ANY, 'sms', 'delivered', 1),
('sms Template Name', False, mock.ANY, 'sms', 'delivered', 8),
('sms Template Name', False, mock.ANY, 'sms', 'delivered', 10),
('sms Template Name', False, mock.ANY, 'sms', 'delivered', 11),
] == sorted(results, key=lambda x: (x.notification_type, x.status, x.name, x.count))
@pytest.mark.parametrize(
"start_date, end_date, expected_email, expected_letters, expected_sms, expected_created_sms",
[