From 312a903e652589744077be101b13c6d45eb09339 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Apr 2016 13:59:24 +0100 Subject: [PATCH 1/3] Limit templates usage on dashboard to last 7 days Previous the table of templates on the dashboard was for all time. This commit uses the `limit_days` parameter of the API endpoint to only show template usage from the last 7 days, aligning with the big numbers shown above. --- app/main/views/dashboard.py | 25 ++++++++++--------- .../template_statistics_api_client.py | 6 ++++- .../test_template_statistics_client.py | 2 +- tests/app/main/views/test_dashboard.py | 2 +- tests/conftest.py | 2 +- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 2fea2ddfd..edb6c521e 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -38,13 +38,8 @@ def service_dashboard(service_id): return render_template( 'views/dashboard/dashboard.html', - statistics=add_rates_to( - statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data'] - ), templates=service_api_client.get_service_templates(service_id)['data'], - template_statistics=aggregate_usage( - template_statistics_client.get_template_statistics_for_service(service_id) - ) + **get_dashboard_statistics_for_service(service_id) ) @@ -54,12 +49,7 @@ def service_dashboard_updates(service_id): return jsonify(**{ 'today': render_template( 'views/dashboard/today.html', - statistics=add_rates_to( - statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data'] - ), - template_statistics=aggregate_usage( - template_statistics_client.get_template_statistics_for_service(service_id) - ) + **get_dashboard_statistics_for_service(service_id) ) }) @@ -125,3 +115,14 @@ def aggregate_usage(template_statistics): key=lambda row: row['usage_count'], reverse=True ) + + +def get_dashboard_statistics_for_service(service_id): + return { + 'statistics': add_rates_to( + statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data'] + ), + 'template_statistics': aggregate_usage( + template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7) + ) + } \ No newline at end of file diff --git a/app/notify_client/template_statistics_api_client.py b/app/notify_client/template_statistics_api_client.py index a47e934fb..2ac088f76 100644 --- a/app/notify_client/template_statistics_api_client.py +++ b/app/notify_client/template_statistics_api_client.py @@ -12,7 +12,11 @@ class TemplateStatisticsApiClient(BaseAPIClient): self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] self.secret = app.config['ADMIN_CLIENT_SECRET'] - def get_template_statistics_for_service(self, service_id): + def get_template_statistics_for_service(self, service_id, limit_days=None): + params = {} + if limit_days is not None: + params['limit_days'] = limit_days return self.get( url='/service/{}/template-statistics'.format(service_id), + params=params )['data'] diff --git a/tests/app/main/notify_client/test_template_statistics_client.py b/tests/app/main/notify_client/test_template_statistics_client.py index 03fb33e4c..938eb4254 100644 --- a/tests/app/main/notify_client/test_template_statistics_client.py +++ b/tests/app/main/notify_client/test_template_statistics_client.py @@ -14,4 +14,4 @@ def test_template_statistics_client_calls_correct_api_endpoint(mocker, api_user_ client.get_template_statistics_for_service(some_service_id) - mock_get.assert_called_once_with(url=expected_url) + mock_get.assert_called_once_with(url=expected_url, params={}) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 9717e60c2..12082f1c7 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -78,7 +78,7 @@ def test_should_show_recent_templates_on_dashboard(app_, assert response.status_code == 200 response.get_data(as_text=True) mock_get_service_statistics.assert_called_once_with(SERVICE_ONE_ID, limit_days=7) - mock_template_stats.assert_called_once_with(SERVICE_ONE_ID) + mock_template_stats.assert_called_once_with(SERVICE_ONE_ID, limit_days=7) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') headers = [header.text.strip() for header in page.find_all('h2')] diff --git a/tests/conftest.py b/tests/conftest.py index d6dbdad17..9d68c6eb9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -826,7 +826,7 @@ def mock_get_template_statistics(mocker, service_one, fake_uuid): "day": "2016-04-04" } - def _get_stats(service_id): + def _get_stats(service_id, limit_days=None): return [data] return mocker.patch( From 09491e880e2fc724b699eed0dacebbc33afd3764 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Apr 2016 14:09:38 +0100 Subject: [PATCH 2/3] =?UTF-8?q?Show=20template=20usage=20for=20all=20time?= =?UTF-8?q?=20on=20it=E2=80=99s=20own=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > We show the last weeks template usage on the dashboard, which is > great, but if you're looking for longer term trends, you're out of > luck... > So, let's let you see more on a more detailed page (linked from the > dashboard). Initially this should just show you all templates that you > have used ever and the count for each. Order same as dashboard, most > popular first. https://www.pivotaltracker.com/story/show/117614585 --- app/main/views/dashboard.py | 14 +++++- .../dashboard/all-template-statistics.html | 17 ++++++++ .../views/dashboard/template-statistics.html | 2 +- app/templates/views/dashboard/today.html | 8 +++- tests/app/main/views/test_dashboard.py | 43 +++++++++++++++++++ 5 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 app/templates/views/dashboard/all-template-statistics.html diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index edb6c521e..d72158abf 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -54,6 +54,18 @@ def service_dashboard_updates(service_id): }) +@main.route("/services//template-activity") +@login_required +@user_has_permissions('view_activity', admin_override=True) +def template_history(service_id): + return render_template( + 'views/dashboard/all-template-statistics.html', + template_statistics=aggregate_usage( + template_statistics_client.get_template_statistics_for_service(service_id) + ) + ) + + def add_rates_to(delivery_statistics): if not delivery_statistics or not delivery_statistics[0]: @@ -125,4 +137,4 @@ def get_dashboard_statistics_for_service(service_id): 'template_statistics': aggregate_usage( template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7) ) - } \ No newline at end of file + } diff --git a/app/templates/views/dashboard/all-template-statistics.html b/app/templates/views/dashboard/all-template-statistics.html new file mode 100644 index 000000000..3404df5f5 --- /dev/null +++ b/app/templates/views/dashboard/all-template-statistics.html @@ -0,0 +1,17 @@ +{% extends "withnav_template.html" %} + +{% block page_title %} + {{ current_service.name }} – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} +

+ Templates used this year +

+

+ 1 April 2016 to date +

+ {% with period = "" %} + {% include 'views/dashboard/template-statistics.html' %} + {% endwith %} +{% endblock %} diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index d3a75867d..53070ef4a 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -3,7 +3,7 @@ template_statistics, caption="By template", caption_visible=False, - empty_message='You haven’t set up any templates yet', + empty_message='You haven’t used any templates {}'.format(period), field_headings=['Template', hidden_field_heading('Type'), right_aligned_field_heading('Messages processed')] ) %} {% call field() %} diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index 7912ffae7..3c2377642 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -25,5 +25,9 @@ ) }} - -{% include 'views/dashboard/template-statistics.html' %} +{% with period = "in the last 7 days" %} + {% include 'views/dashboard/template-statistics.html' %} +{% endwith %} +

