From 444132ad66db83e2a660bcb1863b338c8671b5a0 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 26 Jul 2016 11:00:03 +0100 Subject: [PATCH] rewrite weekly aggregate function to honor week boundaries (and not use the stats table, and also be easier to read) --- app/dao/services_dao.py | 18 ++++++ tests/app/conftest.py | 28 ++++++++++ tests/app/dao/test_services_dao.py | 88 ++++++++++++++++++++++++++++-- 3 files changed, 129 insertions(+), 5 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index f37e2f94d..a16aff007 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -157,3 +157,21 @@ def _stats_for_service_query(service_id): Notification.notification_type, Notification.status, ) + + +def dao_fetch_weekly_historical_stats_for_service(service_id): + monday_of_notification_week = func.date_trunc('week', NotificationHistory.created_at).label('week_start') + return db.session.query( + NotificationHistory.notification_type, + NotificationHistory.status, + monday_of_notification_week, + func.count(NotificationHistory.id).label('count') + ).filter( + NotificationHistory.service_id == service_id + ).group_by( + NotificationHistory.notification_type, + NotificationHistory.status, + monday_of_notification_week + ).order_by( + asc(monday_of_notification_week), NotificationHistory.status + ).all() diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e6295860d..4a95f6e67 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -13,6 +13,7 @@ from app.models import ( ApiKey, Job, Notification, + NotificationHistory, InvitedUser, Permission, ProviderStatistics, @@ -42,6 +43,8 @@ def service_factory(notify_db, notify_db_session): def get(self, service_name, user=None, template_type=None, email_from=None): if not user: user = sample_user(notify_db, notify_db_session) + if not email_from: + email_from = service_name service = sample_service(notify_db, notify_db_session, service_name, user, email_from=email_from) if template_type == 'email': sample_template( @@ -367,6 +370,31 @@ def sample_notification(notify_db, return notification +@pytest.fixture(scope='function') +def sample_notification_history(notify_db, + notify_db_session, + sample_template, + status='created', + created_at=None): + if created_at is None: + created_at = datetime.utcnow() + + notification_history = NotificationHistory( + id=uuid.uuid4(), + service=sample_template.service, + template=sample_template, + template_version=sample_template.version, + status=status, + created_at=created_at, + notification_type=sample_template.template_type, + key_type=KEY_TYPE_NORMAL + ) + notify_db.session.add(notification_history) + notify_db.session.commit() + + return notification_history + + @pytest.fixture(scope='function') def mock_celery_send_sms_code(mocker): return mocker.patch('app.celery.tasks.send_sms_code.apply_async') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index dcf538a43..7aeaeb6d2 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,6 +1,8 @@ +from datetime import datetime import uuid -import pytest +import functools +import pytest from sqlalchemy.orm.exc import FlushError, NoResultFound from sqlalchemy.exc import IntegrityError from freezegun import freeze_time @@ -16,7 +18,8 @@ from app.dao.services_dao import ( dao_update_service, delete_service_and_all_associated_db_objects, dao_fetch_stats_for_service, - dao_fetch_todays_stats_for_service + dao_fetch_todays_stats_for_service, + dao_fetch_weekly_historical_stats_for_service ) from app.dao.users_dao import save_model_user from app.models import ( @@ -36,7 +39,8 @@ from app.models import ( ) from tests.app.conftest import ( - sample_notification as create_notification + sample_notification as create_notification, + sample_notification_history as create_notification_history ) @@ -407,7 +411,7 @@ def test_fetch_stats_counts_correctly(notify_db, notify_db_session, sample_templ create_notification(notify_db, notify_db_session, template=sample_email_template, status='technical-failure') create_notification(notify_db, notify_db_session, template=sample_template, status='created') - stats = dao_fetch_stats_for_service(sample_template.service.id) + stats = dao_fetch_stats_for_service(sample_template.service_id) stats = sorted(stats, key=lambda x: (x.notification_type, x.status)) assert len(stats) == 3 @@ -435,9 +439,83 @@ def test_fetch_stats_for_today_only_includes_today(notify_db, notify_db_session, with freeze_time('2001-01-02T12:00:00'): right_now = create_notification(notify_db, None, to_field='3', status='created') - stats = dao_fetch_todays_stats_for_service(sample_template.service.id) + stats = dao_fetch_todays_stats_for_service(sample_template.service_id) stats = {row.status: row.count for row in stats} assert 'delivered' not in stats assert stats['failed'] == 1 assert stats['created'] == 1 + + +def test_fetch_weekly_historical_stats_separates_weeks(notify_db, notify_db_session, sample_template): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + sample_template + ) + week_53_last_yr = notification_history(created_at=datetime(2016, 1, 1)) + week_1_last_yr = notification_history(created_at=datetime(2016, 1, 5)) + last_sunday = notification_history(created_at=datetime(2016, 7, 24, 23, 59)) + last_monday_morning = notification_history(created_at=datetime(2016, 7, 25, 0, 0)) + last_monday_evening = notification_history(created_at=datetime(2016, 7, 25, 23, 59)) + today = notification_history(created_at=datetime.now(), status='delivered') + + with freeze_time('Wed 27th July 2016'): + ret = dao_fetch_weekly_historical_stats_for_service(sample_template.service_id) + + assert [(row.week_start, row.status) for row in ret] == [ + (datetime(2015, 12, 28), 'created'), + (datetime(2016, 1, 4), 'created'), + (datetime(2016, 7, 18), 'created'), + (datetime(2016, 7, 25), 'created'), + (datetime(2016, 7, 25), 'delivered') + ] + assert ret[-2].count == 2 + assert ret[-1].count == 1 + + +def test_fetch_weekly_historical_stats_ignores_second_service(notify_db, notify_db_session, service_factory): + template_1 = service_factory.get('1').templates[0] + template_2 = service_factory.get('2').templates[0] + + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session + ) + last_sunday = notification_history(template_1, created_at=datetime(2016, 7, 24, 23, 59)) + last_monday_morning = notification_history(template_2, created_at=datetime(2016, 7, 25, 0, 0)) + + with freeze_time('Wed 27th July 2016'): + ret = dao_fetch_weekly_historical_stats_for_service(template_1.service_id) + + assert len(ret) == 1 + assert ret[0].week_start == datetime(2016, 7, 18) + assert ret[0].count == 1 + + +def test_fetch_weekly_historical_stats_separates_types(notify_db, + notify_db_session, + sample_template, + sample_email_template): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + created_at=datetime(2016, 7, 25) + ) + + notification_history(sample_template) + notification_history(sample_email_template) + + with freeze_time('Wed 27th July 2016'): + ret = dao_fetch_weekly_historical_stats_for_service(sample_template.service_id) + + assert len(ret) == 2 + assert ret[0].week_start == datetime(2016, 7, 25) + assert ret[0].count == 1 + assert ret[0].notification_type == 'email' + assert ret[1].week_start == datetime(2016, 7, 25) + assert ret[1].count == 1 + assert ret[1].notification_type == 'sms'