From 10100e51b6f7e9c96ac499587da05a795ea0460a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 28 Nov 2018 13:48:13 +0000 Subject: [PATCH 1/4] Let users add new template from the choose page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we’re letting users add new folders directly from the choose page it makes sense that they should also be able to add templates from there. This resolves the problem we saw in user research where people found it hard to know where to go to add a new folder when they were all behind one green button. --- app/main/forms.py | 29 +++- app/main/views/templates.py | 95 ++++++----- app/templates/views/templates/_move_to.html | 33 ++-- tests/app/main/views/test_template_folders.py | 5 +- tests/app/main/views/test_templates.py | 147 +++++++++++++++--- tests/conftest.py | 4 + 6 files changed, 234 insertions(+), 79 deletions(-) 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') From 8d0b3f47fd52222a8834440c8704c96e526f455f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 30 Nov 2018 15:02:55 +0000 Subject: [PATCH 2/4] Set form choices as a list, not an iterator Iterators can be exhausted, causing options to unexpectedly disappear from the radio buttons. --- app/main/forms.py | 4 +- tests/app/main/views/test_template_folders.py | 39 +++++++++++++++++-- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 6607c99a0..80ded47e9 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1200,12 +1200,12 @@ class TemplateAndFoldersSelectionForm(Form): if item['id'] != str(current_folder_id) ] - self.add_template_by_template_type.choices = filter(None, [ + self.add_template_by_template_type.choices = list(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') diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index dc65ede19..b4c5e97b4 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -935,7 +935,15 @@ def test_should_be_able_to_move_a_sub_item( 'templates_and_folders': [], 'move_to_new_folder_name': 'foo', 'move_to': None - } + }, + # add a new template, but also select move destination + { + 'operation': 'add_template', + 'templates_and_folders': [], + 'move_to_new_folder_name': '', + 'move_to': PARENT_FOLDER_ID, + 'add_template_by_template_type': 'email', + }, ]) def test_no_action_if_user_fills_in_ambiguous_fields( client_request, @@ -946,9 +954,14 @@ def test_no_action_if_user_fills_in_ambiguous_fields( mock_create_template_folder, data, ): - service_one['permissions'] += ['edit_folders'] + service_one['permissions'] += ['edit_folders', 'letter'] - client_request.post( + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'parent folder', 'parent_id': None}, + {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, + ] + + page = client_request.post( 'main.choose_template', service_id=SERVICE_ONE_ID, _data=data, @@ -959,6 +972,26 @@ def test_no_action_if_user_fills_in_ambiguous_fields( assert mock_move_to_template_folder.called is False assert mock_create_template_folder.called is False + assert page.select_one('button[value={}]'.format(data['operation'])) + + assert [ + 'email', + 'sms', + 'letter', + 'copy-existing', + ] == [ + radio['value'] + for radio in page.select('#add_new_template_form input[type=radio]') + ] + + assert [ + FOLDER_TWO_ID, + PARENT_FOLDER_ID, + ] == [ + radio['value'] + for radio in page.select('#move_to_folder_radios input[type=radio]') + ] + def test_new_folder_is_created_if_only_new_folder_is_filled_out( client_request, From 4c148c7c2324519b6bd363bdc062287f1fdd62ac Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 3 Dec 2018 13:19:28 +0000 Subject: [PATCH 3/4] Look at other services only if no templates This saves one call to the API or Redis in the common case where the current service does have templates. This is because `any()` evaluates all expressions before running, whereas `or` will only evaluate the second expression if the first returns `False`-y. --- app/main/views/templates.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index ad81a475e..5e4617ba9 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -258,10 +258,10 @@ def add_template_by_type(service_id, template_folder_id=None): form = ChooseTemplateType( include_letters=current_service.has_permission('letter'), - include_copy=any(( - len(current_service.all_templates) > 0, - len(user_api_client.get_service_ids_for_user(current_user)) > 1, - )), + include_copy=( + current_service.all_templates or + len(user_api_client.get_service_ids_for_user(current_user)) > 1 + ), include_folder=current_service.has_permission('edit_folders') ) From 2fcf278a5b701bdf6296a435eccae7c2ae408ff8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 3 Dec 2018 17:29:23 +0000 Subject: [PATCH 4/4] Use unambiguously magic value for `None` choices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WTForms coerces `None` as a choice to `'None'` as a string when rendering form fields (form fields will only ever have string data because that what the browser posts back). But internally WTForms coerces `None` to mean an unset value, ie where the user hasn’t selected a radio button: https://github.com/wtforms/wtforms/blob/283b2803206825158834f1828bbf749c129b7c47/src/wtforms/utils.py#L1-L20 We shouldn’t use `None` to mean two different things. And in fact we can’t, because it in effect means that we’re always getting a value for the `move_to` field, even if the user hasn’t chosen to move any templates. Which results in some very expected behaviour. --- app/main/forms.py | 32 +++++++++++-------- tests/app/main/views/test_service_settings.py | 7 ++-- tests/app/main/views/test_template_folders.py | 17 +++++++++- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 80ded47e9..d46815e8d 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -707,24 +707,26 @@ class ServicePostageForm(StripWhitespaceForm): class FieldWithNoneOption(): - # This needs to match the data the browser will post from - # - NONE_OPTION_VALUE = 'None' + # This is a special value that is specific to our forms. This is + # more expicit than casting `None` to a string `'None'` which can + # have unexpected edge cases + NONE_OPTION_VALUE = '__NONE__' + # When receiving Python data, eg when instantiating the form object + # we want to convert that data to our special value, so that it gets + # recognised as being one of the valid choices + def process_data(self, value): + self.data = self.NONE_OPTION_VALUE if value is None else value + + # After validation we want to convert it back to a Python `None` for + # use elsewhere, eg posting to the API def post_validate(self, form, validation_stopped): - if self.data == self.NONE_OPTION_VALUE: + if self.data == self.NONE_OPTION_VALUE and not validation_stopped: self.data = None class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): - - def validate(self, *args, **kwargs): - - if self.data == self.NONE_OPTION_VALUE: - self.data = None - return True - - return super().validate(*args, **kwargs) + pass class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField): @@ -1194,10 +1196,13 @@ class TemplateAndFoldersSelectionForm(Form): self.op = None self.is_move_op = self.is_add_folder_op = self.is_add_template_op = False + if current_folder_id is None: + current_folder_id = RadioFieldWithNoneOption.NONE_OPTION_VALUE + self.move_to.choices = [ (item['id'], item['name']) for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders) - if item['id'] != str(current_folder_id) + if item['id'] != current_folder_id ] self.add_template_by_template_type.choices = list(filter(None, [ @@ -1237,5 +1242,6 @@ class TemplateAndFoldersSelectionForm(Form): 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=[ + Optional(), required_for_ops('add_template') ]) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 391051cdb..53e62e402 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -2188,12 +2188,12 @@ def test_set_postage_saves( @pytest.mark.parametrize('current_branding, expected_values, expected_labels', [ (None, [ - 'None', '1', '2', '3', '4', '5', + '__NONE__', '1', '2', '3', '4', '5', ], [ 'GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4', 'org 5' ]), ('5', [ - '5', 'None', '1', '2', '3', '4', + '5', '__NONE__', '1', '2', '3', '4', ], [ 'org 5', 'GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4', ]), @@ -2314,7 +2314,8 @@ def test_should_preview_email_branding( @pytest.mark.parametrize('posted_value, submitted_value', ( ('1', '1'), - ('None', None), + ('__NONE__', None), + pytest.param('None', None, marks=pytest.mark.xfail(raises=AssertionError)), )) def test_should_set_branding_and_organisations( logged_in_platform_admin_client, diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index b4c5e97b4..6f0ff26d2 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -901,7 +901,7 @@ def test_should_be_able_to_move_a_sub_item( template_folder_id=PARENT_FOLDER_ID, _data={ 'operation': 'move_to_existing_folder', - 'move_to': 'None', + 'move_to': '__NONE__', 'templates_and_folders': [GRANDCHILD_FOLDER_ID], }, _expected_status=302, @@ -929,6 +929,13 @@ def test_should_be_able_to_move_a_sub_item( 'move_to_new_folder_name': 'foo', 'move_to': PARENT_FOLDER_ID }, + # move to existing, but no templates to move + { + 'operation': 'move_to_existing_folder', + 'templates_and_folders': [], + 'move_to_new_folder_name': '', + 'move_to': PARENT_FOLDER_ID + }, # move to new, but nothing selected to move { 'operation': 'move_to_new_folder', @@ -944,6 +951,14 @@ def test_should_be_able_to_move_a_sub_item( 'move_to': PARENT_FOLDER_ID, 'add_template_by_template_type': 'email', }, + # add a new template, but also move to root folder + { + 'operation': 'add_template', + 'templates_and_folders': [], + 'move_to_new_folder_name': '', + 'move_to': '__NONE__', + 'add_template_by_template_type': 'email', + }, ]) def test_no_action_if_user_fills_in_ambiguous_fields( client_request,