Merge pull request #3558 from alphagov/only-allow-new-template-if-permission-is-there

Services can only create templates of types they have turned on
This commit is contained in:
Pea M. Tyczynska
2020-08-17 14:39:16 +01:00
committed by GitHub
8 changed files with 121 additions and 20 deletions

View File

@@ -153,12 +153,19 @@
return changed;
};
this.$singleNotificationChannel = (document.querySelector('div[id=add_new_template_form]')).getAttribute("data-channel");
this.$singleChannelService = (document.querySelector('div[id=add_new_template_form]')).getAttribute("data-service");
this.actionButtonClicked = function(event) {
event.preventDefault();
this.currentState = $(event.currentTarget).val();
if (this.stateChanged()) {
this.render();
if (event.currentTarget.value === 'add-new-template' && this.$singleNotificationChannel) {
window.location = "/services/" + this.$singleChannelService + "/templates/add-" + this.$singleNotificationChannel;
} else {
if (this.stateChanged()) {
this.render();
}
}
};

View File

@@ -1997,12 +1997,8 @@ class TemplateAndFoldersSelectionForm(Form):
]
self.add_template_by_template_type.choices = list(filter(None, [
# We want to show email and text message to everyone,
# whether or not the service has them switched on. The
# option to add letter or broadcast templates should only
# be shown to services which have that permission
('email', 'Email'),
('sms', 'Text message'),
('email', 'Email') if 'email' in available_template_types else None,
('sms', 'Text message') if 'sms' in available_template_types else None,
('letter', 'Letter') if 'letter' in available_template_types else None,
('broadcast', 'Broadcast') if 'broadcast' in available_template_types else None,
('copy-existing', 'Copy an existing template') if allow_adding_copy_of_template else None,

View File

@@ -31,6 +31,7 @@ from app.models.service import Service
from app.models.template_list import TemplateList, TemplateLists
from app.template_previews import TemplatePreview, get_page_count_for_letter
from app.utils import (
NOTIFICATION_TYPES,
get_template,
should_skip_template_page,
user_has_permissions,
@@ -130,6 +131,11 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
)
option_hints = {template_folder_id: 'current folder'}
single_notification_channel = None
notification_channels = list(set(current_service.permissions).intersection(NOTIFICATION_TYPES))
if len(notification_channels) == 1:
single_notification_channel = notification_channels[0]
if request.method == 'POST' and templates_and_folders_form.validate_on_submit():
if not current_user.has_permissions('manage_templates'):
abort(403)
@@ -168,6 +174,7 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
templates_and_folders_form=templates_and_folders_form,
move_to_children=templates_and_folders_form.move_to.children(),
user_has_template_folder_permission=user_has_template_folder_permission,
single_notification_channel=single_notification_channel,
option_hints=option_hints
)

View File

@@ -5,13 +5,13 @@
{% from "components/form.html" import form_wrapper %}
{% block service_page_title %}
{{ heading_action }} broadcast template
{{ heading_action }} template
{% endblock %}
{% block maincolumn_content %}
{{ page_header(
'{} broadcast template'.format(heading_action),
'{} template'.format(heading_action),
back_link=url_for('main.view_template', service_id=current_service.id, template_id=template.id) if template else url_for('main.choose_template', service_id=current_service.id, template_folder_id=template_folder_id)
) }}

View File

@@ -28,7 +28,7 @@
{{ page_footer('Add new folder', button_name='operation', button_value='add-new-folder') }}
</fieldset>
</div>
<div id="add_new_template_form">
<div id="add_new_template_form" {% if single_notification_channel %}data-channel="{{single_notification_channel}}" data-service="{{current_service.id}}"{% endif %}>
<div class="js-will-stick-at-bottom-when-scrolling">
{{ radios(templates_and_folders_form.add_template_by_template_type) }}
</div>

View File

@@ -60,6 +60,8 @@ FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure',
'technical-failure', 'virus-scan-failed', 'validation-failed']
REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES
NOTIFICATION_TYPES = ["sms", "email", "letter", "broadcast"]
with open('{}/email_domains.txt'.format(
os.path.dirname(os.path.realpath(__file__))

View File

@@ -287,8 +287,8 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
assert count_of_templates == 4
@pytest.mark.parametrize('extra_permissions, expected_values, expected_labels', (
pytest.param([], [
@pytest.mark.parametrize('service_permissions, expected_values, expected_labels', (
pytest.param(['email', 'sms'], [
'email',
'sms',
'copy-existing',
@@ -297,7 +297,12 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
'Text message',
'Copy an existing template',
]),
pytest.param(['letter'], [
pytest.param(['broadcast'], [
'broadcast',
], [
'Broadcast',
]),
pytest.param(['email', 'sms', 'letter'], [
'email',
'sms',
'letter',
@@ -314,11 +319,11 @@ def test_should_show_new_template_choices_if_service_has_folder_permission(
service_one,
mock_get_service_templates,
mock_get_template_folders,
extra_permissions,
service_permissions,
expected_values,
expected_labels,
):
service_one['permissions'] += extra_permissions
service_one['permissions'] = service_permissions
page = client_request.get(
'main.choose_template',
@@ -339,6 +344,39 @@ def test_should_show_new_template_choices_if_service_has_folder_permission(
] == expected_labels
@pytest.mark.parametrize("permissions,are_data_attrs_added", [
(['sms'], True),
(['email'], True),
(['letter'], True),
(['broadcast'], True),
(['sms', 'email'], False),
])
def test_should_add_data_attributes_for_services_that_only_allow_one_type_of_notifications(
client_request,
service_one,
mock_get_service_templates,
mock_get_template_folders,
permissions,
are_data_attrs_added
):
service_one['permissions'] = permissions
page = client_request.get(
'main.choose_template',
service_id=SERVICE_ONE_ID,
)
if not page.select('#add_new_template_form'):
raise ElementNotFound()
if are_data_attrs_added:
assert page.find(id='add_new_template_form').attrs['data-channel'] == permissions[0]
assert page.find(id='add_new_template_form').attrs['data-service'] == SERVICE_ONE_ID
else:
assert page.find(id='add_new_template_form').attrs.get('data-channel') is None
assert page.find(id='add_new_template_form').attrs.get('data-service') is None
def test_should_show_page_for_one_template(
client_request,
mock_get_service_template,
@@ -2014,6 +2052,26 @@ def test_route_invalid_permissions(
service_one)
@pytest.mark.parametrize('template_type, expected', (
('email', 'New email template'),
('sms', 'New text message template'),
('broadcast', 'New template'),
))
def test_add_template_page_title(
client_request,
service_one,
template_type,
expected,
):
service_one['permissions'] += [template_type]
page = client_request.get(
'.add_service_template',
service_id=SERVICE_ONE_ID,
template_type=template_type,
)
assert normalize_spaces(page.select_one('h1').text) == expected
def test_can_create_email_template_with_emoji(
client_request,
mock_create_service_template

View File

@@ -1,6 +1,6 @@
const helpers = require('./support/helpers');
function setFixtures (hierarchy) {
function setFixtures (hierarchy, newTemplateDataModules = "") {
const foldersCheckboxesHTML = function (filter) {
let count = 0;
@@ -27,7 +27,7 @@ function setFixtures (hierarchy) {
}();
function controlsHTML () {
function controlsHTML (newTemplateDataModules) {
return `<div id="sticky_template_forms">
<button type="submit" name="operation" value="unknown" hidden=""></button>
@@ -78,7 +78,7 @@ function setFixtures (hierarchy) {
</div>
</fieldset>
</div>
<div id="add_new_template_form">
<div id="add_new_template_form" ${newTemplateDataModules}>
<div class="js-will-stick-at-bottom-when-scrolling">
<div class="form-group ">
<fieldset id="add_template_by_template_type">
@@ -127,7 +127,7 @@ function setFixtures (hierarchy) {
document.body.innerHTML = `
<form method="post" data-module="template-folder-form">
${helpers.templatesAndFoldersCheckboxes(hierarchy)}
${controlsHTML()}
${controlsHTML(newTemplateDataModules)}
</form>`;
};
@@ -309,6 +309,37 @@ describe('TemplateFolderForm', () => {
});
describe("Click 'New template' for single channel service", () => {
test("should redirect to new template page", () => {
setFixtures(hierarchy, "data-channel='sms' data-service='123'")
templateFolderForm = document.querySelector('form[data-module=template-folder-form]');
// start module
window.GOVUK.modules.start();
formControls = templateFolderForm.querySelector('#sticky_template_forms');
// reset sticky JS mocks called when the module starts
resetStickyMocks();
// add listener for url change
const descriptor1 = Object.getOwnPropertyDescriptor(window, 'location');
delete window.location
const mockCallback = jest.fn(x => {});
Object.defineProperty(window, 'location', {
set: mockCallback
});
// click
helpers.triggerEvent(formControls.querySelector('[value=add-new-template]'), 'click');
// expect url to change
expect(mockCallback).toHaveBeenCalledWith("/services/123/templates/add-sms")
setFixtures(hierarchy)
resetStickyMocks()
});
})
describe("Clicking 'New template'", () => {
beforeEach(() => {