Add a selectField to edit and create templates that is only visible for platform admins that makes the template a priority template.

There is a check that the template can not be created as priority if the user is not a platform admin.
There is a check that the template can not change the `priority` unless they are a platform admin.
This commit is contained in:
Rebecca Law
2017-01-18 15:11:34 +00:00
parent dfc751ef02
commit 3dfb3806d2
8 changed files with 210 additions and 45 deletions

View File

@@ -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):

View File

@@ -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/<service_id>/templates/<template_id>/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:

View File

@@ -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)

View File

@@ -20,6 +20,9 @@
</div>
<div class="column-three-quarters">
{{ 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,

View File

@@ -19,6 +19,9 @@
</div>
<div class="column-two-thirds">
{{ 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,

View File

@@ -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

View File

@@ -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_,

View File

@@ -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'