From 6699442f6b3a6412426afb35d277d82169fce51d Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 11 May 2016 09:43:55 +0100 Subject: [PATCH] Added provider management pages in. - see priority - change priority --- app/__init__.py | 3 + app/main/__init__.py | 3 +- app/main/forms.py | 22 +- app/main/views/providers.py | 40 ++++ app/notify_client/provider_client.py | 33 +++ app/templates/main_nav.html | 3 + app/templates/views/provider.html | 32 +++ app/templates/views/providers.html | 65 ++++++ tests/app/main/views/test_dashboard.py | 2 + tests/app/main/views/test_providers.py | 277 +++++++++++++++++++++++++ 10 files changed, 465 insertions(+), 15 deletions(-) create mode 100644 app/main/views/providers.py create mode 100644 app/notify_client/provider_client.py create mode 100644 app/templates/views/provider.html create mode 100644 app/templates/views/providers.html create mode 100644 tests/app/main/views/test_providers.py diff --git a/app/__init__.py b/app/__init__.py index f8d86f8c9..b5337f95f 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -39,6 +39,7 @@ from app.notify_client.status_api_client import StatusApiClient from app.notify_client.template_statistics_api_client import TemplateStatisticsApiClient from app.notify_client.user_api_client import UserApiClient from app.notify_client.events_api_client import EventsApiClient +from app.notify_client.provider_client import ProviderClient login_manager = LoginManager() csrf = CsrfProtect() @@ -53,6 +54,7 @@ invite_api_client = InviteApiClient() statistics_api_client = StatisticsApiClient() template_statistics_client = TemplateStatisticsApiClient() events_api_client = EventsApiClient() +provider_client = ProviderClient() asset_fingerprinter = AssetFingerprinter() # The current service attached to the request stack. @@ -78,6 +80,7 @@ def create_app(): statistics_api_client.init_app(application) template_statistics_client.init_app(application) events_api_client.init_app(application) + provider_client.init_app(application) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' diff --git a/app/main/__init__.py b/app/main/__init__.py index adda9f715..f4b532063 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -25,5 +25,6 @@ from app.main.views import ( invites, all_services, tour, - feedback + feedback, + providers ) diff --git a/app/main/forms.py b/app/main/forms.py index 81bcb869d..744724128 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -4,14 +4,15 @@ from notifications_utils.recipients import ( InvalidPhoneError ) from wtforms import ( + validators, StringField, PasswordField, ValidationError, TextAreaField, FileField, BooleanField, - HiddenField -) + HiddenField, + IntegerField) from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp) @@ -27,7 +28,6 @@ def email_address(label='Email address'): class UKMobileNumber(TelField): - def pre_validate(self, form): try: validate_phone_number(self.data) @@ -74,7 +74,6 @@ class LoginForm(Form): class RegisterUserForm(Form): - name = StringField('Full name', validators=[DataRequired(message='Can’t be empty')]) email_address = email_address() @@ -92,14 +91,12 @@ class RegisterUserFromInviteForm(Form): class PermissionsForm(Form): - send_messages = BooleanField("Send messages from existing templates") manage_service = BooleanField("Modify this service, its team, and its templates") manage_api_keys = BooleanField("Create and revoke API keys") class InviteUserForm(PermissionsForm): - email_address = email_address('Email address') def __init__(self, invalid_email_address, *args, **kwargs): @@ -184,7 +181,6 @@ class ServiceNameForm(Form): class ConfirmPasswordForm(Form): - def __init__(self, validate_password_func, *args, **kwargs): self.validate_password_func = validate_password_func super(ConfirmPasswordForm, self).__init__(*args, **kwargs) @@ -211,7 +207,6 @@ class SMSTemplateForm(Form): class EmailTemplateForm(SMSTemplateForm): - subject = TextAreaField( u'Subject', validators=[DataRequired(message="Can’t be empty")]) @@ -226,7 +221,6 @@ class NewPasswordForm(Form): class ChangePasswordForm(Form): - def __init__(self, validate_password_func, *args, **kwargs): self.validate_password_func = validate_password_func super(ChangePasswordForm, self).__init__(*args, **kwargs) @@ -241,7 +235,7 @@ class ChangePasswordForm(Form): class CsvUploadForm(Form): file = FileField('Add recipients', validators=[DataRequired( - message='Please pick a file'), CsvFileValidator()]) + message='Please pick a file'), CsvFileValidator()]) class ChangeNameForm(Form): @@ -249,7 +243,6 @@ class ChangeNameForm(Form): class ChangeEmailForm(Form): - def __init__(self, validate_email_func, *args, **kwargs): self.validate_email_func = validate_email_func super(ChangeEmailForm, self).__init__(*args, **kwargs) @@ -263,7 +256,6 @@ class ChangeEmailForm(Form): class ConfirmEmailForm(Form): - def __init__(self, validate_code_func, *args, **kwargs): self.validate_code_func = validate_code_func super(ConfirmEmailForm, self).__init__(*args, **kwargs) @@ -281,7 +273,6 @@ class ChangeMobileNumberForm(Form): class ConfirmMobileNumberForm(Form): - def __init__(self, validate_code_func, *args, **kwargs): self.validate_code_func = validate_code_func super(ConfirmMobileNumberForm, self).__init__(*args, **kwargs) @@ -309,7 +300,6 @@ class CreateKeyForm(Form): class Feedback(Form): - name = StringField('Name') email_address = StringField('Email address') feedback = TextAreaField(u'', validators=[DataRequired(message="Can’t be empty")]) @@ -320,3 +310,7 @@ class RequestToGoLiveForm(Form): '', validators=[DataRequired(message="Can’t be empty")] ) + + +class ProviderForm(Form): + priority = IntegerField('Priority', [validators.NumberRange(min=1, max=100, message="Must be between 1 and 100")]) diff --git a/app/main/views/providers.py b/app/main/views/providers.py new file mode 100644 index 000000000..9846a364d --- /dev/null +++ b/app/main/views/providers.py @@ -0,0 +1,40 @@ +from flask import ( + render_template, url_for) + +from flask_login import ( + login_required, +) +from werkzeug.utils import redirect +from app.main import main +from app.main.forms import ProviderForm +from app.utils import user_has_permissions +from app import provider_client + + +@main.route("/providers") +@login_required +@user_has_permissions(admin_override=True) +def view_providers(): + providers = provider_client.get_all_providers()['provider_details'] + email_providers = [email for email in providers if email['notification_type'] == 'email'] + sms_providers = [sms for sms in providers if sms['notification_type'] == 'sms'] + return render_template('views/providers.html', email_providers=email_providers, sms_providers=sms_providers) + + +@main.route("/provider/", methods=['GET', 'POST']) +@login_required +@user_has_permissions(admin_override=True) +def view_provider(provider_id): + + provider = provider_client.get_provider_by_id(provider_id)['provider_details'] + + form = ProviderForm(active=provider['active'], priority=provider['priority']) + + print(form) + if form.validate_on_submit(): + print("HERE") + provider_client.update_provider(provider_id, form.priority.data) + + return redirect(url_for('.view_providers')) + + return render_template('views/provider.html', form=form, provider=provider) diff --git a/app/notify_client/provider_client.py b/app/notify_client/provider_client.py new file mode 100644 index 000000000..47ed15aab --- /dev/null +++ b/app/notify_client/provider_client.py @@ -0,0 +1,33 @@ +from notifications_python_client.base import BaseAPIClient +from app.notify_client import _attach_current_user + + +class ProviderClient(BaseAPIClient): + def __init__(self, base_url=None, client_id=None, secret=None): + super(self.__class__, self).__init__( + base_url=base_url or 'base_url', + client_id=client_id or 'client_id', + secret=secret or 'secret' + ) + + def init_app(self, app): + self.base_url = app.config['API_HOST_NAME'] + self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] + self.secret = app.config['ADMIN_CLIENT_SECRET'] + + def get_all_providers(self): + return self.get( + url='/provider-details' + ) + + def get_provider_by_id(self, provider_id): + return self.get( + url='/provider-details/{}'.format(provider_id) + ) + + def update_provider(self, provider_id, priority): + data = { + "priority": priority + } + _attach_current_user(data) + return self.post(url='/provider-details/{}'.format(provider_id), data=data) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 2d69364bf..db5edce4c 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -19,5 +19,8 @@ {% if current_user.has_permissions(admin_override=True) %}
  • List all services
    • {% endif %} + {% if current_user.has_permissions(admin_override=True) %} +
  • View providers
    • + {% endif %} diff --git a/app/templates/views/provider.html b/app/templates/views/provider.html new file mode 100644 index 000000000..d2d197c39 --- /dev/null +++ b/app/templates/views/provider.html @@ -0,0 +1,32 @@ +{% extends "withoutnav_template.html" %} +{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +Provider - {{provider.display_name}} – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +
    +
    + +

    {{provider.display_name}}

    + +

    Update provider:

    + +
      +
    • We only send from the highest priority provider
      • +
    + +
    + {{ textbox(form.priority) }} + {{ page_footer('Save', back_link=url_for('.view_providers'), back_link_text="Back to providers") }} +
    + +
    + +
    + +{% endblock %} diff --git a/app/templates/views/providers.html b/app/templates/views/providers.html new file mode 100644 index 000000000..fd2f33c4e --- /dev/null +++ b/app/templates/views/providers.html @@ -0,0 +1,65 @@ +{% extends "withoutnav_template.html" %} +{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading %} + +{% block page_title %} +Providers – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +
    +
    +

    Providers

    + +

    + Providers on Notify +

    + +

    SMS

    + + {% call(item, row_number) list_table( + sms_providers, + caption="SMS providers", + caption_visible=False, + empty_message='No email providers', + field_headings=['Provider', 'Priority', 'Active', ''], + field_headings_visible=True + ) %} + + {{ text_field(item.display_name) }} + + {{ text_field(item.priority) }} + + {{ text_field(item.active) }} + + {{ link_field('change', url_for('main.view_provider', provider_id=item.id)) }} + + {% endcall %} + +

    Email

    + + + {% call(item, row_number) list_table( + email_providers, + caption="Email providers", + caption_visible=False, + empty_message='No email providers', + field_headings=['Provider', 'Priority', 'Active', ''], + field_headings_visible=True + ) %} + + {{ text_field(item.display_name) }} + + {{ text_field(item.priority) }} + + {{ text_field(item.active) }} + + {{ link_field('change', url_for('main.view_provider', provider_id=item.id)) }} + + {% endcall %} + + +
    +
    + +{% endblock %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index a58a9bb0e..dd32cabab 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -192,6 +192,7 @@ def test_menu_send_messages(mocker, assert url_for('main.service_settings', service_id=service_one['id']) not in page assert url_for('main.api_keys', service_id=service_one['id']) not in page assert url_for('main.show_all_services') not in page + assert url_for('main.view_providers') not in page def test_menu_manage_service(mocker, @@ -276,6 +277,7 @@ def test_menu_all_services_for_platform_admin_user(mocker, []) page = resp.get_data(as_text=True) assert url_for('main.show_all_services') in page + assert url_for('main.view_providers') in page assert url_for('main.choose_template', service_id=service_one['id'], template_type='sms') in page assert url_for('main.choose_template', service_id=service_one['id'], template_type='email') in page assert url_for('main.manage_users', service_id=service_one['id']) in page diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py new file mode 100644 index 000000000..f6b35ce1e --- /dev/null +++ b/tests/app/main/views/test_providers.py @@ -0,0 +1,277 @@ +from bs4 import BeautifulSoup +from flask import url_for +import copy +import re +import app + +stub_providers = { + 'provider_details': [ + { + 'id': '6005e192-4738-4962-beec-ebd982d0b03f', + 'active': True, + 'priority': 1, + 'display_name': 'first_sms_provider', + 'identifier': 'first_sms', + 'notification_type': 'sms' + }, + { + 'active': True, + 'priority': 2, + 'display_name': 'second_sms_provider', + 'identifier': 'second_sms', + 'id': '0bd529cd-a0fd-43e5-80ee-b95ef6b0d51f', + 'notification_type': 'sms' + }, + { + 'id': '6005e192-4738-4962-beec-ebd982d0b03a', + 'active': True, + 'priority': 1, + 'display_name': 'first_email_provider', + 'identifier': 'first_email', + 'notification_type': 'email' + }, + { + 'active': True, + 'priority': 2, + 'display_name': 'second_email_provider', + 'identifier': 'second_email', + 'id': '0bd529cd-a0fd-43e5-80ee-b95ef6b0d51b', + 'notification_type': 'email' + } + ] +} + +stub_provider = { + 'provider_details': + { + 'id': '6005e192-4738-4962-beec-ebd982d0b03f', + 'active': True, + 'priority': 1, + 'display_name': 'first_sms_provider', + 'identifier': 'first_sms', + 'notification_type': 'sms' + } +} + + +def test_should_show_all_providers( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + mock_providers = mocker.patch( + 'app.provider_client.get_all_providers', + return_value=copy.deepcopy(stub_providers) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.get(url_for('main.view_providers')) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + h1 = [header.text.strip() for header in page.find_all('h1')] + + assert 'Providers' in h1 + + h2 = [header.text.strip() for header in page.find_all('h2')] + + assert 'Email' in h2 + assert 'SMS' in h2 + + tables = page.find_all('table') + assert len(tables) == 2 + + sms_table = tables[0] + email_table = tables[1] + + sms_first_row = sms_table.tbody.find_all('tr')[0] + table_data = sms_first_row.find_all('td') + + assert table_data[0].text.strip() == "first_sms_provider" + assert table_data[1].text.strip() == "1" + assert table_data[2].text.strip() == "True" + assert table_data[3].find_all("a")[0]['href'] == '/provider/6005e192-4738-4962-beec-ebd982d0b03f' + + sms_second_row = sms_table.tbody.find_all('tr')[1] + table_data = sms_second_row.find_all('td') + + assert table_data[0].text.strip() == "second_sms_provider" + assert table_data[1].text.strip() == "2" + assert table_data[2].text.strip() == "True" + assert table_data[3].find_all("a")[0]['href'] == '/provider/0bd529cd-a0fd-43e5-80ee-b95ef6b0d51f' + + email_first_row = email_table.tbody.find_all('tr')[0] + email_table_data = email_first_row.find_all('td') + + assert email_table_data[0].text.strip() == "first_email_provider" + assert email_table_data[1].text.strip() == "1" + assert email_table_data[2].text.strip() == "True" + assert email_table_data[3].find_all("a")[0]['href'] == '/provider/6005e192-4738-4962-beec-ebd982d0b03a' + + email_second_row = email_table.tbody.find_all('tr')[1] + email_table_data = email_second_row.find_all('td') + + assert email_table_data[0].text.strip() == "second_email_provider" + assert email_table_data[1].text.strip() == "2" + assert email_table_data[2].text.strip() == "True" + assert email_table_data[3].find_all("a")[0]['href'] == '/provider/0bd529cd-a0fd-43e5-80ee-b95ef6b0d51b' + + +def test_should_show_provider_detail( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + mock_providers = mocker.patch( + 'app.provider_client.get_provider_by_id', + return_value=copy.deepcopy(stub_provider) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.get(url_for('main.view_provider', provider_id='12345')) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + h1 = [header.text.strip() for header in page.find_all('h1')] + + assert 'first_sms_provider' in h1 + + form = [form for form in page.find_all('form')] + + form_elements = [element for element in form[0].find_all('input')] + assert form_elements[0]['value'] == '1' + assert form_elements[0]['name'] == 'priority' + + +def test_should_show_error_on_bad_provider_priority( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + mock_providers = mocker.patch( + 'app.provider_client.get_provider_by_id', + return_value=copy.deepcopy(stub_provider) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.post( + url_for('main.view_provider', provider_id=stub_provider['provider_details']['id']), + data={'priority': "not valid"}) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert response.status_code == 200 + assert "Not a valid integer value" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + + +def test_should_show_error_on_negative_provider_priority( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + mock_providers = mocker.patch( + 'app.provider_client.get_provider_by_id', + return_value=copy.deepcopy(stub_provider) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.post( + url_for('main.view_provider', provider_id=stub_provider['provider_details']['id']), + data={'priority': -1}) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert response.status_code == 200 + assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + + +def test_should_show_error_on_too_big_provider_priority( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + mock_providers = mocker.patch( + 'app.provider_client.get_provider_by_id', + return_value=copy.deepcopy(stub_provider) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.post( + url_for('main.view_provider', provider_id=stub_provider['provider_details']['id']), + data={'priority': 101}) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert response.status_code == 200 + assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + + +def test_should_show_error_on_too_little_provider_priority( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + mock_providers = mocker.patch( + 'app.provider_client.get_provider_by_id', + return_value=copy.deepcopy(stub_provider) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.post( + url_for('main.view_provider', provider_id=stub_provider['provider_details']['id']), + data={'priority': 0}) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert response.status_code == 200 + assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + + +def test_should_update_provider_priority( + app_, + platform_admin_user, + mock_login, + mock_has_permissions, + mocker +): + + mock_providers = mocker.patch( + 'app.provider_client.get_provider_by_id', + return_value=copy.deepcopy(stub_provider) + ) + + mock_updated_providers = mocker.patch( + 'app.provider_client.update_provider', + return_value=copy.deepcopy(stub_provider) + ) + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user) + response = client.post( + url_for('main.view_provider', provider_id=stub_provider['provider_details']['id']), + data={'priority': 2}) + + app.provider_client.update_provider.assert_called_with(stub_provider['provider_details']['id'], 2) + assert response.status_code == 302 + assert response.location == 'http://localhost/providers'