Merge pull request #2517 from alphagov/new-template-button

html for templates move/new flow

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')])

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

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') }}
+  <button type="submit" name="operation" value="unknown" hidden></button>
+  <div id="move_to_folder_radios">
+    {{ radios(templates_and_folders_form.move_to) }}
+    {{ page_footer('Move', button_name='operation', button_value='move_to_existing_folder') }}
+  </div>
+  <div id="move_to_new_folder_form">
+    <fieldset>
+      <legend class="visuallyhidden">Move to a new folder</legend>
+      {{ 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') }}
+    </fieldset>
+  </div>
+  <div id="add_new_folder_form">
+    <fieldset>
+      <legend class="visuallyhidden">Add a new folder</legend>
+      {{ textbox(templates_and_folders_form.add_new_folder_name) }}
+      {{ page_footer('New folder', button_name='operation', button_value='add_new_folder') }}
+    </fieldset>
+  </div>
 {% endif %}

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 %}
-      <form method="post">
+      {% call form_wrapper() %}
         {% include 'views/templates/_template_list.html' %}
-        {% include 'views/templates/_move_to.html' %}
-      </form>
+        {% 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 %}

pytest.ini

@@ -1,4 +1,5 @@
 [pytest]
+xfail_strict=true
 testpaths = tests
 log_level = 999
 env =

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},
+    )

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,

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

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