Remove folders feature flag

Every service has this feature now[1] so we don’t need to check whether
or not it’s switched on any more.

1. https://github.com/alphagov/notifications-api/pull/2286

app/main/forms.py

@@ -947,7 +947,7 @@ class ChooseTemplateType(StripWhitespaceForm):
         ]
     )
 
-    def __init__(self, include_letters=False, include_copy=False, include_folder=False, *args, **kwargs):
+    def __init__(self, include_letters=False, include_copy=False, *args, **kwargs):
 
         super().__init__(*args, **kwargs)
 
@@ -956,7 +956,7 @@ class ChooseTemplateType(StripWhitespaceForm):
             ('sms', 'Text message template'),
             ('letter', 'Letter template') if include_letters else None,
             ('copy-existing', 'Copy of an existing template') if include_copy else None,
-            ('folder', 'Folder') if include_folder else None,
+            ('folder', 'Folder'),
         ])
 
 

app/main/views/templates.py

@@ -126,7 +126,7 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
     option_hints = {template_folder_id: 'current folder'}
 
     if request.method == 'POST' and templates_and_folders_form.validate_on_submit():
-        if not can_manage_folders():
+        if not current_user.has_permissions('manage_templates'):
             abort(403)
         try:
             return process_folder_management_form(templates_and_folders_form, template_folder_id)
@@ -139,7 +139,6 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
     return render_template(
         'views/templates/choose.html',
         current_template_folder_id=template_folder_id,
-        can_manage_folders=can_manage_folders(),
         template_folder_path=current_service.get_template_folder_path(template_folder_id),
         template_list=template_list,
         show_search_box=current_service.count_of_templates_and_folders > 7,
@@ -208,13 +207,6 @@ def get_template_nav_items(template_folder_id):
     ]
 
 
-def can_manage_folders():
-    return (
-        current_service.has_permission('edit_folders') and
-        current_user.has_permissions('manage_templates')
-    )
-
-
 @main.route("/services/<service_id>/templates/<template_id>.<filetype>")
 @login_required
 @user_has_permissions()
@@ -271,7 +263,6 @@ def add_template_by_type(service_id, template_folder_id=None):
         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')
     )
 
     if form.validate_on_submit():
@@ -427,8 +418,6 @@ def action_blocked(service_id, notification_type, return_to, template_id):
 @login_required
 @user_has_permissions('manage_templates')
 def add_template_folder(service_id, template_folder_id=None):
-    if not current_service.has_permission('edit_folders'):
-        abort(403)
 
     form = TemplateFolderForm()
 
@@ -450,8 +439,6 @@ def add_template_folder(service_id, template_folder_id=None):
 @login_required
 @user_has_permissions('manage_templates')
 def manage_template_folder(service_id, template_folder_id):
