Merge pull request #563 from alphagov/week-agg

New weekly aggregate function

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()

app/service/rest.py

@@ -1,4 +1,4 @@
-from datetime import date
+from datetime import date, timedelta
 
 from flask import (
     jsonify,
@@ -8,7 +8,6 @@ from flask import (
 )
 from sqlalchemy.orm.exc import NoResultFound
 
-from app.models import EMAIL_TYPE, SMS_TYPE
 from app.dao.api_key_dao import (
     save_model_api_key,
     get_model_api_keys,
@@ -23,7 +22,8 @@ from app.dao.services_dao import (
     dao_add_user_to_service,
     dao_remove_user_from_service,
     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 import notifications_dao
 from app.dao.provider_statistics_dao import get_fragment_count
@@ -43,6 +43,7 @@ from app.errors import (
     register_errors,
     InvalidRequest
 )
+from app.service import statistics
 
 service = Blueprint('service', __name__)
 register_errors(service)
@@ -236,29 +237,20 @@ def get_all_notifications_for_service(service_id):
     ), 200
 
 
+@service.route('/<uuid:service_id>/notifications/weekly', methods=['GET'])
+def get_weekly_notification_stats(service_id):
+    service = dao_fetch_service_by_id(service_id)
+    stats = dao_fetch_weekly_historical_stats_for_service(service_id)
+    stats = statistics.format_weekly_notification_stats(stats, service.created_at)
+    return jsonify(data={week.date().isoformat(): statistics for week, statistics in stats.items()})
+
+
 def get_detailed_service(service_id, today_only=False):
     service = dao_fetch_service_by_id(service_id)
     stats_fn = dao_fetch_todays_stats_for_service if today_only else dao_fetch_stats_for_service
-    statistics = stats_fn(service_id)
+    stats = stats_fn(service_id)
-    service.statistics = format_statistics(statistics)
+
+    service.statistics = statistics.format_statistics(stats)
+
     data = detailed_service_schema.dump(service).data
     return jsonify(data=data)
-
-
-def format_statistics(statistics):
-    # statistics come in a named tuple with uniqueness from 'notification_type', 'status' - however missing
-    # statuses/notification types won't be represented and the status types need to be simplified/summed up
-    # so we can return emails/sms * created, sent, and failed
-    counts = {
-        template_type: {
-            status: 0 for status in ('requested', 'delivered', 'failed')
-        } for template_type in (EMAIL_TYPE, SMS_TYPE)
-    }
-    for row in statistics:
-        counts[row.notification_type]['requested'] += row.count
-        if row.status == 'delivered':
-            counts[row.notification_type]['delivered'] += row.count
-        elif row.status in ('failed', 'technical-failure', 'temporary-failure', 'permanent-failure'):
-            counts[row.notification_type]['failed'] += row.count
-
-    return counts

app/service/statistics.py

@@ -0,0 +1,53 @@
+import itertools
+from datetime import datetime, timedelta
+
+from app.models import EMAIL_TYPE, SMS_TYPE
+
+
+def format_statistics(statistics):
+    # statistics come in a named tuple with uniqueness from 'notification_type', 'status' - however missing
+    # statuses/notification types won't be represented and the status types need to be simplified/summed up
+    # so we can return emails/sms * created, sent, and failed
+    counts = _create_zeroed_stats_dicts()
+    for row in statistics:
+        _update_statuses_from_row(counts[row.notification_type], row)
+
+    return counts
+
+
+def format_weekly_notification_stats(statistics, service_created_at):
+    preceeding_monday = (service_created_at - timedelta(days=service_created_at.weekday()))
+    # turn a datetime into midnight that day http://stackoverflow.com/a/1937636
+    preceeding_monday_midnight = datetime.combine(preceeding_monday.date(), datetime.min.time())
+    week_dict = {
+        week: _create_zeroed_stats_dicts()
+        for week in _weeks_for_range(preceeding_monday_midnight, datetime.utcnow())
+    }
+    for row in statistics:
+        _update_statuses_from_row(week_dict[row.week_start][row.notification_type], row)
+
+    return week_dict
+
+
+def _create_zeroed_stats_dicts():
+    return {
+        template_type: {
+            status: 0 for status in ('requested', 'delivered', 'failed')
+        } for template_type in (EMAIL_TYPE, SMS_TYPE)
+    }
+
+
+def _update_statuses_from_row(update_dict, row):
+    update_dict['requested'] += row.count
+    if row.status == 'delivered':
+        update_dict['delivered'] += row.count
+    elif row.status in ('failed', 'technical-failure', 'temporary-failure', 'permanent-failure'):
+        update_dict['failed'] += row.count
+
+
+def _weeks_for_range(start, end):
+    """
+    Generator that yields dates from `start` to `end`, in 7 day intervals. End is inclusive.
+    """
+    infinite_date_generator = (start + timedelta(days=i) for i in itertools.count(step=7))
+    return itertools.takewhile(lambda x: x <= end, infinite_date_generator)

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')

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'

tests/app/service/test_rest.py

@@ -1,5 +1,4 @@
 import json
-import collections
 import uuid
 
 import pytest
@@ -18,9 +17,6 @@ from tests.app.conftest import (
 )
 
 
-Row = collections.namedtuple('row', ('notification_type', 'status', 'count'))
-
-
 def test_get_service_list(notify_api, service_factory):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
@@ -1127,37 +1123,27 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s
     assert service['statistics']['sms'] == stats
 
 
-# email_counts and sms_counts are 3-tuple of requested, delivered, failed
+@freeze_time('2016-07-28')
-@pytest.mark.idparametrize('stats, email_counts, sms_counts', {
+def test_get_weekly_notification_stats(notify_api, sample_notification):
-    'empty': ([], [0, 0, 0], [0, 0, 0]),
+    with notify_api.test_request_context(), notify_api.test_client() as client:
-    'always_increment_requested': ([
+        resp = client.get(
-        Row('email', 'delivered', 1),
+            '/service/{}/notifications/weekly'.format(sample_notification.service_id),
-        Row('email', 'failed', 1)
+            headers=[create_authorization_header()]
-    ], [2, 1, 1], [0, 0, 0]),
+        )
-    'dont_mix_email_and_sms': ([
-        Row('email', 'delivered', 1),
-        Row('sms', 'delivered', 1)
-    ], [1, 1, 0], [1, 1, 0]),
-    'convert_fail_statuses_to_failed': ([
-        Row('email', 'failed', 1),
-        Row('email', 'technical-failure', 1),
-        Row('email', 'temporary-failure', 1),
-        Row('email', 'permanent-failure', 1),
-    ], [4, 0, 4], [0, 0, 0]),
-})
-def test_format_statistics(stats, email_counts, sms_counts):
-    from app.service.rest import format_statistics
 
-    ret = format_statistics(stats)
+    assert resp.status_code == 200
-
+    data = json.loads(resp.get_data(as_text=True))['data']
-    assert ret['email'] == {
+    assert data == {
-        status: count
+        '2016-07-25': {
-        for status, count
+            'sms': {
-        in zip(['requested', 'delivered', 'failed'], email_counts)
+                'requested': 1,
-    }
+                'delivered': 0,
-
+                'failed': 0
-    assert ret['sms'] == {
+            },
-        status: count
+            'email': {
-        for status, count
+                'requested': 0,
-        in zip(['requested', 'delivered', 'failed'], sms_counts)
+                'delivered': 0,
+                'failed': 0
+            }
+        }
     }

tests/app/service/test_statistics.py

@@ -0,0 +1,138 @@
+from datetime import datetime
+import collections
+
+import pytest
+from freezegun import freeze_time
+
+from app.service.statistics import (
+    format_statistics,
+    _weeks_for_range,
+    _create_zeroed_stats_dicts,
+    format_weekly_notification_stats
+)
+
+StatsRow = collections.namedtuple('row', ('notification_type', 'status', 'count'))
+WeeklyStatsRow = collections.namedtuple('row', ('notification_type', 'status', 'week_start', 'count'))
+
+
+# email_counts and sms_counts are 3-tuple of requested, delivered, failed
+@pytest.mark.idparametrize('stats, email_counts, sms_counts', {
+    'empty': ([], [0, 0, 0], [0, 0, 0]),
+    'always_increment_requested': ([
+        StatsRow('email', 'delivered', 1),
+        StatsRow('email', 'failed', 1)
+    ], [2, 1, 1], [0, 0, 0]),
+    'dont_mix_email_and_sms': ([
+        StatsRow('email', 'delivered', 1),
+        StatsRow('sms', 'delivered', 1)
+    ], [1, 1, 0], [1, 1, 0]),
+    'convert_fail_statuses_to_failed': ([
+        StatsRow('email', 'failed', 1),
+        StatsRow('email', 'technical-failure', 1),
+        StatsRow('email', 'temporary-failure', 1),
+        StatsRow('email', 'permanent-failure', 1),
+    ], [4, 0, 4], [0, 0, 0]),
+})
+def test_format_statistics(stats, email_counts, sms_counts):
+
+    ret = format_statistics(stats)
+
+    assert ret['email'] == {
+        status: count
+        for status, count
+        in zip(['requested', 'delivered', 'failed'], email_counts)
+    }
+
+    assert ret['sms'] == {
+        status: count
+        for status, count
+        in zip(['requested', 'delivered', 'failed'], sms_counts)
+    }
+
+
+@pytest.mark.parametrize('start,end,dates', [
+    (datetime(2016, 7, 25), datetime(2016, 7, 25), [datetime(2016, 7, 25)]),
+    (datetime(2016, 7, 25), datetime(2016, 7, 28), [datetime(2016, 7, 25)]),
+    (datetime(2016, 7, 25), datetime(2016, 8, 1), [datetime(2016, 7, 25), datetime(2016, 8, 1)]),
+    (datetime(2016, 7, 25), datetime(2016, 8, 10), [
+        datetime(2016, 7, 25), datetime(2016, 8, 1), datetime(2016, 8, 8)
+    ])
+])
+def test_weeks_for_range(start, end, dates):
+    assert list(_weeks_for_range(start, end)) == dates
+
+
+def test_create_zeroed_stats_dicts():
+    assert _create_zeroed_stats_dicts() == {
+        'sms': {'requested': 0, 'delivered': 0, 'failed': 0},
+        'email': {'requested': 0, 'delivered': 0, 'failed': 0},
+    }
+
+
+def _stats(requested, delivered, failed):
+    return {'requested': requested, 'delivered': delivered, 'failed': failed}
+
+
+@freeze_time('2016-07-28T12:00:00')
+@pytest.mark.parametrize('created_at, statistics, expected_results', [
+    # with no stats and just today, return this week's stats
+    (datetime(2016, 7, 28), [], {
+        datetime(2016, 7, 25): {
+            'sms': _stats(0, 0, 0),
+            'email': _stats(0, 0, 0)
+        }
+    }),
+    # with a random created time, still create the dict for midnight
+    (datetime(2016, 7, 28, 12, 13, 14), [], {
+        datetime(2016, 7, 25, 0, 0, 0): {
+            'sms': _stats(0, 0, 0),
+            'email': _stats(0, 0, 0)
+        }
+    }),
+    # with no stats but a service
+    (datetime(2016, 7, 14), [], {
+        datetime(2016, 7, 11): {
+            'sms': _stats(0, 0, 0),
+            'email': _stats(0, 0, 0)
+        },
+        datetime(2016, 7, 18): {
+            'sms': _stats(0, 0, 0),
+            'email': _stats(0, 0, 0)
+        },
+        datetime(2016, 7, 25): {
+            'sms': _stats(0, 0, 0),
+            'email': _stats(0, 0, 0)
+        }
+    }),
+    # two stats for same week dont re-zero each other
+    (datetime(2016, 7, 21), [
+        WeeklyStatsRow('email', 'created', datetime(2016, 7, 18), 1),
+        WeeklyStatsRow('sms', 'created', datetime(2016, 7, 18), 1),
+    ], {
+        datetime(2016, 7, 18): {
+            'sms': _stats(1, 0, 0),
+            'email': _stats(1, 0, 0)
+        },
+        datetime(2016, 7, 25): {
+            'sms': _stats(0, 0, 0),
+            'email': _stats(0, 0, 0)
+        }
+    }),
+    # two stats for same type are added together
+    (datetime(2016, 7, 21), [
+        WeeklyStatsRow('sms', 'created', datetime(2016, 7, 18), 1),
+        WeeklyStatsRow('sms', 'delivered', datetime(2016, 7, 18), 1),
+        WeeklyStatsRow('sms', 'created', datetime(2016, 7, 25), 1),
+    ], {
+        datetime(2016, 7, 18): {
+            'sms': _stats(2, 1, 0),
+            'email': _stats(0, 0, 0)
+        },
+        datetime(2016, 7, 25): {
+            'sms': _stats(1, 0, 0),
+            'email': _stats(0, 0, 0)
+        }
+    })
+])
+def test_format_weekly_notification_stats(statistics, created_at, expected_results):
+    assert format_weekly_notification_stats(statistics, created_at) == expected_results