moved existing template folder tests into test_template_folders

also refactored slightly to work with new html, and separate succesful
and bad-permissions test cases

tests/app/main/views/test_template_folders.py

@@ -3,7 +3,15 @@ import uuid
 import pytest
 from flask import url_for
 
-from tests.conftest import SERVICE_ONE_ID, normalize_spaces
+from tests import sample_uuid
+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 +324,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 +448,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 +673,258 @@ 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,should_show_radio_buttons', [
+    (active_user_with_permissions, ['edit_folders'], True),
+    (active_user_with_permissions, [], False),
+    (active_user_view_permissions, ['edit_folders'], False),
+    (active_caseworking_user, ['edit_folders'], False),
+])
+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,
+    user,
+    extra_service_permissions,
+    should_show_radio_buttons,
+):
+    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]')
+
+    if should_show_radio_buttons:
+        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 set(x['value'] for x in page.find_all('button', {'name': 'operation'})) == {
+            'unknown',
+            'move_to_existing_folder',
+            'move_to_new_folder',
+            'add_new_folder'
+        }
+    else:
+        assert not radios
+        assert not labels
+        assert page.find_all('button', {'name': 'operation'}) == []
+
+
+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,
+            ],
+        },
+        # it's just returned the form. Should this have a 400 because there were errors?
+        _expected_status=200
+    )
+    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',
+            '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,
+    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

tests/app/main/views/test_templates.py

@@ -1,4 +1,3 @@
-import uuid
 from datetime import datetime
 from unittest.mock import ANY, Mock
 
@@ -28,8 +27,6 @@ from tests.conftest import (
     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 +173,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()