From 7a40ad6ac0487edbf2b0663aca9cac3cb88830f5 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 21 Jun 2018 15:04:03 +0100 Subject: [PATCH 1/4] Add new methods to Service and Complaint api clients * Added a new method to the ComplaintApiClient to get the total complaints by date range from the API. * Added a new method to the ServiceAPIClient to get the new platform admin stats data from the API. --- app/notify_client/complaint_api_client.py | 3 +++ app/notify_client/service_api_client.py | 3 +++ tests/app/notify_client/test_compliant_client.py | 9 +++++++++ tests/app/notify_client/test_service_api_client.py | 9 +++++++++ 4 files changed, 24 insertions(+) diff --git a/app/notify_client/complaint_api_client.py b/app/notify_client/complaint_api_client.py index 6e657396c..72760d0b7 100644 --- a/app/notify_client/complaint_api_client.py +++ b/app/notify_client/complaint_api_client.py @@ -9,3 +9,6 @@ class ComplaintApiClient(NotifyAdminAPIClient): def get_all_complaints(self): return self.get('/complaint') + + def get_complaint_count(self, params_dict=None): + return self.get('/complaint/count-by-date-range', params=params_dict) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index af8a2481a..9d5b59ccb 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -409,6 +409,9 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_aggregate_platform_stats(self, params_dict=None): return self.get("/service/platform-stats", params=params_dict) + def get_new_aggregate_platform_stats(self, params_dict=None): + return self.get("/service/platform-stats-new", params=params_dict) + def get_sms_senders(self, service_id): return self.get( "/service/{}/sms-sender".format(service_id) diff --git a/tests/app/notify_client/test_compliant_client.py b/tests/app/notify_client/test_compliant_client.py index 9d05d1490..2665a14a7 100644 --- a/tests/app/notify_client/test_compliant_client.py +++ b/tests/app/notify_client/test_compliant_client.py @@ -8,3 +8,12 @@ def test_get_all_complaints(mocker): client.get_all_complaints() mock.assert_called_once_with('/complaint') + + +def test_get_complaint_count(mocker): + client = ComplaintApiClient() + mock = mocker.patch.object(client, 'get') + params_dict = {'start_date': '2018-06-01', 'end_date': '2018-06-15'} + + client.get_complaint_count(params_dict=params_dict) + mock.assert_called_once_with('/complaint/count-by-date-range', params=params_dict) diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 7bc4afd4f..61d78e766 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -53,6 +53,15 @@ def test_client_only_updates_allowed_attributes(mocker): assert str(error.value) == 'Not allowed to update service attributes: foo' +def test_get_new_aggregate_platform_stats(mocker): + client = ServiceAPIClient() + mock = mocker.patch.object(client, 'get') + params_dict = {'start_date': '2018-06-01', 'end_date': '2018-06-15'} + + client.get_new_aggregate_platform_stats(params_dict=params_dict) + mock.assert_called_once_with('/service/platform-stats-new', params=params_dict) + + def test_client_creates_service_with_correct_data( mocker, active_user_with_permissions, From d9aeac4dca5820b678b0720e3c97963baa863389 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 25 Jun 2018 13:34:54 +0100 Subject: [PATCH 2/4] Add new platform admin page Added a new platform admin page, at '/plaform-admin-new' which shows different data. This no longer offers the option to filter by test-key, only by date, and also gives a more detailed break-down of the notifications and failures sent with a normal / research key. The existing platform admin stats page ('/platform-admin') has not been deleted yet so that both pages can be compared. --- app/assets/stylesheets/app.scss | 6 + .../stylesheets/components/big-number.scss | 12 ++ app/main/views/platform_admin.py | 112 ++++++++++++++++- app/navigation.py | 3 + app/templates/components/big-number.html | 16 +++ app/templates/components/status-box.html | 15 +++ .../views/platform-admin/index_new.html | 59 +++++++++ tests/app/main/views/test_platform_admin.py | 115 ++++++++++++++++++ 8 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 app/templates/components/status-box.html create mode 100644 app/templates/views/platform-admin/index_new.html diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 7bc54d38f..0e64b1e23 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -251,3 +251,9 @@ details .arrow { cursor: pointer; } } + +.bordered-text-box { + padding: 5px; + outline: 2px solid $black; + max-width: 100%; +} diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index 57590d461..7e4fa81a8 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -16,6 +16,18 @@ } +.big-number-dark { + @extend %big-number; + padding: $gutter-half; + position: relative; + background: $black; + color: $white; + + .big-number-number { + @include bold-36($tabular-numbers: true); + } +} + .big-number-smaller { @extend %big-number; diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 6321d862d..a8d80f037 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -1,7 +1,7 @@ import itertools from datetime import datetime -from flask import render_template, request +from flask import render_template, request, url_for from flask_login import login_required from app import complaint_api_client, service_api_client @@ -10,6 +10,10 @@ from app.main.forms import DateFilterForm from app.statistics_utils import get_formatted_percentage from app.utils import user_is_platform_admin +COMPLAINT_THRESHOLD = 0.02 +FAILURE_THRESHOLD = 3 +ZERO_FAILURE_THRESHOLD = 0 + @main.route("/platform-admin") @login_required @@ -38,6 +42,112 @@ def platform_admin(): ) +@main.route("/platform-admin-new") +@login_required +@user_is_platform_admin +def platform_admin_new(): + form = DateFilterForm(request.args) + api_args = {} + + if form.start_date.data: + api_args['start_date'] = form.start_date.data + api_args['end_date'] = form.end_date.data or datetime.utcnow().date() + + platform_stats = service_api_client.get_new_aggregate_platform_stats(api_args) + number_of_complaints = complaint_api_client.get_complaint_count(api_args) + + return render_template( + 'views/platform-admin/index_new.html', + form=form, + global_stats=make_columns(platform_stats, number_of_complaints) + ) + + +def is_over_threshold(number, total, threshold): + percentage = number / total * 100 if total else 0 + return percentage > threshold + + +def get_status_box_data(stats, key, label, threshold=FAILURE_THRESHOLD): + return { + 'number': stats['failures'][key], + 'label': label, + 'failing': is_over_threshold( + stats['failures'][key], + stats['total'], + threshold + ), + 'percentage': get_formatted_percentage(stats['failures'][key], stats['total']) + } + + +def get_tech_failure_status_box_data(stats): + stats = get_status_box_data(stats, 'technical-failure', 'technical failures', ZERO_FAILURE_THRESHOLD) + stats.pop('percentage') + return stats + + +def make_columns(global_stats, complaints_number): + return [ + # email + { + 'black_box': { + 'number': global_stats['email']['total'], + 'notification_type': 'email' + }, + 'other_data': [ + get_tech_failure_status_box_data(global_stats['email']), + get_status_box_data(global_stats['email'], 'permanent-failure', 'permanent failures'), + get_status_box_data(global_stats['email'], 'temporary-failure', 'temporary failures'), + { + 'number': complaints_number, + 'label': 'complaints', + 'failing': is_over_threshold(complaints_number, + global_stats['email']['total'], COMPLAINT_THRESHOLD), + 'percentage': get_formatted_percentage(complaints_number, global_stats['email']['total']), + 'url': url_for('main.platform_admin_list_complaints') + } + ], + 'test_data': { + 'number': global_stats['email']['test-key'], + 'label': 'test emails' + } + }, + # sms + { + 'black_box': { + 'number': global_stats['sms']['total'], + 'notification_type': 'sms' + }, + 'other_data': [ + get_tech_failure_status_box_data(global_stats['sms']), + get_status_box_data(global_stats['sms'], 'permanent-failure', 'permanent failures'), + get_status_box_data(global_stats['sms'], 'temporary-failure', 'temporary failures') + ], + 'test_data': { + 'number': global_stats['sms']['test-key'], + 'label': 'test text messages' + } + }, + # letter + { + 'black_box': { + 'number': global_stats['letter']['total'], + 'notification_type': 'letter' + }, + 'other_data': [ + get_tech_failure_status_box_data(global_stats['letter']), + get_status_box_data(global_stats['letter'], + 'virus-scan-failed', 'virus scan failures', ZERO_FAILURE_THRESHOLD) + ], + 'test_data': { + 'number': global_stats['letter']['test-key'], + 'label': 'test letters' + } + }, + ] + + @main.route("/platform-admin/live-services", endpoint='live_services') @main.route("/platform-admin/trial-services", endpoint='trial_services') @login_required diff --git a/app/navigation.py b/app/navigation.py index 2725f50ca..0b5f8bfc3 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -79,6 +79,7 @@ class HeaderNavigation(Navigation): 'live_services', 'organisations', 'platform_admin', + 'platform_admin_new', 'suspend_service', 'trial_services', 'update_email_branding', @@ -421,6 +422,7 @@ class MainNavigation(Navigation): 'organisation_settings', 'organisations', 'platform_admin', + 'platform_admin_new', 'platform_admin_list_complaints', 'pricing', 'privacy', @@ -590,6 +592,7 @@ class OrgNavigation(Navigation): 'old_using_notify', 'organisations', 'platform_admin', + 'platform_admin_new', 'platform_admin_list_complaints', 'pricing', 'privacy', diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index 454a0cc05..b5db30d5b 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -55,3 +55,19 @@ {% endif %} {% endmacro %} + + +{% macro big_number_simple(number, label) %} +
+
+ {% if number is number %} + {{ "{:,}".format(number) }} + {% else %} + {{ number }} + {% endif %} +
+ {% if label %} + {{ label }} + {% endif %} +
+{% endmacro %} diff --git a/app/templates/components/status-box.html b/app/templates/components/status-box.html new file mode 100644 index 000000000..257430e2b --- /dev/null +++ b/app/templates/components/status-box.html @@ -0,0 +1,15 @@ +{% macro status_box(number, label, failing=false, percentage=None, url=None) %} +
+
+ {% if url %} + {{ number }} {{ label }} + {% else %} + {{ number }} {{ label }} + {% endif %} + + {% if percentage %} + - {{ percentage }}% + {% endif %} +
+
+{% endmacro %} diff --git a/app/templates/views/platform-admin/index_new.html b/app/templates/views/platform-admin/index_new.html new file mode 100644 index 000000000..fd82bb17c --- /dev/null +++ b/app/templates/views/platform-admin/index_new.html @@ -0,0 +1,59 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/big-number.html" import big_number_simple %} +{% from "components/message-count-label.html" import message_count_label %} +{% from "components/status-box.html" import status_box %} + +{% block per_page_title %} + Platform admin +{% endblock %} + +{% block platform_admin_content %} + +

+ Summary +

+ +
+ Apply filters +
+ {{ textbox(form.start_date, hint="Enter start date in format YYYY-MM-DD") }} + {{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }} +
+ +
+
+ +
+ {% for noti_type in global_stats %} +
+ {{ big_number_simple( + noti_type.black_box.number, + message_count_label(noti_type.black_box.number, noti_type.black_box.notification_type) + ) }} + + {% for item in noti_type.other_data %} + {{ status_box( + number=item.number, + label=item.label, + failing=item.failing, + percentage=item.percentage, + url=item.url) + }} + {% endfor %} +
+ {% endfor %} +
+ +
+ {% for noti_type in global_stats %} +
+
+ {{ "{:,}".format(noti_type.test_data.number) }} + {{ noti_type.test_data.label }} +
+
+ {% endfor %} +
+ +{% endblock %} diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index dd0d47f13..0e0878c08 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -1,14 +1,18 @@ import datetime +import re import uuid from unittest.mock import ANY import pytest from bs4 import BeautifulSoup from flask import url_for +from freezegun import freeze_time from app.main.views.platform_admin import ( create_global_stats, format_stats_by_service, + get_tech_failure_status_box_data, + is_over_threshold, sum_service_usage, ) from tests import service_json @@ -667,3 +671,114 @@ def test_platform_admin_list_complaints( resp_data = response.get_data(as_text=True) assert 'Email complaints' in resp_data assert mock.called + + +@pytest.mark.parametrize('number, total, threshold, result', [ + (0, 0, 0, False), + (1, 1, 0, True), + (2, 3, 66, True), + (2, 3, 67, False), +]) +def test_is_over_threshold(number, total, threshold, result): + assert is_over_threshold(number, total, threshold) is result + + +def test_get_tech_failure_status_box_data_removes_percentage_data(): + stats = { + 'failures': + {'permanent-failure': 0, 'technical-failure': 0, 'temporary-failure': 1, 'virus-scan-failed': 0}, + 'test-key': 0, + 'total': 5589 + } + tech_failure_data = get_tech_failure_status_box_data(stats) + + assert 'percentage' not in tech_failure_data + + +def test_platform_admin_new_with_start_and_end_dates_provided(mocker, logged_in_platform_admin_client): + start_date = '2018-01-01' + end_date = '2018-06-01' + api_args = {'start_date': datetime.date(2018, 1, 1), 'end_date': datetime.date(2018, 6, 1)} + + mocker.patch('app.main.views.platform_admin.make_columns') + aggregate_stats_mock = mocker.patch( + 'app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats') + complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') + + logged_in_platform_admin_client.get( + url_for('main.platform_admin_new', start_date=start_date, end_date=end_date) + ) + + aggregate_stats_mock.assert_called_with(api_args) + complaint_count_mock.assert_called_with(api_args) + + +@freeze_time('2018-6-11') +def test_platform_admin_new_with_only_a_start_date_provided(mocker, logged_in_platform_admin_client): + start_date = '2018-01-01' + api_args = {'start_date': datetime.date(2018, 1, 1), 'end_date': datetime.datetime.utcnow().date()} + + mocker.patch('app.main.views.platform_admin.make_columns') + aggregate_stats_mock = mocker.patch( + 'app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats') + complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') + + logged_in_platform_admin_client.get(url_for('main.platform_admin_new', start_date=start_date)) + + aggregate_stats_mock.assert_called_with(api_args) + complaint_count_mock.assert_called_with(api_args) + + +def test_platform_admin_new_without_dates_provided(mocker, logged_in_platform_admin_client): + api_args = {} + + mocker.patch('app.main.views.platform_admin.make_columns') + aggregate_stats_mock = mocker.patch( + 'app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats') + complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') + + logged_in_platform_admin_client.get(url_for('main.platform_admin_new')) + + aggregate_stats_mock.assert_called_with(api_args) + complaint_count_mock.assert_called_with(api_args) + + +def test_platform_admin_new_displays_stats_in_right_boxes_and_with_correct_styling( + mocker, + logged_in_platform_admin_client, +): + platform_stats = { + 'email': {'failures': + {'permanent-failure': 3, 'technical-failure': 0, 'temporary-failure': 0, 'virus-scan-failed': 0}, + 'test-key': 0, + 'total': 145}, + 'sms': {'failures': + {'permanent-failure': 0, 'technical-failure': 1, 'temporary-failure': 0, 'virus-scan-failed': 0}, + 'test-key': 5, + 'total': 168}, + 'letter': {'failures': + {'permanent-failure': 0, 'technical-failure': 0, 'temporary-failure': 1, 'virus-scan-failed': 1}, + 'test-key': 0, + 'total': 500} + } + mocker.patch('app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats', + return_value=platform_stats) + mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count', return_value=15) + + response = logged_in_platform_admin_client.get(url_for('main.platform_admin_new')) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + # Email permanent failure status box - number is correct + assert '3 permanent failures' in page.find_all('div', class_='column-third')[0].find(string=re.compile('permanent')) + # Email complaints status box - link exists and number is correct + assert page.find('a', string='15 complaints') + # SMS total box - number is correct + assert page.find_all('div', class_='big-number-number')[1].text.strip() == '168' + # Test SMS box - number is correct + assert '5' in page.find_all('div', class_='column-third')[4].text + # SMS technical failure status box - number is correct and failure class is used + assert '1 technical failures' in page.find_all('div', class_='column-third')[1].find( + 'div', class_='big-number-status-failing').text + # Letter virus scan failure status box - number is correct and failure class is used + assert '1 virus scan failures' in page.find_all('div', class_='column-third')[2].find( + 'div', class_='big-number-status-failing').text From ca16bef7f779243d52900c89b67544246bd34397 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 28 Jun 2018 10:28:57 +0100 Subject: [PATCH 3/4] Add PlatformStatsAPIClient In API, the endpoint for the new platform admin stats page has been moved to a platform stats blueprint. This means we now need a platform stats client. --- app/__init__.py | 3 +++ app/main/views/platform_admin.py | 8 ++++++-- app/notify_client/platform_stats_api_client.py | 11 +++++++++++ app/notify_client/service_api_client.py | 3 --- tests/app/main/views/test_platform_admin.py | 8 ++++---- .../notify_client/test_platform_stats_api_client.py | 10 ++++++++++ tests/app/notify_client/test_service_api_client.py | 9 --------- 7 files changed, 34 insertions(+), 18 deletions(-) create mode 100644 app/notify_client/platform_stats_api_client.py create mode 100644 tests/app/notify_client/test_platform_stats_api_client.py diff --git a/app/__init__.py b/app/__init__.py index ee6a4a899..d3f74b9b5 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -59,6 +59,7 @@ from app.notify_client.letter_jobs_client import LetterJobsClient from app.notify_client.inbound_number_client import InboundNumberClient from app.notify_client.billing_api_client import BillingAPIClient from app.notify_client.complaint_api_client import ComplaintApiClient +from app.notify_client.platform_stats_api_client import PlatformStatsAPIClient from app.commands import setup_commands from app.utils import get_cdn_domain from app.utils import gmt_timezones @@ -86,6 +87,7 @@ letter_jobs_client = LetterJobsClient() inbound_number_client = InboundNumberClient() billing_api_client = BillingAPIClient() complaint_api_client = ComplaintApiClient() +platform_stats_api_client = PlatformStatsAPIClient() # The current service attached to the request stack. current_service = LocalProxy(partial(_lookup_req_object, 'service')) @@ -131,6 +133,7 @@ def create_app(application): inbound_number_client.init_app(application) billing_api_client.init_app(application) complaint_api_client.init_app(application) + platform_stats_api_client.init_app(application) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index a8d80f037..f69c90236 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -4,7 +4,11 @@ from datetime import datetime from flask import render_template, request, url_for from flask_login import login_required -from app import complaint_api_client, service_api_client +from app import ( + complaint_api_client, + platform_stats_api_client, + service_api_client, +) from app.main import main from app.main.forms import DateFilterForm from app.statistics_utils import get_formatted_percentage @@ -53,7 +57,7 @@ def platform_admin_new(): api_args['start_date'] = form.start_date.data api_args['end_date'] = form.end_date.data or datetime.utcnow().date() - platform_stats = service_api_client.get_new_aggregate_platform_stats(api_args) + platform_stats = platform_stats_api_client.get_aggregate_platform_stats(api_args) number_of_complaints = complaint_api_client.get_complaint_count(api_args) return render_template( diff --git a/app/notify_client/platform_stats_api_client.py b/app/notify_client/platform_stats_api_client.py new file mode 100644 index 000000000..9ca9390de --- /dev/null +++ b/app/notify_client/platform_stats_api_client.py @@ -0,0 +1,11 @@ +from app.notify_client import NotifyAdminAPIClient + + +class PlatformStatsAPIClient(NotifyAdminAPIClient): + # Fudge assert in the super __init__ so + # we can set those variables later. + def __init__(self): + super().__init__("a" * 73, "b") + + def get_aggregate_platform_stats(self, params_dict=None): + return self.get("/platform-stats", params=params_dict) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 9d5b59ccb..af8a2481a 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -409,9 +409,6 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_aggregate_platform_stats(self, params_dict=None): return self.get("/service/platform-stats", params=params_dict) - def get_new_aggregate_platform_stats(self, params_dict=None): - return self.get("/service/platform-stats-new", params=params_dict) - def get_sms_senders(self, service_id): return self.get( "/service/{}/sms-sender".format(service_id) diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 0e0878c08..8c587a23e 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -702,7 +702,7 @@ def test_platform_admin_new_with_start_and_end_dates_provided(mocker, logged_in_ mocker.patch('app.main.views.platform_admin.make_columns') aggregate_stats_mock = mocker.patch( - 'app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats') + 'app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') logged_in_platform_admin_client.get( @@ -720,7 +720,7 @@ def test_platform_admin_new_with_only_a_start_date_provided(mocker, logged_in_pl mocker.patch('app.main.views.platform_admin.make_columns') aggregate_stats_mock = mocker.patch( - 'app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats') + 'app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') logged_in_platform_admin_client.get(url_for('main.platform_admin_new', start_date=start_date)) @@ -734,7 +734,7 @@ def test_platform_admin_new_without_dates_provided(mocker, logged_in_platform_ad mocker.patch('app.main.views.platform_admin.make_columns') aggregate_stats_mock = mocker.patch( - 'app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats') + 'app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') logged_in_platform_admin_client.get(url_for('main.platform_admin_new')) @@ -761,7 +761,7 @@ def test_platform_admin_new_displays_stats_in_right_boxes_and_with_correct_styli 'test-key': 0, 'total': 500} } - mocker.patch('app.main.views.platform_admin.service_api_client.get_new_aggregate_platform_stats', + mocker.patch('app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats', return_value=platform_stats) mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count', return_value=15) diff --git a/tests/app/notify_client/test_platform_stats_api_client.py b/tests/app/notify_client/test_platform_stats_api_client.py new file mode 100644 index 000000000..e0f510c86 --- /dev/null +++ b/tests/app/notify_client/test_platform_stats_api_client.py @@ -0,0 +1,10 @@ +from app.notify_client.platform_stats_api_client import PlatformStatsAPIClient + + +def test_get_aggregate_platform_stats(mocker): + client = PlatformStatsAPIClient() + mock = mocker.patch.object(client, 'get') + params_dict = {'start_date': '2018-06-01', 'end_date': '2018-06-15'} + + client.get_aggregate_platform_stats(params_dict=params_dict) + mock.assert_called_once_with('/platform-stats', params=params_dict) diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 61d78e766..7bc4afd4f 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -53,15 +53,6 @@ def test_client_only_updates_allowed_attributes(mocker): assert str(error.value) == 'Not allowed to update service attributes: foo' -def test_get_new_aggregate_platform_stats(mocker): - client = ServiceAPIClient() - mock = mocker.patch.object(client, 'get') - params_dict = {'start_date': '2018-06-01', 'end_date': '2018-06-15'} - - client.get_new_aggregate_platform_stats(params_dict=params_dict) - mock.assert_called_once_with('/service/platform-stats-new', params=params_dict) - - def test_client_creates_service_with_correct_data( mocker, active_user_with_permissions, From 9c5fd5f23c63137db5646a45f9df36b830cd201b Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 29 Jun 2018 15:28:25 +0100 Subject: [PATCH 4/4] Validate date format on new platform admin page Check that the format of the dates entered is in the `YYYY-MM-DD` as specified and show an error message if it is not. If the date was not in the specified format, there would be no error message but the content of the page would change which was misleading. --- app/main/views/platform_admin.py | 4 +++- app/templates/views/platform-admin/index_new.html | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index f69c90236..c4164ce10 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -50,9 +50,11 @@ def platform_admin(): @login_required @user_is_platform_admin def platform_admin_new(): - form = DateFilterForm(request.args) + form = DateFilterForm(request.args, meta={'csrf': False}) api_args = {} + form.validate() + if form.start_date.data: api_args['start_date'] = form.start_date.data api_args['end_date'] = form.end_date.data or datetime.utcnow().date() diff --git a/app/templates/views/platform-admin/index_new.html b/app/templates/views/platform-admin/index_new.html index fd82bb17c..4754af1b0 100644 --- a/app/templates/views/platform-admin/index_new.html +++ b/app/templates/views/platform-admin/index_new.html @@ -13,8 +13,7 @@

Summary

- -
+
Apply filters
{{ textbox(form.start_date, hint="Enter start date in format YYYY-MM-DD") }}