diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 74072aa2b..67588672a 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -1,7 +1,8 @@ -from datetime import datetime, date, timedelta +from datetime import datetime, timedelta from collections import namedtuple from itertools import groupby +import dateutil from flask import ( render_template, url_for, @@ -9,13 +10,11 @@ from flask import ( jsonify, current_app ) - from flask_login import login_required from app.main import main from app import ( job_api_client, - statistics_api_client, service_api_client, template_statistics_client ) @@ -90,20 +89,10 @@ def usage(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def weekly(service_id): - - earliest_date = date(2016, 4, 1) # start of tax year - while earliest_date.weekday() != 0: # 0 for monday - earliest_date -= timedelta(days=1) - + stats = service_api_client.get_weekly_notification_stats(service_id)['data'] return render_template( 'views/weekly.html', - days=( - add_rates_to(day) for day in - statistics_api_client.get_7_day_aggregate_for_service( - service_id, - date_from=earliest_date - )['data'] - ), + days=format_weekly_stats_to_list(stats), now=datetime.utcnow() ) @@ -199,3 +188,21 @@ def calculate_usage(usage): 'sms_chargeable': max(0, sms_sent - sms_free_allowance), 'sms_rate': sms_rate } + + +def format_weekly_stats_to_list(historical_stats): + out = [] + for week, weekly_stats in historical_stats.items(): + for stats in weekly_stats.values(): + stats['failure_rate'] = get_formatted_percentage(stats['failed'], stats['requested']) + + week_start = dateutil.parser.parse(week) + week_end = week_start + timedelta(days=6) + weekly_stats.update({ + 'week_start': week, + 'week_end': week_end.date().isoformat(), + 'week_end_datetime': week_end, + }) + out.append(weekly_stats) + + return sorted(out, key=lambda x: x['week_start'], reverse=True) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index f2896a49e..230c53b3e 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -193,6 +193,9 @@ class ServiceAPIClient(NotificationsAPIClient): def get_service_usage(self, service_id): return self.get('/service/{0}/fragment/aggregate_statistics'.format(service_id)) + def get_weekly_notification_stats(self, service_id): + return self.get(url='/service/{}/notifications/weekly'.format(service_id)) + class ServicesBrowsableItem(BrowsableItem): @property diff --git a/app/notify_client/statistics_api_client.py b/app/notify_client/statistics_api_client.py index cc3fdc480..ba1a6c496 100644 --- a/app/notify_client/statistics_api_client.py +++ b/app/notify_client/statistics_api_client.py @@ -23,17 +23,6 @@ class StatisticsApiClient(BaseAPIClient): else: raise e - def get_7_day_aggregate_for_service(self, service_id, date_from=None, week_count=None): - params = {} - if date_from is not None: - params['date_from'] = date_from - if week_count is not None: - params['week_count'] = week_count - return self.get( - url='/service/{}/notifications-statistics/seven_day_aggregate'.format(service_id), - params=params - ) - def get_statistics_for_all_services_for_day(self, day): params = { 'day': day diff --git a/app/templates/views/weekly.html b/app/templates/views/weekly.html index 10a516290..5a5a87493 100644 --- a/app/templates/views/weekly.html +++ b/app/templates/views/weekly.html @@ -3,7 +3,7 @@ {% extends "withnav_template.html" %} {% block page_title %} - Daily – GOV.UK Notify + Previous weeks – GOV.UK Notify {% endblock %} {% block maincolumn_content %} @@ -33,16 +33,16 @@ {% endif %} {% endcall %} {% call field(align='right') %} - {{ item.emails_requested }} + {{ item.email.requested }} {% endcall %} {% call field(align='right') %} - {{ item.emails_failure_rate }}% + {{ item.email.failure_rate }}% {% endcall %} {% call field(align='right') %} - {{ item.sms_requested }} + {{ item.sms.requested }} {% endcall %} {% call field(align='right') %} - {{ item.sms_failure_rate }}% + {{ item.sms.failure_rate }}% {% endcall %} {% endcall %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index dc5a35d80..b1cc51ccb 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1,3 +1,4 @@ +from datetime import datetime import copy from flask import url_for @@ -5,7 +6,7 @@ import pytest from bs4 import BeautifulSoup from freezegun import freeze_time -from app.main.views.dashboard import get_dashboard_totals +from app.main.views.dashboard import get_dashboard_totals, format_weekly_stats_to_list from tests import validate_route_permission from tests.conftest import SERVICE_ONE_ID @@ -65,7 +66,6 @@ def test_get_started( api_user_active, mock_get_service, mock_get_service_templates_when_no_templates_exist, - mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, mock_login, @@ -94,7 +94,6 @@ def test_get_started_is_hidden_once_templates_exist( api_user_active, mock_get_service, mock_get_service_templates, - mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, mock_login, @@ -119,7 +118,6 @@ def test_should_show_recent_templates_on_dashboard(app_, api_user_active, mock_get_service, mock_get_service_templates, - mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, mock_login, @@ -204,7 +202,6 @@ def test_should_show_recent_jobs_on_dashboard( api_user_active, mock_get_service, mock_get_service_templates, - mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, mock_login, @@ -482,3 +479,46 @@ def test_get_dashboard_totals_adds_warning(failures, expected): } } assert get_dashboard_totals(stats)['sms']['show_warning'] == expected + + +def test_format_weekly_stats_to_list_empty_case(): + assert format_weekly_stats_to_list({}) == [] + + +def test_format_weekly_stats_to_list_sorts_by_week(): + stats = { + '2016-07-04': {}, + '2016-07-11': {}, + '2016-07-18': {}, + '2016-07-25': {} + } + resp = format_weekly_stats_to_list(stats) + assert resp[0]['week_start'] == '2016-07-25' + assert resp[1]['week_start'] == '2016-07-18' + assert resp[2]['week_start'] == '2016-07-11' + assert resp[3]['week_start'] == '2016-07-04' + + +def test_format_weekly_stats_to_list_includes_datetime_for_comparison(): + stats = { + '2016-07-25': {} + } + resp = format_weekly_stats_to_list(stats) + assert resp == [{ + 'week_start': '2016-07-25', + 'week_end': '2016-07-31', + 'week_end_datetime': datetime(2016, 7, 31, 0, 0, 0) + }] + + +def test_format_weekly_stats_to_list_has_stats_with_failure_rate(): + stats = { + '2016-07-25': {'sms': _stats(3, 1, 2)} + } + resp = format_weekly_stats_to_list(stats) + assert resp[0]['sms']['failure_rate'] == '66.7' + assert resp[0]['sms']['requested'] == 3 + + +def _stats(requested, delivered, failed): + return {'requested': requested, 'delivered': delivered, 'failed': failed} diff --git a/tests/conftest.py b/tests/conftest.py index 1535d0080..195244990 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -220,15 +220,6 @@ def mock_get_service_statistics_for_day(mocker): 'app.statistics_api_client.get_statistics_for_service_for_day', side_effect=_stats) -@pytest.fixture(scope='function') -def mock_get_aggregate_service_statistics(mocker): - def _create(service_id, limit_days=None): - return {'data': [{}]} - - return mocker.patch( - 'app.statistics_api_client.get_7_day_aggregate_for_service', side_effect=_create) - - @pytest.fixture(scope='function') def mock_get_all_service_statistics(mocker): def _create(day):