From df41da4860ab93cbdfe3b8382dc89627139c485f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 28 Dec 2016 14:44:53 +0000 Subject: [PATCH 1/5] [WIP] Attempt at adding a form to the platform admin page is not working well for me here. But I want to commit it so I can look at it again. --- app/main/forms.py | 17 ++++++- app/main/views/platform_admin.py | 52 ++++++++++++++++----- app/notify_client/service_api_client.py | 1 + app/templates/views/platform-admin.html | 7 +++ tests/app/main/views/test_platform_admin.py | 5 +- 5 files changed, 64 insertions(+), 18 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 57599ee78..1a4c577f7 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -16,8 +16,8 @@ from wtforms import ( HiddenField, IntegerField, RadioField, - FieldList -) + FieldList, + DateField) from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) @@ -516,3 +516,16 @@ class Whitelist(Form): max_entries=5, label="Mobile numbers" ) + + +class DateFilterForm(Form): + start_date = DateField("Start Date", [validators.optional()]) + end_date = StringField("End Date", [validators.optional()]) + + def validate(self): + print("****In validate") + if self.start_date.data and not self.end_date.data: + print("***** false {}".format(type(self.end_date.errors))) + raise ValidationError('Both required') + else: + return True diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index d6494620f..6c5fb8f06 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -5,33 +5,62 @@ from flask import ( request ) from flask_login import login_required +from wtforms import ValidationError from app import service_api_client from app.main import main +from app.main.forms import DateFilterForm from app.utils import user_has_permissions from app.statistics_utils import get_formatted_percentage -@main.route("/platform-admin") +@main.route("/platform-admin", methods=['GET', 'POST']) @login_required @user_has_permissions(admin_override=True) def platform_admin(): + form = DateFilterForm() include_from_test_key = request.args.get('include_from_test_key') != 'False' # specifically DO get inactive services api_args = {'detailed': True} if not include_from_test_key: api_args['include_from_test_key'] = False - services = service_api_client.get_services(api_args)['data'] - return render_template( - 'views/platform-admin.html', - include_from_test_key=include_from_test_key, - **get_statistics(sorted( - services, - key=lambda service: (service['active'], service['created_at']), - reverse=True - )) - ) + if form.validate_on_submit(): + start_date = form.start_date.data + end_date = form.end_date.data + if start_date: + print(start_date) + print(end_date) + api_args['start_date'] = start_date + if not end_date: + raise ValidationError(message='requires end date', field =form.end_date) + api_args['end_date'] = end_date + + services = service_api_client.get_services(api_args)['data'] + + return render_template( + 'views/platform-admin.html', + include_from_test_key=include_from_test_key, + form=form, + **get_statistics(sorted( + services, + key=lambda service: (service['active'], service['created_at']), + reverse=True + )) + ) + else: + services = service_api_client.get_services(api_args)['data'] + + return render_template( + 'views/platform-admin.html', + include_from_test_key=include_from_test_key, + form=form, + **get_statistics(sorted( + services, + key=lambda service: (service['active'], service['created_at']), + reverse=True + )) + ) def get_statistics(services): return { @@ -64,7 +93,6 @@ def create_global_stats(services): for stat in stats.values(): stat['failure_rate'] = get_formatted_percentage(stat['failed'], stat['requested']) - return stats diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index e974890db..a4f0ef9d2 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -60,6 +60,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): """ Retrieve a list of services. """ + print(params_dict) return self.get('/service', params=params_dict) def get_active_services(self, params_dict=None): diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index 52eb48d4b..b5a94cd10 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -1,4 +1,6 @@ {% extends "withoutnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} {% from "components/big-number.html" import big_number, big_number_with_status %} {% from "components/message-count-label.html" import message_count_label %} {% from "components/table.html" import mapping_table, field, stats_fields, row_group, row, right_aligned_field_heading, hidden_field_heading, text_field %} @@ -94,6 +96,11 @@ {% else %} Excluding test keys (change) {% endif %} +
+ {{ 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") }} + {{ page_footer('Apply filter') }} +

diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 6f6fd5411..8956115d3 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -1,14 +1,11 @@ -from datetime import date - from flask import url_for -from freezegun import freeze_time import pytest from bs4 import BeautifulSoup from tests.conftest import mock_get_user from tests import service_json -from app.main.views.platform_admin import get_statistics, format_stats_by_service, create_global_stats +from app.main.views.platform_admin import format_stats_by_service, create_global_stats def test_should_redirect_if_not_logged_in(app_): From 5572d0f25ff0e75a29484f0b9aa4cccf668d2fbd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 28 Dec 2016 15:23:19 +0000 Subject: [PATCH 2/5] This commit renders the platform admin page with the given start and end date query params. Need to add a form to the page to take these params. --- app/main/forms.py | 13 ----- app/main/views/platform_admin.py | 56 +++++++-------------- app/templates/views/platform-admin.html | 5 -- tests/app/main/views/test_platform_admin.py | 22 ++++++++ 4 files changed, 41 insertions(+), 55 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 1a4c577f7..9e96a8ccc 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -516,16 +516,3 @@ class Whitelist(Form): max_entries=5, label="Mobile numbers" ) - - -class DateFilterForm(Form): - start_date = DateField("Start Date", [validators.optional()]) - end_date = StringField("End Date", [validators.optional()]) - - def validate(self): - print("****In validate") - if self.start_date.data and not self.end_date.data: - print("***** false {}".format(type(self.end_date.errors))) - raise ValidationError('Both required') - else: - return True diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 6c5fb8f06..70f598efb 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -5,62 +5,44 @@ from flask import ( request ) from flask_login import login_required -from wtforms import ValidationError from app import service_api_client from app.main import main -from app.main.forms import DateFilterForm from app.utils import user_has_permissions from app.statistics_utils import get_formatted_percentage -@main.route("/platform-admin", methods=['GET', 'POST']) +@main.route("/platform-admin") @login_required @user_has_permissions(admin_override=True) def platform_admin(): - form = DateFilterForm() include_from_test_key = request.args.get('include_from_test_key') != 'False' + start_date = request.args.get('start_date', None) + end_date = request.args.get('end_date', None) # specifically DO get inactive services api_args = {'detailed': True} if not include_from_test_key: api_args['include_from_test_key'] = False - if form.validate_on_submit(): - start_date = form.start_date.data - end_date = form.end_date.data - if start_date: - print(start_date) - print(end_date) - api_args['start_date'] = start_date - if not end_date: - raise ValidationError(message='requires end date', field =form.end_date) - api_args['end_date'] = end_date + if start_date: + # For now the start and end date are only set as query params. The previous commit added a form + # but I could get the validation right, not did the page look good. + # This is just an intermediate pass at returning the data. + api_args['start_date'] = start_date + api_args['end_date'] = end_date - services = service_api_client.get_services(api_args)['data'] + services = service_api_client.get_services(api_args)['data'] - return render_template( - 'views/platform-admin.html', - include_from_test_key=include_from_test_key, - form=form, - **get_statistics(sorted( - services, - key=lambda service: (service['active'], service['created_at']), - reverse=True - )) - ) - else: - services = service_api_client.get_services(api_args)['data'] + return render_template( + 'views/platform-admin.html', + include_from_test_key=include_from_test_key, + **get_statistics(sorted( + services, + key=lambda service: (service['active'], service['created_at']), + reverse=True + )) + ) - return render_template( - 'views/platform-admin.html', - include_from_test_key=include_from_test_key, - form=form, - **get_statistics(sorted( - services, - key=lambda service: (service['active'], service['created_at']), - reverse=True - )) - ) def get_statistics(services): return { diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index b5a94cd10..6147edaa3 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -96,11 +96,6 @@ {% else %} Excluding test keys (change) {% endif %} -
- {{ 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") }} - {{ page_footer('Apply filter') }} -

diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 8956115d3..efa700b30 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -121,6 +121,28 @@ def test_platform_admin_toggle_including_from_test_key( mock_get_detailed_services.assert_called_once_with(api_args) +def test_platform_admin_with_date_filter( + app_, + platform_admin_user, + mocker, + mock_get_detailed_services +): + with app_.test_request_context(): + with app_.test_client() as client: + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + response = client.get(url_for('main.platform_admin', start_date='2016-12-20', end_date='2012-12-28')) + + assert response.status_code == 200 + resp_data = response.get_data(as_text=True) + assert 'Platform admin' in resp_data + assert 'Showing stats for today' in resp_data + assert 'Live services' in resp_data + assert 'Trial mode services' in resp_data + mock_get_detailed_services.assert_called_once_with({'detailed': True, + 'start_date': '2016-12-20', 'end_date': '2012-12-28'}) + + def test_create_global_stats_sets_failure_rates(fake_uuid): services = [ service_json(fake_uuid, 'a', []), From 2e69d3c680d7f338d24a944e5d93bfeb26e0f842 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 3 Jan 2017 10:45:06 +0000 Subject: [PATCH 3/5] Add date filter for platform admin page. --- app/main/forms.py | 5 +++++ app/main/views/platform_admin.py | 12 ++++++++---- app/templates/views/platform-admin.html | 19 +++++++++++-------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 9e96a8ccc..fe83ae0b4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -516,3 +516,8 @@ class Whitelist(Form): max_entries=5, label="Mobile numbers" ) + +class DateFilterForm(Form): + start_date = DateField("Start Date", [validators.optional()]) + end_date = StringField("End Date", [validators.optional()]) + include_from_test_key = BooleanField("Include test keys", default='checked') diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 70f598efb..a91a111a5 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -1,5 +1,6 @@ import itertools +from datetime import datetime from flask import ( render_template, request @@ -8,6 +9,7 @@ from flask_login import login_required from app import service_api_client from app.main import main +from app.main.forms import DateFilterForm from app.utils import user_has_permissions from app.statistics_utils import get_formatted_percentage @@ -16,9 +18,10 @@ from app.statistics_utils import get_formatted_percentage @login_required @user_has_permissions(admin_override=True) def platform_admin(): - include_from_test_key = request.args.get('include_from_test_key') != 'False' - start_date = request.args.get('start_date', None) - end_date = request.args.get('end_date', None) + form = DateFilterForm(request.args) + include_from_test_key = form.include_from_test_key.data + start_date = form.start_date.data + end_date = form.end_date.data # specifically DO get inactive services api_args = {'detailed': True} if not include_from_test_key: @@ -29,13 +32,14 @@ def platform_admin(): # but I could get the validation right, not did the page look good. # This is just an intermediate pass at returning the data. api_args['start_date'] = start_date - api_args['end_date'] = end_date + api_args['end_date'] = end_date or datetime.utcnow().date() services = service_api_client.get_services(api_args)['data'] return render_template( 'views/platform-admin.html', include_from_test_key=include_from_test_key, + form=form, **get_statistics(sorted( services, key=lambda service: (service['active'], service['created_at']), diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index 6147edaa3..94b1ae7dc 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -1,5 +1,6 @@ {% extends "withoutnav_template.html" %} {% from "components/textbox.html" import textbox %} +{% from "components/checkbox.html" import checkbox %} {% from "components/page-footer.html" import page_footer %} {% from "components/big-number.html" import big_number, big_number_with_status %} {% from "components/message-count-label.html" import message_count_label %} @@ -89,14 +90,16 @@ Platform admin -

- Showing stats for today  - {% if include_from_test_key %} - Including test keys (change) - {% else %} - Excluding test keys (change) - {% endif %} -

+
+ 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") }} + {{ checkbox(form.include_from_test_key) }} +
+ +
+
From 21582cadb38a1b4d624db961a1a4fd24c6940df0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 3 Jan 2017 16:14:25 +0000 Subject: [PATCH 4/5] Get the BooleanField working in the form. --- app/main/forms.py | 5 ++- app/main/views/platform_admin.py | 21 ++++------ tests/app/main/views/test_platform_admin.py | 44 ++++++++------------- 3 files changed, 26 insertions(+), 44 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index fe83ae0b4..45b40abb5 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -517,7 +517,8 @@ class Whitelist(Form): label="Mobile numbers" ) + class DateFilterForm(Form): start_date = DateField("Start Date", [validators.optional()]) - end_date = StringField("End Date", [validators.optional()]) - include_from_test_key = BooleanField("Include test keys", default='checked') + end_date = DateField("End Date", [validators.optional()]) + include_from_test_key = BooleanField("Include test keys", default="checked", false_values={"N"}) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index a91a111a5..462320c26 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -19,26 +19,19 @@ from app.statistics_utils import get_formatted_percentage @user_has_permissions(admin_override=True) def platform_admin(): form = DateFilterForm(request.args) - include_from_test_key = form.include_from_test_key.data - start_date = form.start_date.data - end_date = form.end_date.data - # specifically DO get inactive services - api_args = {'detailed': True} - if not include_from_test_key: - api_args['include_from_test_key'] = False + api_args = {'detailed': True, # specifically DO get inactive services + 'include_from_test_key': form.include_from_test_key.data + } - if start_date: - # For now the start and end date are only set as query params. The previous commit added a form - # but I could get the validation right, not did the page look good. - # This is just an intermediate pass at returning the data. - api_args['start_date'] = start_date - api_args['end_date'] = end_date or datetime.utcnow().date() + 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() services = service_api_client.get_services(api_args)['data'] return render_template( 'views/platform-admin.html', - include_from_test_key=include_from_test_key, + include_from_test_key=form.include_from_test_key.data, form=form, **get_statistics(sorted( services, diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index efa700b30..e7654fcb3 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -1,3 +1,5 @@ +import datetime + from flask import url_for import pytest from bs4 import BeautifulSoup @@ -55,7 +57,7 @@ def test_should_show_research_and_restricted_mode( response = client.get(url_for('main.platform_admin')) assert response.status_code == 200 - mock_get_detailed_services.assert_called_once_with({'detailed': True}) + mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') # get first column in second row, which contains flags as text. table_body = page.find_all('table')[table_index].find_all('tbody')[0] @@ -78,20 +80,17 @@ def test_should_render_platform_admin_page( assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert 'Platform admin' in resp_data - assert 'Showing stats for today' in resp_data assert 'Live services' in resp_data assert 'Trial mode services' in resp_data - mock_get_detailed_services.assert_called_once_with({'detailed': True}) + mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) -@pytest.mark.parametrize('include_from_test_key, expected_text, unexpected_text, api_args', [ - (True, 'Including test keys', 'Excluding test keys', {'detailed': True}), - (False, 'Excluding test keys', 'Including test keys', {'detailed': True, 'include_from_test_key': False}) +@pytest.mark.parametrize('include_from_test_key, api_args', [ + ("Y", {'detailed': True, 'include_from_test_key': True}), + ("N", {'detailed': True, 'include_from_test_key': False}) ]) def test_platform_admin_toggle_including_from_test_key( include_from_test_key, - expected_text, - unexpected_text, api_args, app_, platform_admin_user, @@ -100,24 +99,12 @@ def test_platform_admin_toggle_including_from_test_key( ): with app_.test_request_context(): with app_.test_client() as client: + print(include_from_test_key) mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) - response = client.get(url_for('main.platform_admin', include_from_test_key=str(include_from_test_key))) + response = client.get(url_for('main.platform_admin', include_from_test_key=include_from_test_key)) assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert expected_text in resp_data - assert unexpected_text not in resp_data - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - change_link = page.find('a', text='change') - assert change_link['href'] - query_param = 'include_from_test_key=False' - if include_from_test_key: - assert query_param in change_link['href'] - else: - assert query_param not in change_link['href'] - mock_get_detailed_services.assert_called_once_with(api_args) @@ -131,16 +118,17 @@ def test_platform_admin_with_date_filter( with app_.test_client() as client: mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) - response = client.get(url_for('main.platform_admin', start_date='2016-12-20', end_date='2012-12-28')) + response = client.get(url_for('main.platform_admin', start_date='2016-12-20', end_date='2016-12-28')) assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert 'Platform admin' in resp_data - assert 'Showing stats for today' in resp_data assert 'Live services' in resp_data assert 'Trial mode services' in resp_data - mock_get_detailed_services.assert_called_once_with({'detailed': True, - 'start_date': '2016-12-20', 'end_date': '2012-12-28'}) + mock_get_detailed_services.assert_called_once_with({'include_from_test_key': False, + 'start_date': datetime.date(2016, 12, 20), + 'end_date': datetime.date(2016, 12, 28), + 'detailed': True}) def test_create_global_stats_sets_failure_rates(fake_uuid): @@ -254,7 +242,7 @@ def test_should_show_email_and_sms_stats_for_all_service_types( response = client.get(url_for('main.platform_admin')) assert response.status_code == 200 - mock_get_detailed_services.assert_called_once_with({'detailed': True}) + mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') table_body = page.find_all('table')[table_index].find_all('tbody')[0] @@ -300,7 +288,7 @@ def test_should_show_archived_services_last( response = client.get(url_for('main.platform_admin')) assert response.status_code == 200 - mock_get_detailed_services.assert_called_once_with({'detailed': True}) + mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') table_body = page.find_all('table')[table_index].find_all('tbody')[0] From 8b89b7f9112ad978a683107880866a5acfb987a1 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 4 Jan 2017 13:02:01 +0000 Subject: [PATCH 5/5] Remove print statements. --- app/notify_client/service_api_client.py | 1 - tests/app/main/views/test_platform_admin.py | 1 - 2 files changed, 2 deletions(-) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index a4f0ef9d2..e974890db 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -60,7 +60,6 @@ class ServiceAPIClient(NotifyAdminAPIClient): """ Retrieve a list of services. """ - print(params_dict) return self.get('/service', params=params_dict) def get_active_services(self, params_dict=None): diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index e7654fcb3..0ac1aafea 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -99,7 +99,6 @@ def test_platform_admin_toggle_including_from_test_key( ): with app_.test_request_context(): with app_.test_client() as client: - print(include_from_test_key) mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) response = client.get(url_for('main.platform_admin', include_from_test_key=include_from_test_key))