From 56ba653f488aad84ec54e8272e55497a2aed9711 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 30 Jan 2017 15:17:26 +0000 Subject: [PATCH] Add endpoint for breakdown of activity by month MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This endpoint will eventualy replace the weekly breakdown one. By month for a given financial year is better, because it gives us consistency with the breakdown of financial usage (and eventually consistency with the template usage). The code to do this is a bit convoluted, in order to fill out the counts for months and statuses where we don’t have notifications. This will make the admin side of this easier, because we can rely on there always being numbers available. The admin side will deal with summing the statuses (eg `temporary-failure` > `failed`) because this is presentational. This commit also modifies the usage count to use `.between()` for consistency. --- app/dao/notifications_dao.py | 3 +- app/dao/services_dao.py | 56 +++++++++++++++++++++++++++++- app/service/rest.py | 16 ++++++++- tests/app/dao/test_services_dao.py | 45 ++++++++++++++++++++++++ tests/app/service/test_rest.py | 32 +++++++++++++++++ 5 files changed, 148 insertions(+), 4 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 3dff3f390..518811a28 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -241,8 +241,7 @@ def get_notification_billable_unit_count_per_month(service_id, year): ).filter( NotificationHistory.billable_units != 0, NotificationHistory.service_id == service_id, - NotificationHistory.created_at >= start, - NotificationHistory.created_at < end + NotificationHistory.created_at.between(*get_financial_year(year)), ).group_by( month ).order_by( diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index eb53c769c..50f582ad2 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -9,6 +9,7 @@ from app.dao.dao_utils import ( transactional, version_class ) +from app.dao.notifications_dao import get_financial_year from app.models import ( NotificationStatistics, TemplateStatistics, @@ -24,7 +25,10 @@ from app.models import ( User, InvitedUser, Service, - KEY_TYPE_TEST) + KEY_TYPE_TEST, + NOTIFICATION_STATUS_TYPES, + TEMPLATE_TYPES, +) from app.statsd_decorators import statsd @@ -238,6 +242,56 @@ def dao_fetch_weekly_historical_stats_for_service(service_id): ).all() +@statsd(namespace="dao") +def dao_fetch_monthly_historical_stats_for_service(service_id, year): + monday_of_notification_week = func.date_trunc('week', NotificationHistory.created_at).label('week_start') + start, end = get_financial_year(year) + + month = func.date_trunc( + "month", + func.timezone( + "Europe/London", + func.timezone("UTC", NotificationHistory.created_at) + ) + ) + + rows = db.session.query( + NotificationHistory.notification_type, + NotificationHistory.status, + month, + func.count(NotificationHistory.id).label('count') + ).filter( + NotificationHistory.service_id == service_id, + NotificationHistory.created_at.between(*get_financial_year(year)), + ).group_by( + NotificationHistory.notification_type, + NotificationHistory.status, + month + ).order_by( + month + ) + + months = { + datetime.strftime(date, '%Y-%m'): dict.fromkeys( + TEMPLATE_TYPES, + dict.fromkeys( + NOTIFICATION_STATUS_TYPES, + 0 + ) + ) + for date in [ + datetime(year, month, 1) for month in range(4, 13) + ] + [ + datetime(year + 1, month, 1) for month in range(1, 4) + ] + } + + for notification_type, status, date, count in rows: + months[datetime.strftime(date, "%Y-%m")][notification_type][status] = count + + return months + + @statsd(namespace='dao') def dao_fetch_todays_stats_for_all_services(include_from_test_key=True): query = db.session.query( diff --git a/app/service/rest.py b/app/service/rest.py index 416bace74..8ba9230e0 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -30,7 +30,9 @@ from app.dao.services_dao import ( dao_archive_service, fetch_stats_by_date_range_for_all_services, dao_suspend_service, - dao_resume_service) + dao_resume_service, + dao_fetch_monthly_historical_stats_for_service, +) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, dao_add_and_commit_whitelisted_contacts, @@ -273,6 +275,18 @@ def get_weekly_notification_stats(service_id): return jsonify(data={week.date().isoformat(): statistics for week, statistics in stats.items()}) +@service_blueprint.route('//notifications/monthly', methods=['GET']) +def get_monthly_notification_stats(service_id): + service = dao_fetch_service_by_id(service_id) + try: + return jsonify(data=dao_fetch_monthly_historical_stats_for_service( + service.id, + int(request.args.get('year', 'NaN')) + )) + except ValueError: + raise InvalidRequest('Year must be a number', status_code=400) + + 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 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 52624e0d1..943831ae9 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -20,6 +20,7 @@ from app.dao.services_dao import ( dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, + dao_fetch_monthly_historical_stats_for_service, fetch_todays_total_message_count, dao_fetch_todays_stats_for_all_services, fetch_stats_by_date_range_for_all_services, @@ -507,6 +508,50 @@ def test_fetch_weekly_historical_stats_separates_weeks(notify_db, notify_db_sess assert ret[-1].count == 1 +def test_fetch_monthly_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 + ) + _before_start_of_financial_year = notification_history(created_at=datetime(2016, 3, 31)) + start_of_financial_year = notification_history(created_at=datetime(2016, 4, 1)) + start_of_summer = notification_history(created_at=datetime(2016, 6, 20)) + start_of_autumn = notification_history(created_at=datetime(2016, 9, 22)) + start_of_winter = notification_history(created_at=datetime(2016, 12, 1), status='delivered') + start_of_spring = notification_history(created_at=datetime(2017, 3, 11)) + end_of_financial_year = notification_history(created_at=datetime(2017, 3, 31)) + _after_end_of_financial_year = notification_history(created_at=datetime(2017, 4, 1)) + + result = dao_fetch_monthly_historical_stats_for_service(sample_template.service_id, 2016) + + assert result['2016-04']['sms']['created'] == 1 + assert result['2016-04']['sms']['sending'] == 0 + assert result['2016-04']['sms']['delivered'] == 0 + assert result['2016-04']['sms']['pending'] == 0 + assert result['2016-04']['sms']['failed'] == 0 + assert result['2016-04']['sms']['technical-failure'] == 0 + assert result['2016-04']['sms']['temporary-failure'] == 0 + assert result['2016-04']['sms']['permanent-failure'] == 0 + + assert result['2016-06']['sms']['created'] == 1 + + assert result['2016-09']['sms']['created'] == 1 + + assert result['2016-12']['sms']['created'] == 0 + assert result['2016-12']['sms']['delivered'] == 1 + + assert result['2017-03']['sms']['created'] == 2 + + assert result.keys() == { + '2016-04', '2016-05', '2016-06', + '2016-07', '2016-08', '2016-09', + '2016-10', '2016-11', '2016-12', + '2017-01', '2017-02', '2017-03', + } + + 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] diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 5835d5159..936789e33 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1221,6 +1221,38 @@ def test_get_weekly_notification_stats(notify_api, notify_db, notify_db_session) } +@pytest.mark.parametrize( + 'url, expected_status, expected_json', [ + ( + '/service/{}/notifications/monthly?year=2001', + 200, + {'data': {'foo': 'bar'}}, + ), + ( + '/service/{}/notifications/monthly?year=baz', + 400, + {'message': 'Year must be a number', 'result': 'error'}, + ), + ( + '/service/{}/notifications/monthly', + 400, + {'message': 'Year must be a number', 'result': 'error'}, + ), + ] +) +def test_get_monthly_notification_stats(mocker, client, sample_service, url, expected_status, expected_json): + mock_dao = mocker.patch( + 'app.service.rest.dao_fetch_monthly_historical_stats_for_service', + return_value={'foo': 'bar'}, + ) + response = client.get( + url.format(sample_service.id), + headers=[create_authorization_header()] + ) + assert response.status_code == expected_status + assert json.loads(response.get_data(as_text=True)) == expected_json + + def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_session): notifications = [ create_sample_notification(notify_db, notify_db_session),