+ See all templates used this year +

diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 12082f1c7..9fc7f57cc 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -101,6 +101,49 @@ def test_should_show_recent_templates_on_dashboard(app_, assert table_data[2].text.strip() == '13' +def test_should_show_all_templates_on_template_statistics_page( + app_, + mocker, + api_user_active, + mock_get_service, + mock_get_service_templates, + mock_get_service_statistics, + mock_get_user, + mock_get_user_by_email, + mock_login, + mock_get_jobs, + mock_has_permissions +): + + mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', + return_value=copy.deepcopy(stub_template_stats)) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.template_history', service_id=SERVICE_ONE_ID)) + + assert response.status_code == 200 + response.get_data(as_text=True) + mock_template_stats.assert_called_once_with(SERVICE_ONE_ID) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + headers = [header.text.strip() for header in page.find_all('h2')] + table_rows = page.tbody.find_all('tr') + + assert len(table_rows) == 2 + + first_row = page.tbody.find_all('tr')[0] + table_data = first_row.find_all('td') + assert len(table_data) == 3 + assert table_data[2].text.strip() == '206' + + second_row = page.tbody.find_all('tr')[1] + table_data = second_row.find_all('td') + assert len(table_data) == 3 + assert table_data[2].text.strip() == '13' + + def _test_dashboard_menu(mocker, app_, usr, service, permissions): with app_.test_request_context(): with app_.test_client() as client: From 0080bce56266a35b0ab807c505cf02dc40464690 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Apr 2016 14:25:05 +0100 Subject: [PATCH 3/3] =?UTF-8?q?Say=20=E2=80=98sent=E2=80=99=20rather=20tha?= =?UTF-8?q?n=20=E2=80=98processed=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hopefully users understand that sent != delivered --- app/templates/views/dashboard/template-statistics.html | 2 +- tests/app/main/views/test_dashboard.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index 53070ef4a..a29651ee2 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -4,7 +4,7 @@ caption="By template", caption_visible=False, empty_message='You haven’t used any templates {}'.format(period), - field_headings=['Template', hidden_field_heading('Type'), right_aligned_field_heading('Messages processed')] + field_headings=['Template', hidden_field_heading('Type'), right_aligned_field_heading('Messages sent')] ) %} {% call field() %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 9fc7f57cc..e5276b856 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -85,7 +85,7 @@ def test_should_show_recent_templates_on_dashboard(app_, assert 'Test Service' in headers assert 'In the last 7 days' in headers template_usage_headers = [th.text.strip() for th in page.thead.find_all('th')] - for th in ['Template', 'Type', 'Messages processed']: + for th in ['Template', 'Type', 'Messages sent']: assert th in template_usage_headers table_rows = page.tbody.find_all('tr')