diff --git a/app/main/forms.py b/app/main/forms.py index 2a1b5b66f..6607c99a0 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -717,7 +717,14 @@ class FieldWithNoneOption(): class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): - pass + + def validate(self, *args, **kwargs): + + if self.data == self.NONE_OPTION_VALUE: + self.data = None + return True + + return super().validate(*args, **kwargs) class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField): @@ -1174,6 +1181,8 @@ class TemplateAndFoldersSelectionForm(Form): all_template_folders, template_list, current_folder_id, + allow_adding_letter_template, + allow_adding_copy_of_template, *args, **kwargs ): @@ -1183,7 +1192,7 @@ class TemplateAndFoldersSelectionForm(Form): self.templates_and_folders.choices = template_list.as_id_and_name self.op = None - self.is_move_op = self.is_add_op = False + self.is_move_op = self.is_add_folder_op = self.is_add_template_op = False self.move_to.choices = [ (item['id'], item['name']) @@ -1191,13 +1200,21 @@ class TemplateAndFoldersSelectionForm(Form): if item['id'] != str(current_folder_id) ] + self.add_template_by_template_type.choices = filter(None, [ + ('email', 'Email template'), + ('sms', 'Text message template'), + ('letter', 'Letter template') if allow_adding_letter_template else None, + ('copy-existing', 'Copy of an existing template') if allow_adding_copy_of_template else None, + ]) + def validate(self): self.op = request.form.get('operation') self.is_move_op = self.op in {'move_to_existing_folder', 'move_to_new_folder'} - self.is_add_op = self.op in {'add_new_folder', 'move_to_new_folder'} + self.is_add_folder_op = self.op in {'add_new_folder', 'move_to_new_folder'} + self.is_add_template_op = self.op in {'add_template'} - if not (self.is_add_op or self.is_move_op): + if not (self.is_add_folder_op or self.is_move_op or self.is_add_template_op): return False return super().validate() @@ -1218,3 +1235,7 @@ class TemplateAndFoldersSelectionForm(Form): ]) add_new_folder_name = StringField('Folder name', validators=[required_for_ops('add_new_folder')]) move_to_new_folder_name = StringField('Folder name', validators=[required_for_ops('move_to_new_folder')]) + + add_template_by_template_type = RadioFieldWithNoneOption('Add new', validators=[ + required_for_ops('add_template') + ]) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 4a2854a9a..ad81a475e 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -116,6 +116,11 @@ def choose_template(service_id, template_type='all', template_folder_id=None): template_list=template_list, template_type=template_type, current_folder_id=template_folder_id, + allow_adding_letter_template=current_service.has_permission('letter'), + allow_adding_copy_of_template=( + current_service.all_templates or + len(user_api_client.get_service_ids_for_user(current_user)) > 1 + ), ) if request.method == 'POST' and templates_and_folders_form.validate_on_submit(): @@ -144,7 +149,13 @@ def choose_template(service_id, template_type='all', template_folder_id=None): def process_folder_management_form(form, current_folder_id): new_folder_id = None - if form.is_add_op: + if form.is_add_template_op: + return _add_template_by_type( + form.add_template_by_template_type.data, + current_folder_id, + ) + + if form.is_add_folder_op: new_folder_id = template_folder_api_client.create_template_folder( current_service.id, name=form.get_folder_name(), @@ -248,55 +259,59 @@ def add_template_by_type(service_id, template_folder_id=None): form = ChooseTemplateType( include_letters=current_service.has_permission('letter'), include_copy=any(( - service_api_client.count_service_templates(service_id) > 0, + len(current_service.all_templates) > 0, len(user_api_client.get_service_ids_for_user(current_user)) > 1, )), include_folder=current_service.has_permission('edit_folders') ) if form.validate_on_submit(): - - if form.template_type.data == 'copy-existing': - return redirect(url_for( - '.choose_template_to_copy', - service_id=service_id, - )) - - if form.template_type.data == 'letter': - blank_letter = service_api_client.create_service_template( - 'Untitled', - 'letter', - 'Body', - service_id, - 'Main heading', - 'normal', - template_folder_id - ) - return redirect(url_for( - '.view_template', - service_id=service_id, - template_id=blank_letter['data']['id'], - )) - - if email_or_sms_not_enabled(form.template_type.data, current_service.permissions): - return redirect(url_for( - '.action_blocked', - service_id=service_id, - notification_type=form.template_type.data, - return_to='add_new_template', - template_id='0' - )) - else: - return redirect(url_for( - '.add_service_template', - service_id=service_id, - template_type=form.template_type.data, - template_folder_id=template_folder_id, - )) + return _add_template_by_type(form.template_type.data, template_folder_id) return render_template('views/templates/add.html', form=form) +def _add_template_by_type(template_type, template_folder_id): + + if template_type == 'copy-existing': + return redirect(url_for( + '.choose_template_to_copy', + service_id=current_service.id, + )) + + if template_type == 'letter': + blank_letter = service_api_client.create_service_template( + 'Untitled', + 'letter', + 'Body', + current_service.id, + 'Main heading', + 'normal', + template_folder_id + ) + return redirect(url_for( + '.view_template', + service_id=current_service.id, + template_id=blank_letter['data']['id'], + )) + + if email_or_sms_not_enabled(template_type, current_service.permissions): + return redirect(url_for( + '.action_blocked', + service_id=current_service.id, + notification_type=template_type, + return_to='add_new_template', + template_id='0' + )) + else: + return redirect(url_for( + '.add_service_template', + service_id=current_service.id, + template_type=template_type, + template_folder_id=template_folder_id, + )) + + @main.route("/services//templates/copy") @login_required @user_has_permissions('manage_templates') diff --git a/app/templates/views/templates/_move_to.html b/app/templates/views/templates/_move_to.html index 10c6b7341..b8e35407e 100644 --- a/app/templates/views/templates/_move_to.html +++ b/app/templates/views/templates/_move_to.html @@ -1,19 +1,20 @@ {% from "components/radios.html" import radios %} {% from "components/page-footer.html" import page_footer %} -{% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %} -
- {{ radios(templates_and_folders_form.move_to) }} - {{ page_footer('Move', button_name='operation', button_value='move_to_existing_folder') }} -
-
-
- Move to a new folder - {{ textbox(templates_and_folders_form.move_to_new_folder_name) }} - {{ page_footer('Move to a new folder', button_name='operation', button_value='move_to_new_folder') }} -
-
+ {% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %} +
+ {{ radios(templates_and_folders_form.move_to) }} + {{ page_footer('Move', button_name='operation', button_value='move_to_existing_folder') }} +
+
+
+ Move to a new folder + {{ textbox(templates_and_folders_form.move_to_new_folder_name) }} + {{ page_footer('Move to a new folder', button_name='operation', button_value='move_to_new_folder') }} +
+
+ {% endif %}
Add a new folder @@ -21,4 +22,10 @@ {{ page_footer('New folder', button_name='operation', button_value='add_new_folder') }}
-{% endif %} +
+
+ Add a new template + {{ radios(templates_and_folders_form.add_template_by_template_type) }} + {{ page_footer('Continue', button_name='operation', button_value='add_template') }} +
+
diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 2943a6a6c..dc65ede19 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -785,7 +785,7 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi 'main.choose_template', service_id=SERVICE_ONE_ID, ) - radios = page.select('input[type=radio]') + radios = page.select('#move_to_folder_radios input[type=radio]') radio_div = page.find('div', {'id': 'move_to_folder_radios'}) assert radios == page.select('input[name=move_to]') @@ -799,7 +799,8 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi 'unknown', 'move_to_existing_folder', 'move_to_new_folder', - 'add_new_folder' + 'add_new_folder', + 'add_template', } diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index be1a2c5e2..6ade834fc 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -21,6 +21,7 @@ from tests.conftest import ( SERVICE_ONE_ID, SERVICE_TWO_ID, TEMPLATE_ONE_ID, + ElementNotFound, active_caseworking_user, active_user_view_permissions, mock_get_service_email_template, @@ -40,6 +41,7 @@ from tests.conftest import single_letter_contact_block def test_should_show_empty_page_when_no_templates( client_request, service_one, + mock_get_organisations_and_services_for_user, mock_get_service_templates_when_no_templates_exist, mock_get_template_folders, extra_permissions, @@ -128,6 +130,7 @@ def test_should_show_empty_page_when_no_templates( ) def test_should_show_page_for_choosing_a_template( client_request, + mock_get_organisations_and_services_for_user, mock_get_service_templates, mock_get_template_folders, mock_has_no_jobs, @@ -242,6 +245,63 @@ 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(['edit_folders'], [ + 'email', + 'sms', + 'copy-existing', + ], [ + 'Email template', + 'Text message template', + 'Copy of an existing template', + ]), + pytest.param(['edit_folders', 'letter'], [ + 'email', + 'sms', + 'letter', + 'copy-existing', + ], [ + 'Email template', + 'Text message template', + 'Letter template', + 'Copy of an existing template', + ]), + pytest.param( + [], [], [], + marks=pytest.mark.xfail(raises=ElementNotFound) + ), +)) +def test_should_show_new_template_choices_if_service_has_folder_permission( + client_request, + service_one, + mock_get_organisations_and_services_for_user, + mock_get_service_templates, + mock_get_template_folders, + extra_permissions, + expected_values, + expected_labels, +): + service_one['permissions'] += extra_permissions + + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + + if not page.select('#add_new_template_form'): + raise ElementNotFound() + + assert normalize_spaces(page.select_one('#add_new_template_form fieldset fieldset legend').text) == ( + 'Add new' + ) + assert [ + choice['value'] for choice in page.select('#add_new_template_form input[type=radio]') + ] == expected_values + assert [ + normalize_spaces(choice.text) for choice in page.select('#add_new_template_form label') + ] == expected_labels + + def test_should_show_page_for_one_template( logged_in_client, mock_get_service_template, @@ -551,15 +611,35 @@ def test_dont_show_preview_letter_templates_for_bad_filetype( assert mock_get_service_template.called is False +@pytest.mark.parametrize('endpoint, data', ( + ('main.add_template_by_type', { + 'template_type': 'copy-existing' + }), + ('main.choose_template', { + 'operation': 'add_template', + 'add_template_by_template_type': 'copy-existing' + }), +)) def test_choosing_to_copy_redirects( client_request, + service_one, mock_get_service_templates, + mock_get_template_folders, mock_get_organisations_and_services_for_user, + endpoint, + data, ): + service_one['permissions'] += ['edit_folders'] client_request.post( - 'main.add_template_by_type', + endpoint, service_id=SERVICE_ONE_ID, - _data={'template_type': 'copy-existing'} + _data=data, + _expected_status=302, + _expected_redirect=url_for( + 'main.choose_template_to_copy', + service_id=SERVICE_ONE_ID, + _external=True, + ), ) @@ -693,29 +773,56 @@ def test_cant_copy_template_from_non_member_service( assert mock_get_service_email_template.call_args_list == [] -@pytest.mark.parametrize('type_of_template', ['email', 'sms']) +@pytest.mark.parametrize('endpoint, data, expected_error', ( + ( + 'main.add_template_by_type', + { + 'template_type': 'email', + }, + "Sending emails has been disabled for your service." + ), + ( + 'main.add_template_by_type', + { + 'template_type': 'sms', + }, + "Sending text messages has been disabled for your service." + ), + ( + 'main.choose_template', + { + 'operation': 'add_template', + 'add_template_by_template_type': 'email', + }, + "Sending emails has been disabled for your service." + ), + ( + 'main.choose_template', + { + 'operation': 'add_template', + 'add_template_by_template_type': 'sms', + }, + "Sending text messages has been disabled for your service." + ), +)) def test_should_not_allow_creation_of_template_through_form_without_correct_permission( - logged_in_client, + client_request, service_one, - mocker, mock_get_service_templates, + mock_get_template_folders, mock_get_organisations_and_services_for_user, - type_of_template, + endpoint, + data, + expected_error, ): - service_one['permissions'] = [] - template_description = {'sms': 'text messages', 'email': 'emails'} - - response = logged_in_client.post(url_for( - '.add_template_by_type', - service_id=service_one['id']), - data={'template_type': type_of_template}, - follow_redirects=True) - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert response.status_code == 200 - assert page.select('main p')[0].text.strip() == \ - "Sending {} has been disabled for your service.".format(template_description[type_of_template]) + service_one['permissions'] = ['edit_folders'] + page = client_request.post( + endpoint, + service_id=SERVICE_ONE_ID, + _data=data, + _follow_redirects=True, + ) + assert normalize_spaces(page.select('main p')[0].text) == expected_error assert page.select(".page-footer-back-link")[0].text == "Back to add new template" assert page.select(".page-footer-back-link")[0]['href'] == url_for( '.add_template_by_type', diff --git a/tests/conftest.py b/tests/conftest.py index 0d964d14b..824d580d7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -32,6 +32,10 @@ from . import ( ) +class ElementNotFound(Exception): + pass + + @pytest.fixture def app_(request): app = Flask('app')