diff --git a/app/main/forms.py b/app/main/forms.py index cf1d3d99d..4506f4028 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,4 +1,5 @@ import pytz +from flask_login import current_user from flask_wtf import Form from datetime import datetime, timedelta from notifications_utils.recipients import ( @@ -17,7 +18,8 @@ from wtforms import ( IntegerField, RadioField, FieldList, - DateField) + DateField, + SelectField) from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) @@ -268,6 +270,9 @@ class SMSTemplateForm(Form): NoCommasInPlaceHolders() ] ) + process_type = SelectField(u'Select priority or normal', choices=[('normal', 'normal'), + ('priority', 'priority')], + default='normal') class EmailTemplateForm(SMSTemplateForm): diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 13c5bf180..5fa6be365 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -2,7 +2,7 @@ from datetime import datetime, timedelta from string import ascii_uppercase from flask import request, render_template, redirect, url_for, flash, abort, send_file -from flask_login import login_required +from flask_login import login_required, current_user from flask_weasyprint import HTML, render_pdf from dateutil.parser import parse @@ -143,6 +143,7 @@ def view_template_version_as_png(service_id, template_id, version): @login_required @user_has_permissions('manage_templates', admin_override=True) def add_service_template(service_id, template_type): + if template_type not in ['sms', 'email', 'letter']: abort(404) if not current_service['can_send_letters'] and template_type == 'letter': @@ -150,13 +151,16 @@ def add_service_template(service_id, template_type): form = form_objects[template_type]() if form.validate_on_submit(): + if form.process_type.data == 'priority': + abort_403_if_not_admin_user() try: service_api_client.create_service_template( form.name.data, template_type, form.template_content.data, service_id, - form.subject.data if hasattr(form, 'subject') else None + form.subject.data if hasattr(form, 'subject') else None, + form.process_type.data ) except HTTPError as e: if e.status_code == 400: @@ -179,6 +183,11 @@ def add_service_template(service_id, template_type): ) +def abort_403_if_not_admin_user(): + if not current_user.has_permissions([], admin_override=True): + abort(403) + + @main.route("/services//templates//edit", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_templates', admin_override=True) @@ -188,13 +197,17 @@ def edit_service_template(service_id, template_id): form = form_objects[template['template_type']](**template) if form.validate_on_submit(): + if form.process_type.data != template['process_type']: + abort_403_if_not_admin_user() + subject = form.subject.data if hasattr(form, 'subject') else None new_template = get_template({ 'name': form.name.data, 'content': form.template_content.data, 'subject': subject, 'template_type': template['template_type'], - 'id': template['id'] + 'id': template['id'], + 'process_type': form.process_type.data }, current_service) template_change = get_template(template, current_service).compare_to(new_template) if template_change.has_different_placeholders and not request.form.get('confirm'): @@ -221,7 +234,8 @@ def edit_service_template(service_id, template_id): template['template_type'], form.template_content.data, service_id, - subject + subject, + form.process_type.data ) except HTTPError as e: if e.status_code == 400: diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index e974890db..0c5d8811b 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -116,7 +116,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): data = _attach_current_user({}) return self.delete(endpoint, data) - def create_service_template(self, name, type_, content, service_id, subject=None): + def create_service_template(self, name, type_, content, service_id, subject=None, process_type='normal'): """ Create a service template. """ @@ -124,7 +124,8 @@ class ServiceAPIClient(NotifyAdminAPIClient): "name": name, "template_type": type_, "content": content, - "service": service_id + "service": service_id, + "process_type": process_type } if subject: data.update({ @@ -132,9 +133,10 @@ class ServiceAPIClient(NotifyAdminAPIClient): }) data = _attach_current_user(data) endpoint = "/service/{0}/template".format(service_id) + print('got here in create_service_template') return self.post(endpoint, data) - def update_service_template(self, id_, name, type_, content, service_id, subject=None): + def update_service_template(self, id_, name, type_, content, service_id, subject=None, process_type=None): """ Update a service template. """ @@ -149,6 +151,10 @@ class ServiceAPIClient(NotifyAdminAPIClient): data.update({ 'subject': subject }) + if process_type: + data.update({ + 'process_type': process_type + }) data = _attach_current_user(data) endpoint = "/service/{0}/template/{1}".format(service_id, id_) return self.post(endpoint, data) diff --git a/app/templates/views/edit-email-template.html b/app/templates/views/edit-email-template.html index 5e90f48b1..56abd029a 100644 --- a/app/templates/views/edit-email-template.html +++ b/app/templates/views/edit-email-template.html @@ -20,6 +20,9 @@
{{ textbox(form.template_content, highlight_tags=True, width='1-1', rows=8) }} + {% if current_user.has_permissions([], admin_override=True) %} + {{ textbox(form.process_type, width='1-1') }} + {% endif %} {{ page_footer( 'Save', delete_link=url_for('.delete_service_template', service_id=current_service.id, template_id=template_id) if template_id or None, diff --git a/app/templates/views/edit-sms-template.html b/app/templates/views/edit-sms-template.html index 6f8ab64f8..a54f6544b 100644 --- a/app/templates/views/edit-sms-template.html +++ b/app/templates/views/edit-sms-template.html @@ -19,6 +19,9 @@
{{ textbox(form.template_content, highlight_tags=True, width='1-1', rows=5) }} + {% if current_user.has_permissions([], admin_override=True) %} + {{ textbox(form.process_type, width='1-1') }} + {% endif %} {{ page_footer( 'Save', delete_link=url_for('.delete_service_template', service_id=current_service.id, template_id=template_id) if template_id or None, diff --git a/tests/__init__.py b/tests/__init__.py index 0b169f68c..b93a487f4 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -84,7 +84,8 @@ def template_json(service_id, content="template content", subject=None, version=1, - archived=False): + archived=False, + process_type='normal'): template = { 'id': id_, 'name': name, @@ -93,7 +94,8 @@ def template_json(service_id, 'service': service_id, 'version': version, 'updated_at': datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S.%f'), - 'archived': archived + 'archived': archived, + 'process_type': process_type } if subject is not None: template['subject'] = subject diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 149c88879..0c42d25d5 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,4 +1,3 @@ -from functools import partial from itertools import repeat from datetime import datetime from unittest.mock import Mock, patch @@ -8,30 +7,26 @@ from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time from notifications_python_client.errors import HTTPError - +from tests.conftest import service_one as create_sample_service from tests import validate_route_permission, template_json, single_notification_json -from tests.conftest import ( - mock_get_service_email_template, - mock_get_template_version, -) + from app.main.views.templates import get_last_use_message, get_human_readable_delta def test_should_show_page_for_one_template( app_, - api_user_active, - mock_login, - mock_get_service, + active_user_with_permissions, + mocker, mock_get_service_template, - mock_get_user, - mock_get_user_by_email, - mock_has_permissions, fake_uuid ): + + service = create_sample_service(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - client.login(api_user_active) - service_id = fake_uuid + client.login(active_user_with_permissions, mocker, service) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) + service_id = service['id'] template_id = fake_uuid response = client.get(url_for( '.edit_service_template', @@ -41,6 +36,35 @@ def test_should_show_page_for_one_template( assert response.status_code == 200 assert "Two week reminder" in response.get_data(as_text=True) assert "Your vehicle tax is about to expire" in response.get_data(as_text=True) + assert "Select priority or normal" not in response.get_data(as_text=True) + mock_get_service_template.assert_called_with( + service_id, template_id) + + +def test_should_show_page_template_with_priority_select_if_platform_admin( + app_, + platform_admin_user, + mocker, + mock_get_service_template, + fake_uuid +): + + service = create_sample_service(platform_admin_user) + with app_.test_request_context(): + with app_.test_client() as client: + client.login(platform_admin_user, mocker, service) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[platform_admin_user]) + service_id = service['id'] + template_id = fake_uuid + response = client.get(url_for( + '.edit_service_template', + service_id=service_id, + template_id=template_id)) + + assert response.status_code == 200 + assert "Two week reminder" in response.get_data(as_text=True) + assert "Your vehicle tax is about to expire" in response.get_data(as_text=True) + assert "Select priority or normal" in response.get_data(as_text=True) mock_get_service_template.assert_called_with( service_id, template_id) @@ -92,19 +116,16 @@ def test_should_show_preview_letter_templates( def test_should_redirect_when_saving_a_template(app_, - api_user_active, - mock_login, + active_user_with_permissions, + mocker, mock_get_service_template, mock_update_service_template, - mock_get_user, - mock_get_service, - mock_get_user_by_email, - mock_has_permissions, fake_uuid): + service = create_sample_service(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - client.login(api_user_active) - service_id = fake_uuid + client.login(active_user_with_permissions, mocker, service) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) template_id = fake_uuid name = "new name" content = "template content" @@ -113,18 +134,109 @@ def test_should_redirect_when_saving_a_template(app_, 'name': name, 'template_content': content, 'template_type': 'sms', - 'service': service_id + 'service': service['id'], + 'process_type': 'normal' } response = client.post(url_for( '.edit_service_template', - service_id=service_id, + service_id=service['id'], template_id=template_id), data=data) assert response.status_code == 302 assert response.location == url_for( - '.view_template', service_id=service_id, template_id=template_id, _external=True) + '.view_template', service_id=service['id'], template_id=template_id, _external=True) mock_update_service_template.assert_called_with( - template_id, name, 'sms', content, service_id, None) + template_id, name, 'sms', content, service['id'], None, 'normal') + + +def test_should_edit_content_when_process_type_is_priority_not_platform_admin( + app_, + active_user_with_permissions, + mocker, + mock_get_service_template_with_priority, + mock_update_service_template, + fake_uuid): + service = create_sample_service(active_user_with_permissions) + with app_.test_request_context(): + with app_.test_client() as client: + client.login(active_user_with_permissions, mocker, service) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) + template_id = fake_uuid + data = { + 'id': template_id, + 'name': "new name", + 'template_content': "change the content", + 'template_type': 'sms', + 'service': service['id'], + 'process_type': 'priority' + } + response = client.post(url_for( + '.edit_service_template', + service_id=service['id'], + template_id=template_id), data=data) + assert response.status_code == 302 + assert response.location == url_for( + '.view_template', service_id=service['id'], template_id=template_id, _external=True) + mock_update_service_template.assert_called_with( + template_id, "new name", 'sms', "change the content", service['id'], None, 'priority') + + +def test_should_403_when_edit_template_with_process_type_of_priority_for_non_platform_admin( + app_, + active_user_with_permissions, + mocker, + mock_get_service_template, + mock_update_service_template, + fake_uuid): + service = create_sample_service(active_user_with_permissions) + with app_.test_request_context(): + with app_.test_client() as client: + client.login(active_user_with_permissions, mocker, service) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) + template_id = fake_uuid + data = { + 'id': template_id, + 'name': "new name", + 'template_content': "template content", + 'template_type': 'sms', + 'service': service['id'], + 'process_type': 'priority' + } + response = client.post(url_for( + '.edit_service_template', + service_id=service['id'], + template_id=template_id), data=data) + assert response.status_code == 403 + mock_update_service_template.called == 0 + + +def test_should_403_when_create_template_with_process_type_of_priority_for_non_platform_admin( + app_, + active_user_with_permissions, + mocker, + mock_get_service_template, + mock_update_service_template, + fake_uuid): + service = create_sample_service(active_user_with_permissions) + with app_.test_request_context(): + with app_.test_client() as client: + client.login(active_user_with_permissions, mocker, service) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) + template_id = fake_uuid + data = { + 'id': template_id, + 'name': "new name", + 'template_content': "template content", + 'template_type': 'sms', + 'service': service['id'], + 'process_type': 'priority' + } + response = client.post(url_for( + '.add_service_template', + service_id=service['id'], + template_type='sms'), data=data) + assert response.status_code == 403 + mock_update_service_template.called == 0 def test_should_show_interstitial_when_making_breaking_change( @@ -152,7 +264,8 @@ def test_should_show_interstitial_when_making_breaking_change( 'template_content': "hello", 'template_type': 'email', 'subject': 'reminder', - 'service': service_id + 'service': service_id, + 'process_type': 'normal' } ) @@ -188,7 +301,8 @@ def test_should_not_create_too_big_template(app_, 'name': "new name", 'template_content': "template content", 'template_type': template_type, - 'service': service_id + 'service': service_id, + 'process_type': 'normal' } resp = client.post(url_for( '.add_service_template', @@ -214,14 +328,14 @@ def test_should_not_update_too_big_template(app_, with app_.test_client() as client: client.login(api_user_active) service_id = fake_uuid - template_type = 'sms' template_id = fake_uuid data = { 'id': fake_uuid, 'name': "new name", 'template_content': "template content", 'service': service_id, - 'template_type': 'sms' + 'template_type': 'sms', + 'process_type': 'normal' } resp = client.post(url_for( '.edit_service_template', @@ -256,7 +370,8 @@ def test_should_redirect_when_saving_a_template_email(app_, 'template_content': content, 'template_type': 'email', 'service': service_id, - 'subject': subject + 'subject': subject, + 'process_type': 'normal' } response = client.post(url_for( '.edit_service_template', @@ -269,7 +384,7 @@ def test_should_redirect_when_saving_a_template_email(app_, template_id=template_id, _external=True) mock_update_service_template.assert_called_with( - template_id, name, 'email', content, service_id, subject) + template_id, name, 'email', content, service_id, subject, 'normal') def test_should_show_delete_template_page_with_time_block(app_, diff --git a/tests/conftest.py b/tests/conftest.py index 40da34ac5..054952fd9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -243,6 +243,23 @@ def mock_get_service_template(mocker): ) +@pytest.fixture(scope='function') +def mock_get_service_template_with_priority(mocker): + def _get(service_id, template_id, version=None): + + template = template_json( + service_id, template_id, "Two week reminder", "sms", "Your vehicle tax is about to expire", + process_type='priority') + if version: + template.update({'version': version}) + return {'data': template} + + return mocker.patch( + 'app.service_api_client.get_service_template', + side_effect=_get + ) + + @pytest.fixture(scope='function') def mock_get_deleted_template(mocker): def _get(service_id, template_id, version=None): @@ -381,7 +398,7 @@ def mock_create_service_template(mocker, fake_uuid): @pytest.fixture(scope='function') def mock_update_service_template(mocker): - def _update(id_, name, type_, content, service, subject=None): + def _update(id_, name, type_, content, service, subject=None, process_type=None): template = template_json( service, id_, name, type_, content, subject) return {'data': template} @@ -393,7 +410,7 @@ def mock_update_service_template(mocker): @pytest.fixture(scope='function') def mock_create_service_template_content_too_big(mocker): - def _create(name, type_, content, service, subject=None): + def _create(name, type_, content, service, subject=None, process_type=None): json_mock = Mock(return_value={ 'message': {'content': ["Content has a character count greater than the limit of 459"]}, 'result': 'error' @@ -411,7 +428,7 @@ def mock_create_service_template_content_too_big(mocker): @pytest.fixture(scope='function') def mock_update_service_template_400_content_too_big(mocker): - def _update(id_, name, type_, content, service, subject=None): + def _update(id_, name, type_, content, service, subject=None, process_type=None): json_mock = Mock(return_value={ 'message': {'content': ["Content has a character count greater than the limit of 459"]}, 'result': 'error'