diff --git a/app/main/forms.py b/app/main/forms.py index b4ddb4b6a..2a1b5b66f 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -3,6 +3,7 @@ from datetime import datetime, timedelta from itertools import chain import pytz +from flask import request from flask_wtf import FlaskForm as Form from flask_wtf.file import FileAllowed from flask_wtf.file import FileField as FileField_wtf @@ -1134,7 +1135,34 @@ class TemplateFolderForm(StripWhitespaceForm): name = StringField('Folder name', validators=[DataRequired(message='Can’t be empty')]) +def required_for_ops(*operations): + operations = set(operations) + + def validate(form, field): + if form.op not in operations and field.data: + # super weird + raise ValidationError('Must be empty') + if form.op in operations and not field.data: + raise ValidationError('Can’t be empty') + return validate + + class TemplateAndFoldersSelectionForm(Form): + """ + This form also expects the form data to include an operation, based on which submit button is clicked. + If enter is pressed, unknown will be sent by a hidden submit button at the top of the form. + The value of this operation affects which fields are required, expected to be empty, or optional. + + * unknown + currently not implemented, but in the future will try and work out if there are any obvious commands that can be + assumed based on which fields are empty vs populated. + * move_to_existing_folder + must have data for templates_and_folders checkboxes, and move_to radios + * move_to_new_folder + must have data for move_to_new_folder_name, cannot have data for move_to_existing_folder_name + * add_new_folder + must have data for move_to_existing_folder_name, cannot have data for move_to_new_folder_name + """ ALL_TEMPLATES_FOLDER = { 'name': 'All templates', @@ -1154,11 +1182,39 @@ 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.move_to.choices = [ (item['id'], item['name']) for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders) if item['id'] != str(current_folder_id) ] - templates_and_folders = MultiCheckboxField('Choose templates or folders') - move_to = RadioFieldWithNoneOption('Choose a folder') + 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'} + + if not (self.is_add_op or self.is_move_op): + return False + + return super().validate() + + def get_folder_name(self): + if self.op == 'add_new_folder': + return self.add_new_folder_name.data + elif self.op == 'move_to_new_folder': + return self.move_to_new_folder_name.data + return None + + templates_and_folders = MultiCheckboxField('Choose templates or folders', validators=[ + required_for_ops('move_to_new_folder', 'move_to_existing_folder') + ]) + move_to = RadioFieldWithNoneOption('Choose a folder', validators=[ + Optional(), + required_for_ops('move_to_new_folder', 'move_to_existing_folder') + ]) + 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')]) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 4b5c43f85..4a2854a9a 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -118,12 +118,10 @@ def choose_template(service_id, template_type='all', template_folder_id=None): current_folder_id=template_folder_id, ) - if is_valid_template_operation('move', templates_and_folders_form): - current_service.move_to_folder( - ids_to_move=templates_and_folders_form.templates_and_folders.data, - move_to=templates_and_folders_form.move_to.data, - ) - return redirect(request.url) + if request.method == 'POST' and templates_and_folders_form.validate_on_submit(): + if not can_manage_folders(): + abort(403) + return process_folder_management_form(templates_and_folders_form, template_folder_id) return render_template( 'views/templates/choose.html', @@ -139,19 +137,30 @@ def choose_template(service_id, template_type='all', template_folder_id=None): template_nav_items=get_template_nav_items(template_folder_id), template_type=template_type, search_form=SearchTemplatesForm(), - templates_and_folders_form=templates_and_folders_form, + templates_and_folders_form=templates_and_folders_form ) -def is_valid_template_operation(operation_name, form): +def process_folder_management_form(form, current_folder_id): + new_folder_id = None - if ( - can_manage_folders() and - request.method == 'POST' and - request.form.get('operation') == operation_name and - form.validate_on_submit() - ): - return True + if form.is_add_op: + new_folder_id = template_folder_api_client.create_template_folder( + current_service.id, + name=form.get_folder_name(), + parent_id=current_folder_id + ) + + if form.is_move_op: + # if we've just made a folder, we also want to move there + move_to_id = new_folder_id or form.move_to.data + + current_service.move_to_folder( + ids_to_move=form.templates_and_folders.data, + move_to=move_to_id + ) + + return redirect(request.url) def get_template_nav_label(value): diff --git a/app/templates/views/templates/_move_to.html b/app/templates/views/templates/_move_to.html index 10ab965a2..10c6b7341 100644 --- a/app/templates/views/templates/_move_to.html +++ b/app/templates/views/templates/_move_to.html @@ -2,6 +2,23 @@ {% 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 selected', button_name='operation', button_value='move') }} + +
+ {{ 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') }} +
+
+
+
+ Add a new folder + {{ textbox(templates_and_folders_form.add_new_folder_name) }} + {{ page_footer('New folder', button_name='operation', button_value='add_new_folder') }} +
+
{% endif %} diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index 40552d38e..8cba7139c 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -3,6 +3,8 @@ {% from "components/message-count-label.html" import message_count_label %} {% from "components/textbox.html" import textbox %} {% from "components/live-search.html" import live_search %} +{% from "components/form.html" import form_wrapper %} +{% from "components/page-footer.html" import page_footer %} {% extends "withnav_template.html" %} @@ -76,10 +78,13 @@ {{ live_search(target_selector='#template-list .template-list-item', show=show_search_box, form=search_form) }} {% if can_manage_folders %} -
+ {% call form_wrapper() %} {% include 'views/templates/_template_list.html' %} - {% include 'views/templates/_move_to.html' %} -
+ {% with templates=templates, template_folders=template_folders, templates_and_folders_form=templates_and_folders_form %} + {% include 'views/templates/_move_to.html' %} + {% endwith %} + {% endcall %} + {% else %} {% include 'views/templates/_template_list.html' %} {% endif %} diff --git a/pytest.ini b/pytest.ini index a9b6c3bc3..cbc831544 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,4 +1,5 @@ [pytest] +xfail_strict=true testpaths = tests log_level = 999 env = diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 2e951a070..2943a6a6c 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -3,7 +3,14 @@ import uuid import pytest from flask import url_for -from tests.conftest import SERVICE_ONE_ID, normalize_spaces +from tests.conftest import ( + SERVICE_ONE_ID, + TEMPLATE_ONE_ID, + active_caseworking_user, + active_user_view_permissions, + active_user_with_permissions, + normalize_spaces, +) PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0' CHILD_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b' @@ -316,7 +323,6 @@ def test_should_show_templates_folder_page( mock_has_no_jobs, service_one, mocker, - fake_uuid, expected_title_tag, expected_page_title, expected_parent_link_args, @@ -441,7 +447,6 @@ def test_add_template_by_type_should_redirect_to_view_template_for_letter( service_one, mock_get_service_templates, mock_get_organisations_and_services_for_user, - fake_uuid, mock_create_service_template ): service_one['permissions'] += ['edit_folders'] @@ -667,3 +672,370 @@ def test_delete_folder(client_request, service_one, mock_get_template_folders, m ) mock_delete.assert_called_once_with(service_one['id'], folder_id) + + +@pytest.mark.parametrize('user', [ + pytest.param( + active_user_with_permissions + ), + pytest.param( + active_user_view_permissions, + marks=pytest.mark.xfail(raises=AssertionError) + ), + pytest.param( + active_caseworking_user, + marks=pytest.mark.xfail(raises=AssertionError) + ), +]) +@pytest.mark.parametrize('extra_service_permissions', [ + pytest.param( + ['edit_folders'] + ), + pytest.param( + [], + marks=pytest.mark.xfail(raises=AssertionError) + ), +]) +def test_should_show_checkboxes_for_selecting_templates( + client_request, + mocker, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_has_no_jobs, + fake_uuid, + user, + extra_service_permissions, +): + service_one['permissions'] += extra_service_permissions + client_request.login(user(fake_uuid)) + + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + checkboxes = page.select('input[name=templates_and_folders]') + + assert len(checkboxes) == 4 + + assert checkboxes[0]['value'] == TEMPLATE_ONE_ID + assert checkboxes[0]['id'] == 'templates-or-folder-{}'.format(TEMPLATE_ONE_ID) + + for index in (1, 2, 3): + assert checkboxes[index]['value'] != TEMPLATE_ONE_ID + assert TEMPLATE_ONE_ID not in checkboxes[index]['id'] + + +@pytest.mark.parametrize('user,extra_service_permissions', [ + (active_user_with_permissions, []), + (active_user_view_permissions, ['edit_folders']), + (active_caseworking_user, ['edit_folders']), +]) +def test_should_not_show_radios_and_buttons_for_move_destination_if_incorrect_permissions( + client_request, + mocker, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_has_no_jobs, + fake_uuid, + user, + extra_service_permissions, +): + service_one['permissions'] += extra_service_permissions + + client_request.login(user(fake_uuid)) + + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + radios = page.select('input[type=radio]') + radio_div = page.find('div', {'id': 'move_to_folder_radios'}) + assert radios == page.select('input[name=move_to]') + + assert not radios + assert not radio_div + assert page.find_all('button', {'name': 'operation'}) == [] + + +def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissions( + client_request, + mocker, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_has_no_jobs, + fake_uuid, + active_user_with_permissions +): + service_one['permissions'] += ['edit_folders'] + + client_request.login(active_user_with_permissions) + + FOLDER_TWO_ID = str(uuid.uuid4()) + FOLDER_ONE_TWO_ID = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, + {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, + {'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID}, + {'id': FOLDER_ONE_TWO_ID, 'name': 'folder_one_two', 'parent_id': PARENT_FOLDER_ID}, + ] + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + radios = page.select('input[type=radio]') + radio_div = page.find('div', {'id': 'move_to_folder_radios'}) + assert radios == page.select('input[name=move_to]') + + assert [x['value'] for x in radios] == [ + PARENT_FOLDER_ID, CHILD_FOLDER_ID, FOLDER_ONE_TWO_ID, FOLDER_TWO_ID, + ] + assert [x.text.strip() for x in radio_div.select('label')] == [ + 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two', + ] + assert set(x['value'] for x in page.find_all('button', {'name': 'operation'})) == { + 'unknown', + 'move_to_existing_folder', + 'move_to_new_folder', + 'add_new_folder' + } + + +def test_should_be_able_to_move_to_existing_folder( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, +): + service_one['permissions'] += ['edit_folders'] + FOLDER_TWO_ID = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, + {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, + ] + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + _data={ + 'operation': 'move_to_existing_folder', + 'move_to': PARENT_FOLDER_ID, + 'templates_and_folders': [ + FOLDER_TWO_ID, + TEMPLATE_ONE_ID, + ], + }, + _expected_status=302, + _expected_redirect=url_for( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ) + mock_move_to_template_folder.assert_called_once_with( + service_id=SERVICE_ONE_ID, + folder_id=PARENT_FOLDER_ID, + folder_ids={FOLDER_TWO_ID}, + template_ids={TEMPLATE_ONE_ID}, + ) + + +@pytest.mark.parametrize('user,extra_service_permissions', [ + (active_user_view_permissions, ['edit_folders']), + (active_user_with_permissions, []) +]) +def test_should_not_be_able_to_move_to_existing_folder_if_dont_have_permission( + client_request, + service_one, + fake_uuid, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, + user, + extra_service_permissions, +): + service_one['permissions'] += extra_service_permissions + client_request.login(user(fake_uuid)) + FOLDER_TWO_ID = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, + {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, + ] + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + _data={ + 'operation': 'move_to_existing_folder', + 'move_to': PARENT_FOLDER_ID, + 'templates_and_folders': [ + FOLDER_TWO_ID, + TEMPLATE_ONE_ID, + ], + }, + _expected_status=403 + ) + assert mock_move_to_template_folder.called is False + + +def test_should_be_able_to_move_a_sub_item( + client_request, + service_one, + fake_uuid, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, +): + service_one['permissions'] += ['edit_folders'] + GRANDCHILD_FOLDER_ID = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, + {'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID}, + {'id': GRANDCHILD_FOLDER_ID, 'name': 'folder_one_one_one', 'parent_id': CHILD_FOLDER_ID}, + ] + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + template_folder_id=PARENT_FOLDER_ID, + _data={ + 'operation': 'move_to_existing_folder', + 'move_to': 'None', + 'templates_and_folders': [GRANDCHILD_FOLDER_ID], + }, + _expected_status=302, + ) + mock_move_to_template_folder.assert_called_once_with( + service_id=SERVICE_ONE_ID, + folder_id=None, + folder_ids={GRANDCHILD_FOLDER_ID}, + template_ids=set(), + ) + + +@pytest.mark.parametrize('data', [ + # move to existing, but add new folder name given + { + 'operation': 'move_to_existing_folder', + 'templates_and_folders': [], + 'add_new_folder_name': 'foo', + 'move_to': PARENT_FOLDER_ID + }, + # move to existing, but move to new folder name given + { + 'operation': 'move_to_existing_folder', + 'templates_and_folders': [TEMPLATE_ONE_ID], + 'move_to_new_folder_name': 'foo', + 'move_to': PARENT_FOLDER_ID + }, + # move to new, but nothing selected to move + { + 'operation': 'move_to_new_folder', + 'templates_and_folders': [], + 'move_to_new_folder_name': 'foo', + 'move_to': None + } +]) +def test_no_action_if_user_fills_in_ambiguous_fields( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, + mock_create_template_folder, + data, +): + service_one['permissions'] += ['edit_folders'] + + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + _data=data, + _expected_status=200, + _expected_redirect=None, + ) + + assert mock_move_to_template_folder.called is False + assert mock_create_template_folder.called is False + + +def test_new_folder_is_created_if_only_new_folder_is_filled_out( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, + mock_create_template_folder +): + data = { + 'move_to_new_folder_name': '', + 'add_new_folder_name': 'new folder', + 'operation': 'add_new_folder' + } + + service_one['permissions'] += ['edit_folders'] + + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + _data=data, + _expected_status=302, + _expected_redirect=url_for( + 'main.choose_template', + service_id=service_one['id'], + template_folder_id=None, + _external=True, + ), + ) + + assert mock_move_to_template_folder.called is False + mock_create_template_folder.assert_called_once_with( + SERVICE_ONE_ID, + name='new folder', + parent_id=None + ) + + +def test_should_be_able_to_move_to_new_folder( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, + mock_create_template_folder, +): + service_one['permissions'] += ['edit_folders'] + new_folder_id = mock_create_template_folder.return_value + FOLDER_TWO_ID = str(uuid.uuid4()) + 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}, + ] + + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + template_folder_id=None, + _data={ + 'operation': 'move_to_new_folder', + 'move_to_new_folder_name': 'new folder', + 'templates_and_folders': [ + FOLDER_TWO_ID, + TEMPLATE_ONE_ID, + ], + }, + _expected_status=302, + _expected_redirect=url_for('main.choose_template', service_id=SERVICE_ONE_ID, _external=True), + ) + + mock_create_template_folder.assert_called_once_with( + SERVICE_ONE_ID, + name='new folder', + parent_id=None + ) + mock_move_to_template_folder.assert_called_once_with( + service_id=SERVICE_ONE_ID, + folder_id=new_folder_id, + folder_ids={FOLDER_TWO_ID}, + template_ids={TEMPLATE_ONE_ID}, + ) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 28f623382..be1a2c5e2 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,4 +1,3 @@ -import uuid from datetime import datetime from unittest.mock import ANY, Mock @@ -17,19 +16,13 @@ from tests import ( template_json, validate_route_permission, ) -from tests.app.main.views.test_template_folders import ( - CHILD_FOLDER_ID, - PARENT_FOLDER_ID, - _folder, -) +from tests.app.main.views.test_template_folders import PARENT_FOLDER_ID, _folder from tests.conftest import ( SERVICE_ONE_ID, SERVICE_TWO_ID, TEMPLATE_ONE_ID, active_caseworking_user, active_user_view_permissions, - active_user_with_permissions, - fake_uuid, mock_get_service_email_template, mock_get_service_letter_template, mock_get_service_template, @@ -176,265 +169,6 @@ def test_should_show_page_for_choosing_a_template( mock_get_template_folders.assert_called_once_with(SERVICE_ONE_ID) -@pytest.mark.parametrize('user', [ - pytest.param( - active_user_with_permissions - ), - pytest.param( - active_user_view_permissions, - marks=pytest.mark.xfail(raises=AssertionError) - ), - pytest.param( - active_caseworking_user, - marks=pytest.mark.xfail(raises=AssertionError) - ), -]) -@pytest.mark.parametrize('extra_service_permissions', [ - pytest.param( - ['edit_folders'] - ), - pytest.param( - [], - marks=pytest.mark.xfail(raises=AssertionError) - ), -]) -def test_should_show_checkboxes_for_selecting_templates( - client_request, - mocker, - service_one, - mock_get_service_templates, - mock_get_template_folders, - mock_has_no_jobs, - fake_uuid, - user, - extra_service_permissions, -): - service_one['permissions'] += extra_service_permissions - client_request.login(user(fake_uuid)) - - page = client_request.get( - 'main.choose_template', - service_id=SERVICE_ONE_ID, - ) - checkboxes = page.select('input[name=templates_and_folders]') - - assert len(checkboxes) == 4 - - assert checkboxes[0]['value'] == TEMPLATE_ONE_ID - assert checkboxes[0]['id'] == 'templates-or-folder-{}'.format(TEMPLATE_ONE_ID) - - for index in (1, 2, 3): - assert checkboxes[index]['value'] != TEMPLATE_ONE_ID - assert TEMPLATE_ONE_ID not in checkboxes[index]['id'] - - -@pytest.mark.parametrize('user', [ - pytest.param( - active_user_with_permissions - ), - pytest.param( - active_user_view_permissions, - marks=pytest.mark.xfail(raises=AssertionError) - ), - pytest.param( - active_caseworking_user, - marks=pytest.mark.xfail(raises=AssertionError) - ), -]) -@pytest.mark.parametrize('extra_service_permissions', [ - pytest.param( - ['edit_folders'] - ), - pytest.param( - [], - marks=pytest.mark.xfail(raises=AssertionError) - ), -]) -@pytest.mark.parametrize('folder_id, expected_destinations', [ - (None, []), - (fake_uuid(), []), -]) -def test_should_show_radio_buttons_for_move_destination( - client_request, - mocker, - service_one, - mock_get_service_templates, - mock_get_template_folders, - mock_has_no_jobs, - fake_uuid, - user, - extra_service_permissions, - folder_id, - expected_destinations, -): - service_one['permissions'] += extra_service_permissions - client_request.login(user(fake_uuid)) - FOLDER_TWO_ID = str(uuid.uuid4()) - FOLDER_ONE_TWO_ID = str(uuid.uuid4()) - mock_get_template_folders.return_value = [ - {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, - {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, - {'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID}, - {'id': FOLDER_ONE_TWO_ID, 'name': 'folder_one_two', 'parent_id': PARENT_FOLDER_ID}, - ] - page = client_request.get( - 'main.choose_template', - service_id=SERVICE_ONE_ID, - ) - radios = page.select('input[type=radio]') - labels = page.select('label[for^=move_to]') - assert radios == page.select('input[name=move_to]') - - assert [x['value'] for x in radios] == [ - PARENT_FOLDER_ID, CHILD_FOLDER_ID, FOLDER_ONE_TWO_ID, FOLDER_TWO_ID, - ] - assert [x.text.strip() for x in labels] == [ - 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two', - ] - assert page.select_one('button[name=operation]')['value'] == 'move' - - -@pytest.mark.parametrize('user', [ - pytest.param( - active_user_with_permissions - ), - pytest.param( - active_user_view_permissions, - marks=pytest.mark.xfail(raises=AssertionError) - ), - pytest.param( - active_caseworking_user, - marks=pytest.mark.xfail(raises=AssertionError) - ), -]) -@pytest.mark.parametrize('extra_service_permissions', [ - pytest.param( - ['edit_folders'] - ), - pytest.param( - [], - marks=pytest.mark.xfail(raises=AssertionError) - ), -]) -@pytest.mark.parametrize('folder_id, expected_destinations', [ - (None, []), - (fake_uuid(), []), -]) -def test_should_post_move_to_api( - client_request, - service_one, - fake_uuid, - mock_get_service_templates, - mock_get_template_folders, - mock_move_to_template_folder, - user, - extra_service_permissions, - folder_id, - expected_destinations, -): - service_one['permissions'] += extra_service_permissions - client_request.login(user(fake_uuid)) - FOLDER_TWO_ID = str(uuid.uuid4()) - mock_get_template_folders.return_value = [ - {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, - {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, - ] - client_request.post( - 'main.choose_template', - service_id=SERVICE_ONE_ID, - _data={ - 'operation': 'move', - 'move_to': PARENT_FOLDER_ID, - 'templates_and_folders': [ - FOLDER_TWO_ID, - TEMPLATE_ONE_ID, - ], - }, - _expected_status=302, - _expected_redirect=url_for( - 'main.choose_template', - service_id=SERVICE_ONE_ID, - _external=True, - ), - ) - mock_move_to_template_folder.assert_called_once_with( - service_id=SERVICE_ONE_ID, - folder_id=PARENT_FOLDER_ID, - folder_ids={FOLDER_TWO_ID}, - template_ids={TEMPLATE_ONE_ID}, - ) - - -def test_should_be_able_to_move_a_sub_item( - client_request, - service_one, - fake_uuid, - mock_get_service_templates, - mock_get_template_folders, - mock_move_to_template_folder, -): - service_one['permissions'] += ['edit_folders'] - GRANDCHILD_FOLDER_ID = str(uuid.uuid4()) - mock_get_template_folders.return_value = [ - {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, - {'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID}, - {'id': GRANDCHILD_FOLDER_ID, 'name': 'folder_one_one_one', 'parent_id': CHILD_FOLDER_ID}, - ] - client_request.post( - 'main.choose_template', - service_id=SERVICE_ONE_ID, - template_folder_id=PARENT_FOLDER_ID, - _data={ - 'operation': 'move', - 'move_to': 'None', - 'templates_and_folders': [GRANDCHILD_FOLDER_ID], - }, - _expected_status=302, - ) - mock_move_to_template_folder.assert_called_once_with( - service_id=SERVICE_ONE_ID, - folder_id=None, - folder_ids={GRANDCHILD_FOLDER_ID}, - template_ids=set(), - ) - - -@pytest.mark.parametrize('thing_to_move', [ - PARENT_FOLDER_ID, # Can’t move a folder inside itself - CHILD_FOLDER_ID, # Can’t move a folder which doesn’t belong to the service -]) -def test_should_validate_illegal_moves( - client_request, - service_one, - fake_uuid, - mock_get_service_templates, - mock_get_template_folders, - mock_move_to_template_folder, - thing_to_move, -): - service_one['permissions'] += 'edit_folders' - - FOLDER_TWO_ID = str(uuid.uuid4()) - mock_get_template_folders.return_value = [ - {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, - {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, - ] - client_request.post( - 'main.choose_template', - service_id=SERVICE_ONE_ID, - _data={ - 'operation': 'move', - 'move_to': PARENT_FOLDER_ID, - 'templates_and_folders': [ - thing_to_move, - ], - }, - _expected_status=200, - _expected_redirect=None, - ) - assert mock_move_to_template_folder.called is False - - def test_should_not_show_template_nav_if_only_one_type_of_template( client_request, mock_get_template_folders, diff --git a/tests/app/notify_client/test_template_folder_client.py b/tests/app/notify_client/test_template_folder_client.py index 4057797ca..3ab808930 100644 --- a/tests/app/notify_client/test_template_folder_client.py +++ b/tests/app/notify_client/test_template_folder_client.py @@ -8,7 +8,7 @@ from app.notify_client.template_folder_api_client import TemplateFolderAPIClient @pytest.mark.parametrize('parent_id', [uuid.uuid4(), None]) -def test_create_template_folder_calls_correct_api_endpoint(mocker, api_user_active, parent_id): +def test_create_template_folder_calls_correct_api_endpoint(mocker, parent_id): mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete') some_service_id = uuid.uuid4() @@ -25,7 +25,7 @@ def test_create_template_folder_calls_correct_api_endpoint(mocker, api_user_acti mock_redis_delete.assert_called_once_with('service-{}-template-folders'.format(some_service_id)) -def test_get_template_folders_calls_correct_api_endpoint(mocker, api_user_active): +def test_get_template_folders_calls_correct_api_endpoint(mocker): mock_redis_get = mocker.patch('app.notify_client.RedisClient.get', return_value=None) mock_redis_set = mocker.patch('app.notify_client.RedisClient.set') mock_api_get = mocker.patch( @@ -48,7 +48,7 @@ def test_get_template_folders_calls_correct_api_endpoint(mocker, api_user_active mock_redis_set.assert_called_once_with(redis_key, '{"a": "b"}', ex=604800) -def test_move_templates_and_folders(mocker, api_user_active): +def test_move_templates_and_folders(mocker): mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete') mock_api_post = mocker.patch('app.notify_client.NotifyAdminAPIClient.post') @@ -83,7 +83,7 @@ def test_move_templates_and_folders(mocker, api_user_active): ] -def test_move_templates_and_folders_to_root(mocker, api_user_active): +def test_move_templates_and_folders_to_root(mocker): mock_api_post = mocker.patch('app.notify_client.NotifyAdminAPIClient.post') @@ -105,7 +105,7 @@ def test_move_templates_and_folders_to_root(mocker, api_user_active): ) -def test_update_template_folder_calls_correct_api_endpoint(mocker, api_user_active): +def test_update_template_folder_calls_correct_api_endpoint(mocker): mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete') some_service_id = uuid.uuid4() @@ -123,7 +123,7 @@ def test_update_template_folder_calls_correct_api_endpoint(mocker, api_user_acti mock_redis_delete.assert_called_once_with('service-{}-template-folders'.format(some_service_id)) -def test_delete_template_folder_calls_correct_api_endpoint(mocker, api_user_active): +def test_delete_template_folder_calls_correct_api_endpoint(mocker): mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete') some_service_id = uuid.uuid4() diff --git a/tests/conftest.py b/tests/conftest.py index 5c64914fc..0d964d14b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3255,3 +3255,8 @@ def mock_get_template_folders(mocker): @pytest.fixture def mock_move_to_template_folder(mocker): return mocker.patch('app.template_folder_api_client.move_to_folder') + + +@pytest.fixture +def mock_create_template_folder(mocker): + return mocker.patch('app.template_folder_api_client.create_template_folder', return_value=sample_uuid())