-    if not current_service.has_permission('edit_folders'):
-        abort(403)
 
     form = TemplateFolderForm(
         name=current_service.get_template_folder(template_folder_id)['name']
@@ -480,9 +467,6 @@ def manage_template_folder(service_id, template_folder_id):
 @user_has_permissions('manage_templates')
 def delete_template_folder(service_id, template_folder_id):
 
-    if not current_service.has_permission('edit_folders'):
-        abort(403)
-
     template_folder = current_service.get_template_folder(template_folder_id)
 
     form = TemplateFolderForm(name=template_folder['name'])

app/templates/views/templates/_template_list.html

@@ -13,7 +13,7 @@
   <nav id=template-list>
     {% for item in template_list %}
       <div class="template-list-item {% if can_manage_folders %}template-list-item-with-checkbox{% endif %} {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}">
-        {% if can_manage_folders %}
+        {% if current_user.has_permissions('manage_templates') %}
           {{ unlabelled_checkbox(
             id='templates-or-folder-{}'.format(item.id),
             name='templates_and_folders',

app/templates/views/templates/choose.html

@@ -48,7 +48,7 @@
   {% else %}
 
     <div class="grid-row {% if current_service.all_template_folders %}bottom-gutter-1-2{% else %}bottom-gutter-2-3{% endif %}">
-      <div class="{% if can_manage_folders %} column-five-sixths {% else %} column-two-thirds {% endif %}">
+      <div class="{% if current_user.has_permissions('manage_templates') %} column-five-sixths {% else %} column-two-thirds {% endif %}">
         {{ folder_path(
           folders=template_folder_path,
           service_id=current_service.id,
@@ -58,12 +58,12 @@
         ) }}
       </div>
       {% if current_user.has_permissions('manage_templates') %}
-        {% if not can_manage_folders %}
+        {% if not current_user.has_permissions('manage_templates') %}
           <div class="column-one-third">
             <a href="{{ url_for('.add_template_by_type', service_id=current_service.id, template_folder_id=current_template_folder_id) }}" class="button align-with-heading">Add new template</a>
           </div>
         {% endif %}
-        {% if can_manage_folders and current_template_folder_id %}
+        {% if current_user.has_permissions('manage_templates') and current_template_folder_id %}
           <div class="column-one-sixth">
             <a href="{{ url_for('.manage_template_folder', service_id=current_service.id, template_folder_id=current_template_folder_id) }}" class="folder-heading-manage-link">Manage</a>
           </div>
@@ -79,7 +79,7 @@
 
     {{ live_search(target_selector='#template-list .template-list-item', show=show_search_box, form=search_form) }}
 
-    {% if can_manage_folders %}
+    {% if current_user.has_permissions('manage_templates') %}
       {% call form_wrapper(
           module='template-folder-form',
           data_kwargs={'prev-state': templates_and_folders_form.op or None}

tests/app/main/views/test_letters.py

@@ -56,11 +56,11 @@ def test_letters_lets_in_without_permission(
 @pytest.mark.parametrize('permissions, choices', [
     (
         ['email', 'sms', 'letter'],
-        ['Email template', 'Text message template', 'Letter template', 'Copy of an existing template']
+        ['Email template', 'Text message template', 'Letter template', 'Copy of an existing template', 'Folder']
     ),
     (
         ['email', 'sms'],
-        ['Email template', 'Text message template', 'Copy of an existing template']
+        ['Email template', 'Text message template', 'Copy of an existing template', 'Folder']
     ),
 ])
 def test_given_option_to_add_letters_if_allowed(

tests/app/main/views/test_template_folders.py

@@ -46,7 +46,6 @@ def test_add_page_shows_option_for_folder(
     mock_get_service_templates,
     mock_get_organisations_and_services_for_user,
 ):
-    service_one['permissions'] += ['edit_folders']
     mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
 
     page = client_request.get(
@@ -70,7 +69,6 @@ def test_add_page_shows_option_for_folder(
 
 @pytest.mark.parametrize('parent_folder_id', [None, PARENT_FOLDER_ID])
 def test_get_add_template_folder_page(client_request, service_one, parent_folder_id):
-    service_one['permissions'] += ['edit_folders']
 
     page = client_request.get(
         'main.add_template_folder',
@@ -81,17 +79,10 @@ def test_get_add_template_folder_page(client_request, service_one, parent_folder
     assert page.select_one('input[name=name]') is not None
 
 
-def test_add_template_folder_page_rejects_if_service_doesnt_have_permission(client_request, service_one):
-    client_request.get('main.add_template_folder', service_id=service_one['id'], _expected_status=403)
-    client_request.post('main.add_template_folder', service_id=service_one['id'], _expected_status=403)
-
-
 @pytest.mark.parametrize('parent_folder_id', [None, PARENT_FOLDER_ID])
 def test_post_add_template_folder_page(client_request, service_one, mocker, parent_folder_id):
     mock_create = mocker.patch('app.template_folder_api_client.create_template_folder')
 
-    service_one['permissions'] += ['edit_folders']
-
     client_request.post(
         'main.add_template_folder',
         service_id=service_one['id'],
@@ -381,7 +372,7 @@ def test_should_show_templates_folder_page(
         ]}
     )
 
-    service_one['permissions'] += ['letter', 'edit_folders']
+    service_one['permissions'] += ['letter']
 
     page = client_request.get(
         'main.choose_template',
@@ -454,7 +445,6 @@ def test_add_template_by_type_should_redirect_to_add_service_template(
     mock_get_service_templates,
     mock_get_organisations_and_services_for_user,
 ):
-    service_one['permissions'] += ['edit_folders']
     client_request.post(
         'main.add_template_by_type',
         service_id=SERVICE_ONE_ID,
@@ -475,7 +465,6 @@ def test_add_template_by_type_should_redirect_to_view_template_for_letter(
         mock_get_organisations_and_services_for_user,
         mock_create_service_template
 ):
-    service_one['permissions'] += ['edit_folders']
     service_one['permissions'] += ['letter']
     client_request.post(
         'main.add_template_by_type',
@@ -538,7 +527,6 @@ def test_get_manage_folder_page(
     mock_get_template_folders.return_value = [
         {'id': folder_id, 'name': 'folder_two', 'parent_id': None},
     ]
-    service_one['permissions'] += ['edit_folders']
 
     page = client_request.get(
         'main.manage_template_folder',
@@ -557,7 +545,6 @@ def test_get_manage_folder_page(
 
 
 def test_manage_folder_page_404s(client_request, service_one, mock_get_template_folders):
-    service_one['permissions'] += ['edit_folders']
     client_request.get(
         'main.manage_template_folder',
         service_id=service_one['id'],
@@ -566,13 +553,15 @@ def test_manage_folder_page_404s(client_request, service_one, mock_get_template_
     )
 
 
-def test_get_manage_folder_page_no_permissions(client_request, service_one, mock_get_template_folders):
+def test_get_manage_folder_page_no_permissions(
-    folder_id = str(uuid.uuid4())
+    client_request,
-
+    active_user_view_permissions,
+):
+    client_request.login(active_user_view_permissions)
     client_request.get(
         'main.manage_template_folder',
-        service_id=service_one['id'],
+        service_id=SERVICE_ONE_ID,
-        template_folder_id=folder_id,
+        template_folder_id=PARENT_FOLDER_ID,
         _expected_status=403
     )
 
@@ -583,7 +572,6 @@ def test_rename_folder(client_request, service_one, mock_get_template_folders, m
     mock_get_template_folders.return_value = [
         {'id': folder_id, 'name': 'folder_two', 'parent_id': None},
     ]
-    service_one['permissions'] += ['edit_folders']
 
     client_request.post(
         'main.manage_template_folder',
@@ -606,7 +594,6 @@ def test_rename_folder(client_request, service_one, mock_get_template_folders, m
 def test_delete_template_folder_should_request_confirmation(
     client_request, service_one, mock_get_template_folders, mocker
 ):
-    service_one['permissions'] += ['edit_folders']
     folder_id = str(uuid.uuid4())
     mock_get_template_folders.side_effect = [[
         {'id': folder_id, 'name': 'sacrifice', 'parent_id': None},
@@ -644,7 +631,6 @@ def test_delete_template_folder_should_request_confirmation(
 def test_delete_template_folder_should_detect_non_empty_folder_on_get(
     client_request, service_one, mock_get_template_folders, mocker
 ):
-    service_one['permissions'] += ['edit_folders']
     folder_id = str(uuid.uuid4())
     template_id = str(uuid.uuid4())
     mock_get_template_folders.side_effect = [
@@ -683,7 +669,6 @@ def test_delete_folder(client_request, service_one, mock_get_template_folders, m
         'app.models.service.Service.get_templates',
         return_value=[],
     )
-    service_one['permissions'] += ['edit_folders']
 
     client_request.post(
         'main.delete_template_folder',
@@ -713,15 +698,6 @@ def test_delete_folder(client_request, service_one, mock_get_template_folders, m
         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,
@@ -731,9 +707,7 @@ def test_should_show_checkboxes_for_selecting_templates(
     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(
@@ -752,10 +726,13 @@ def test_should_show_checkboxes_for_selecting_templates(
         assert TEMPLATE_ONE_ID not in checkboxes[index]['id']
 
 
-@pytest.mark.parametrize('user,extra_service_permissions', [
+@pytest.mark.parametrize('user', [
-    (active_user_with_permissions, []),
+    active_user_view_permissions,
-    (active_user_view_permissions, ['edit_folders']),
+    active_caseworking_user,
-    (active_caseworking_user, ['edit_folders']),
+    pytest.param(
+        active_user_with_permissions,
+        marks=pytest.mark.xfail(raises=AssertionError),
+    ),
 ])
 def test_should_not_show_radios_and_buttons_for_move_destination_if_incorrect_permissions(
     client_request,
@@ -766,10 +743,7 @@ def test_should_not_show_radios_and_buttons_for_move_destination_if_incorrect_pe
     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(
@@ -795,8 +769,6 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi
     fake_uuid,
     active_user_with_permissions
 ):
-    service_one['permissions'] += ['edit_folders']
-
     client_request.login(active_user_with_permissions)
 
     FOLDER_TWO_ID = str(uuid.uuid4())
@@ -840,8 +812,6 @@ def test_move_to_shouldnt_select_a_folder_by_default(
     fake_uuid,
     active_user_with_permissions
 ):
-    service_one['permissions'] += ['edit_folders']
-
     client_request.login(active_user_with_permissions)
 
     FOLDER_TWO_ID = str(uuid.uuid4())
@@ -864,7 +834,6 @@ def test_should_be_able_to_move_to_existing_folder(
     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},
@@ -896,9 +865,9 @@ def test_should_be_able_to_move_to_existing_folder(
     )
 
 
-@pytest.mark.parametrize('user,extra_service_permissions', [
+@pytest.mark.parametrize('user, expected_status, expected_called', [
-    (active_user_view_permissions, ['edit_folders']),
+    (active_user_view_permissions, 403, False),
-    (active_user_with_permissions, [])
+    (active_user_with_permissions, 302, True),
 ])
 def test_should_not_be_able_to_move_to_existing_folder_if_dont_have_permission(
     client_request,
@@ -908,9 +877,9 @@ def test_should_not_be_able_to_move_to_existing_folder_if_dont_have_permission(
     mock_get_template_folders,
     mock_move_to_template_folder,
     user,
-    extra_service_permissions,
+    expected_status,
+    expected_called,
 ):
-    service_one['permissions'] += extra_service_permissions
     client_request.login(user(fake_uuid))
     FOLDER_TWO_ID = str(uuid.uuid4())
     mock_get_template_folders.return_value = [
@@ -928,9 +897,9 @@ def test_should_not_be_able_to_move_to_existing_folder_if_dont_have_permission(
                 TEMPLATE_ONE_ID,
             ],
         },
-        _expected_status=403
+        _expected_status=expected_status
     )
-    assert mock_move_to_template_folder.called is False
+    assert mock_move_to_template_folder.called is expected_called
 
 
 def test_move_folder_form_shows_current_folder_hint_when_in_a_folder(
@@ -939,7 +908,6 @@ def test_move_folder_form_shows_current_folder_hint_when_in_a_folder(
     mock_get_service_templates,
     mock_get_template_folders,
 ):
-    service_one['permissions'] += ['edit_folders']
     mock_get_template_folders.return_value = [
         {'id': PARENT_FOLDER_ID, 'name': 'parent_folder', 'parent_id': None},
         {'id': CHILD_FOLDER_ID, 'name': 'child_folder', 'parent_id': PARENT_FOLDER_ID},
@@ -967,7 +935,6 @@ def test_move_folder_form_does_not_show_current_folder_hint_at_the_top_level(
     mock_get_service_templates,
     mock_get_template_folders,
 ):
-    service_one['permissions'] += ['edit_folders']
     mock_get_template_folders.return_value = [
         {'id': PARENT_FOLDER_ID, 'name': 'parent_folder', 'parent_id': None},
         {'id': CHILD_FOLDER_ID, 'name': 'child_folder', 'parent_id': PARENT_FOLDER_ID},
@@ -996,7 +963,6 @@ def test_should_be_able_to_move_a_sub_item(
     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},
@@ -1081,7 +1047,7 @@ def test_no_action_if_user_fills_in_ambiguous_fields(
     mock_create_template_folder,
     data,
 ):
-    service_one['permissions'] += ['edit_folders', 'letter']
+    service_one['permissions'] += ['letter']
 
     mock_get_template_folders.return_value = [
         {'id': PARENT_FOLDER_ID, 'name': 'parent folder', 'parent_id': None},
@@ -1135,8 +1101,6 @@ def test_new_folder_is_created_if_only_new_folder_is_filled_out(
         'operation': 'add-new-folder'
     }
 
-    service_one['permissions'] += ['edit_folders']
-
     client_request.post(
         'main.choose_template',
         service_id=SERVICE_ONE_ID,
@@ -1166,7 +1130,6 @@ def test_should_be_able_to_move_to_new_folder(
     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 = [
@@ -1211,8 +1174,6 @@ def test_radio_button_with_no_value_shows_custom_error_message(
     mock_move_to_template_folder,
     mock_create_template_folder,
 ):
-    service_one['permissions'] += ['edit_folders']
-
     page = client_request.post(
         'main.choose_template',
         service_id=SERVICE_ONE_ID,
@@ -1254,7 +1215,6 @@ def test_show_custom_error_message(
         data,
         error_msg,
 ):
-    service_one['permissions'] += ['edit_folders']
     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},

tests/app/main/views/test_templates.py

@@ -59,11 +59,8 @@ def test_should_show_empty_page_when_no_templates(
     assert normalize_spaces(page.select_one('main p').text) == (
         'You need a template before you can send emails or text messages.'
     )
-    assert page.select_one('main a')['href'] == url_for(
+    assert page.select_one('#add_new_folder_form')
-        'main.add_template_by_type',
+    assert page.select_one('#add_new_template_form')
-        service_id=service_one['id'],
-    )
-    assert len(page.select('main a')) == 1
 
 
 def test_should_show_add_template_form_if_service_has_folder_permission(
@@ -73,9 +70,6 @@ def test_should_show_add_template_form_if_service_has_folder_permission(
     mock_get_service_templates_when_no_templates_exist,
     mock_get_template_folders,
 ):
-
-    service_one['permissions'] += ['edit_folders']
-
     page = client_request.get(
         'main.choose_template',
         service_id=service_one['id'],
@@ -275,7 +269,7 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
 
 
 @pytest.mark.parametrize('extra_permissions, expected_values, expected_labels', (
-    pytest.param(['edit_folders'], [
+    pytest.param([], [
         'email',
         'sms',
         'copy-existing',
@@ -284,7 +278,7 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
         'Text message template',
         'Copy of an existing template',
     ]),
-    pytest.param(['edit_folders', 'letter'], [
+    pytest.param(['letter'], [
         'email',
         'sms',
         'letter',
@@ -295,10 +289,6 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
         '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,
@@ -771,7 +761,6 @@ def test_choosing_to_copy_redirects(
     endpoint,
     data,
 ):
-    service_one['permissions'] += ['edit_folders']
     client_request.post(
         endpoint,
         service_id=SERVICE_ONE_ID,
@@ -1210,7 +1199,7 @@ def test_should_not_allow_creation_of_template_through_form_without_correct_perm
     data,
     expected_error,
 ):
-    service_one['permissions'] = ['edit_folders']
+    service_one['permissions'] = []
     page = client_request.post(
         endpoint,
         service_id=SERVICE_ONE_ID,