From 9015ffef4855a6c54543c78117e73c357ae55172 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 30 Oct 2018 16:31:57 +0000 Subject: [PATCH 1/5] Fetch notification statistics for last 7 days frpm FactNotificationStatus --- app/dao/fact_notification_status_dao.py | 20 ++++++- .../dao/test_fact_notification_status_dao.py | 52 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 7f2d7a272..a676e2b56 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -8,7 +8,7 @@ from sqlalchemy.types import DateTime from app import db from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST -from app.utils import convert_bst_to_utc, get_london_midnight_in_utc +from app.utils import convert_bst_to_utc, get_london_midnight_in_utc, midnight_n_days_ago def fetch_notification_status_for_day(process_day, service_id=None): @@ -95,6 +95,24 @@ def fetch_notification_status_for_service_by_month(start_date, end_date, service ).all() +def fetch_notification_status_for_service_for_7_days(service_id): + start_date = midnight_n_days_ago(7) + return db.session.query( + FactNotificationStatus.bst_date, + FactNotificationStatus.notification_type, + FactNotificationStatus.notification_status, + func.sum(FactNotificationStatus.notification_count).label('count') + ).filter( + FactNotificationStatus.service_id == service_id, + FactNotificationStatus.bst_date >= start_date, + FactNotificationStatus.key_type != KEY_TYPE_TEST + ).group_by( + FactNotificationStatus.bst_date, + FactNotificationStatus.notification_type, + FactNotificationStatus.notification_status, + ).all() + + def fetch_notification_status_for_service_for_day(bst_day, service_id): return db.session.query( # return current month as a datetime so the data has the same shape as the ft_notification_status query diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index e2eea0513..9bf4101eb 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -5,9 +5,11 @@ from app.dao.fact_notification_status_dao import ( update_fact_notification_status, fetch_notification_status_for_day, fetch_notification_status_for_service_by_month, + fetch_notification_status_for_service_for_7_days, fetch_notification_status_for_service_for_day, ) from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM +from freezegun import freeze_time from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status @@ -135,6 +137,56 @@ def test_fetch_notification_status_for_service_by_month(notify_db_session): assert results[3].count == 1 +@freeze_time('2018-10-30T10:00:00') +def test_fetch_notification_status_for_service_for_7_days(notify_db_session): + service_1 = create_service(service_name='service_1') + service_2 = create_service(service_name='service_2') + + create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=10) + create_ft_notification_status(date(2018, 10, 23), '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) + + # not included - too early + create_ft_notification_status(date(2018, 10, 22), 'sms', service_1) + # not included - wrong service + create_ft_notification_status(date(2018, 10, 29), 'sms', service_2) + # not included - test keys + create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, key_type=KEY_TYPE_TEST) + results = sorted( + fetch_notification_status_for_service_for_7_days(service_1.id), + key=lambda x: (x.bst_date, x.notification_type, x.notification_status) + ) + + assert len(results) == 5 + + assert results[2].bst_date == date(2018, 10, 29) + assert results[2].notification_type == 'email' + assert results[2].notification_status == 'delivered' + assert results[2].count == 3 + + assert results[1].bst_date == date(2018, 10, 26) + assert results[1].notification_type == 'letter' + assert results[1].notification_status == 'delivered' + assert results[1].count == 5 + + assert results[3].bst_date == date(2018, 10, 29) + assert results[3].notification_type == 'sms' + assert results[3].notification_status == 'created' + assert results[3].count == 1 + + assert results[0].bst_date == date(2018, 10, 23) + assert results[0].notification_type == 'sms' + assert results[0].notification_status == 'delivered' + assert results[0].count == 8 + + assert results[4].bst_date == date(2018, 10, 29) + assert results[4].notification_type == 'sms' + assert results[4].notification_status == 'delivered' + assert results[4].count == 10 + + def test_fetch_notification_status_for_service_for_day(notify_db_session): service_1 = create_service(service_name='service_1') service_2 = create_service(service_name='service_2') From 8a3dc8e039023b60ec13be1a73e785f04df5f852 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 30 Oct 2018 17:15:15 +0000 Subject: [PATCH 2/5] Refactor --- tests/app/dao/test_fact_notification_status_dao.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 9bf4101eb..c522cbe8e 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -154,10 +154,7 @@ def test_fetch_notification_status_for_service_for_7_days(notify_db_session): create_ft_notification_status(date(2018, 10, 29), 'sms', service_2) # not included - test keys create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, key_type=KEY_TYPE_TEST) - results = sorted( - fetch_notification_status_for_service_for_7_days(service_1.id), - key=lambda x: (x.bst_date, x.notification_type, x.notification_status) - ) + results = fetch_notification_status_for_service_for_7_days(service_1.id) assert len(results) == 5 From c37c3997413189387e875567b65089bf4a2aa037 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 6 Nov 2018 11:38:50 +0000 Subject: [PATCH 3/5] Add results for notification stats for last 7 days and today --- app/dao/fact_notification_status_dao.py | 54 +++++++---- .../dao/test_fact_notification_status_dao.py | 94 +++++++++---------- 2 files changed, 81 insertions(+), 67 deletions(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index a676e2b56..c9210a958 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -95,24 +95,6 @@ def fetch_notification_status_for_service_by_month(start_date, end_date, service ).all() -def fetch_notification_status_for_service_for_7_days(service_id): - start_date = midnight_n_days_ago(7) - return db.session.query( - FactNotificationStatus.bst_date, - FactNotificationStatus.notification_type, - FactNotificationStatus.notification_status, - func.sum(FactNotificationStatus.notification_count).label('count') - ).filter( - FactNotificationStatus.service_id == service_id, - FactNotificationStatus.bst_date >= start_date, - FactNotificationStatus.key_type != KEY_TYPE_TEST - ).group_by( - FactNotificationStatus.bst_date, - FactNotificationStatus.notification_type, - FactNotificationStatus.notification_status, - ).all() - - def fetch_notification_status_for_service_for_day(bst_day, service_id): return db.session.query( # return current month as a datetime so the data has the same shape as the ft_notification_status query @@ -129,3 +111,39 @@ def fetch_notification_status_for_service_for_day(bst_day, service_id): Notification.notification_type, Notification.status ).all() + + +def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id): + start_date = midnight_n_days_ago(7) + now = datetime.utcnow() + stats_for_7_days = db.session.query( + FactNotificationStatus.notification_type.label('notification_type'), + FactNotificationStatus.notification_status.label('notification_status'), + FactNotificationStatus.notification_count.label('count') + ).filter( + FactNotificationStatus.service_id == service_id, + FactNotificationStatus.bst_date >= start_date, + FactNotificationStatus.key_type != KEY_TYPE_TEST + ) + + stats_for_today = db.session.query( + Notification.notification_type.cast(db.Text), + Notification.status.label('notification_status'), + func.count().label('count') + ).filter( + Notification.created_at >= get_london_midnight_in_utc(now), + Notification.service_id == service_id, + Notification.key_type != KEY_TYPE_TEST + ).group_by( + Notification.notification_type, + Notification.status + ) + all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery() + return db.session.query( + all_stats_table.c.notification_type, + all_stats_table.c.notification_status, + func.sum(all_stats_table.c.count).label('count') + ).group_by( + all_stats_table.c.notification_type, + all_stats_table.c.notification_status, + ).all() diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index c522cbe8e..7bc427cb7 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -5,10 +5,10 @@ from app.dao.fact_notification_status_dao import ( update_fact_notification_status, fetch_notification_status_for_day, fetch_notification_status_for_service_by_month, - fetch_notification_status_for_service_for_7_days, fetch_notification_status_for_service_for_day, + fetch_notification_status_for_service_for_today_and_7_previous_days ) -from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM +from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE from freezegun import freeze_time from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status @@ -137,53 +137,6 @@ def test_fetch_notification_status_for_service_by_month(notify_db_session): assert results[3].count == 1 -@freeze_time('2018-10-30T10:00:00') -def test_fetch_notification_status_for_service_for_7_days(notify_db_session): - service_1 = create_service(service_name='service_1') - service_2 = create_service(service_name='service_2') - - create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=10) - create_ft_notification_status(date(2018, 10, 23), '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) - - # not included - too early - create_ft_notification_status(date(2018, 10, 22), 'sms', service_1) - # not included - wrong service - create_ft_notification_status(date(2018, 10, 29), 'sms', service_2) - # not included - test keys - create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, key_type=KEY_TYPE_TEST) - results = fetch_notification_status_for_service_for_7_days(service_1.id) - - assert len(results) == 5 - - assert results[2].bst_date == date(2018, 10, 29) - assert results[2].notification_type == 'email' - assert results[2].notification_status == 'delivered' - assert results[2].count == 3 - - assert results[1].bst_date == date(2018, 10, 26) - assert results[1].notification_type == 'letter' - assert results[1].notification_status == 'delivered' - assert results[1].count == 5 - - assert results[3].bst_date == date(2018, 10, 29) - assert results[3].notification_type == 'sms' - assert results[3].notification_status == 'created' - assert results[3].count == 1 - - assert results[0].bst_date == date(2018, 10, 23) - assert results[0].notification_type == 'sms' - assert results[0].notification_status == 'delivered' - assert results[0].count == 8 - - assert results[4].bst_date == date(2018, 10, 29) - assert results[4].notification_type == 'sms' - assert results[4].notification_status == 'delivered' - assert results[4].count == 10 - - def test_fetch_notification_status_for_service_for_day(notify_db_session): service_1 = create_service(service_name='service_1') service_2 = create_service(service_name='service_2') @@ -224,3 +177,46 @@ def test_fetch_notification_status_for_service_for_day(notify_db_session): assert results[1].notification_type == 'sms' assert results[1].notification_status == 'delivered' assert results[1].count == 1 + + +@freeze_time('2018-10-31T18:00:00') +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) + 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, 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(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 = sorted( + fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id), + key=lambda x: (x.notification_type, x.notification_status) + ) + + assert len(results) == 4 + + assert results[0].notification_type == 'email' + assert results[0].notification_status == 'delivered' + assert results[0].count == 4 + + assert results[1].notification_type == 'letter' + assert results[1].notification_status == 'delivered' + assert results[1].count == 5 + + assert results[2].notification_type == 'sms' + assert results[2].notification_status == 'created' + assert results[2].count == 2 + + assert results[3].notification_type == 'sms' + assert results[3].notification_status == 'delivered' + assert results[3].count == 19 From c146b866439086bebcb758a9058e4cb3a5668189 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 6 Nov 2018 13:30:37 +0000 Subject: [PATCH 4/5] Adjust attribute key names and data types to work with format_statistics --- app/dao/fact_notification_status_dao.py | 16 ++++++++-------- app/service/rest.py | 5 +++-- .../app/dao/test_fact_notification_status_dao.py | 10 +++++----- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index c9210a958..017eb7e9a 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -4,7 +4,7 @@ from flask import current_app from sqlalchemy import func from sqlalchemy.dialects.postgresql import insert from sqlalchemy.sql.expression import literal -from sqlalchemy.types import DateTime +from sqlalchemy.types import DateTime, Integer from app import db from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST @@ -113,12 +113,12 @@ 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): - start_date = midnight_n_days_ago(7) +def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, 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('notification_status'), + FactNotificationStatus.notification_status.label('status'), FactNotificationStatus.notification_count.label('count') ).filter( FactNotificationStatus.service_id == service_id, @@ -128,7 +128,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.label('notification_status'), + Notification.status, func.count().label('count') ).filter( Notification.created_at >= get_london_midnight_in_utc(now), @@ -141,9 +141,9 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_ all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery() return db.session.query( all_stats_table.c.notification_type, - all_stats_table.c.notification_status, - func.sum(all_stats_table.c.count).label('count') + all_stats_table.c.status, + func.cast(func.sum(all_stats_table.c.count), Integer).label('count'), ).group_by( all_stats_table.c.notification_type, - all_stats_table.c.notification_status, + all_stats_table.c.status, ).all() diff --git a/app/service/rest.py b/app/service/rest.py index d2e383817..ddb70dae5 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -20,7 +20,8 @@ from app.dao.api_key_dao import ( expire_api_key) from app.dao.fact_notification_status_dao import ( 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 ) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service from app.dao.organisation_dao import dao_get_organisation_by_service_id @@ -433,7 +434,7 @@ def get_service_statistics(service_id, today_only, limit_days=7): if today_only: stats = dao_fetch_todays_stats_for_service(service_id) else: - stats = dao_fetch_stats_for_service(service_id, limit_days=limit_days) + stats = fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, limit_days=limit_days) return statistics.format_statistics(stats) diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 7bc427cb7..5d0296b15 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -200,23 +200,23 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days(not results = sorted( fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id), - key=lambda x: (x.notification_type, x.notification_status) + key=lambda x: (x.notification_type, x.status) ) assert len(results) == 4 assert results[0].notification_type == 'email' - assert results[0].notification_status == 'delivered' + assert results[0].status == 'delivered' assert results[0].count == 4 assert results[1].notification_type == 'letter' - assert results[1].notification_status == 'delivered' + assert results[1].status == 'delivered' assert results[1].count == 5 assert results[2].notification_type == 'sms' - assert results[2].notification_status == 'created' + assert results[2].status == 'created' assert results[2].count == 2 assert results[3].notification_type == 'sms' - assert results[3].notification_status == 'delivered' + assert results[3].status == 'delivered' assert results[3].count == 19 From 5d806c2437642f72bc4ff030ba37a2a20d3967bb Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 6 Nov 2018 14:40:40 +0000 Subject: [PATCH 5/5] Adjust tests to new way of querying for notification stats --- app/service/rest.py | 1 - tests/app/service/test_rest.py | 4 ++-- tests/app/service/test_statistics_rest.py | 7 +++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index ddb70dae5..ce0b37048 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -48,7 +48,6 @@ from app.dao.services_dao import ( dao_fetch_all_services_by_user, dao_fetch_monthly_historical_usage_by_template_for_service, dao_fetch_service_by_id, - dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_todays_stats_for_all_services, dao_resume_service, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 7dfaad89a..13b0cb422 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -34,6 +34,7 @@ from tests.app.conftest import ( sample_notification_with_job ) from tests.app.db import ( + create_ft_notification_status, create_service, create_service_with_inbound_number, create_template, @@ -1469,8 +1470,7 @@ def test_set_sms_prefixing_for_service_cant_be_none( ], ids=['seven_days', 'today']) def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_service, today_only, stats): with notify_api.test_request_context(), notify_api.test_client() as client: - with freeze_time('2000-01-01T12:00:00'): - create_sample_notification(notify_db, notify_db_session, status='delivered') + create_ft_notification_status(date(2000, 1, 1), 'sms', sample_service, count=1) with freeze_time('2000-01-02T12:00:00'): create_sample_notification(notify_db, notify_db_session, status='created') resp = client.get( diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index 4fe36a62a..5b9719637 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -1,5 +1,5 @@ import uuid -from datetime import datetime +from datetime import datetime, date import pytest from freezegun import freeze_time @@ -133,9 +133,8 @@ def test_get_template_usage_by_month_returns_two_templates(admin_request, sample (False, {'requested': 2, 'delivered': 1, 'failed': 0}), (True, {'requested': 1, 'delivered': 0, 'failed': 0}) ], ids=['seven_days', 'today']) -def test_get_service_notification_statistics(admin_request, sample_template, today_only, stats): - with freeze_time('2000-01-01T12:00:00'): - create_notification(sample_template, status='delivered') +def test_get_service_notification_statistics(admin_request, sample_service, sample_template, today_only, stats): + create_ft_notification_status(date(2000, 1, 1), 'sms', sample_service, count=1) with freeze_time('2000-01-02T12:00:00'): create_notification(sample_template, status='created') resp = admin_request.get(