Merge pull request #2856 from alphagov/edit-members-who-see-folder

Edit members who see folder

app/main/forms.py

@@ -1285,6 +1285,15 @@ class ReturnedLettersForm(StripWhitespaceForm):
 
 
 class TemplateFolderForm(StripWhitespaceForm):
+    def __init__(self, all_service_users=None, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if all_service_users is not None:
+            self.users_with_permission.all_service_users = all_service_users
+            self.users_with_permission.choices = [
+                (item.id, item.name) for item in all_service_users
+            ]
+
+    users_with_permission = MultiCheckboxField('Users who can see this folder:')
     name = StringField('Folder name', validators=[DataRequired(message='Can’t be empty')])
 
 

app/main/views/templates.py

@@ -430,14 +430,19 @@ def action_blocked(service_id, notification_type, return_to, template_id):
 @login_required
 @user_has_permissions('manage_templates')
 def manage_template_folder(service_id, template_folder_id):
-
+    current_folder = current_service.get_template_folder(template_folder_id)
     form = TemplateFolderForm(
-        name=current_service.get_template_folder(template_folder_id)['name']
+        name=current_folder['name'],
+        users_with_permission=current_folder.get('users_with_permission', None),
+        all_service_users=[user for user in current_service.active_users if user.id != current_user.id]
     )
-
     if form.validate_on_submit():
+        users_with_permission = form.users_with_permission.data + [current_user.id]
         template_folder_api_client.update_template_folder(
-            current_service.id, template_folder_id, name=form.name.data
+            current_service.id,
+            template_folder_id,
+            name=form.name.data,
+            users_with_permission=users_with_permission
         )
         return redirect(
             url_for('.choose_template', service_id=service_id, template_folder_id=template_folder_id)
@@ -449,7 +454,7 @@ def manage_template_folder(service_id, template_folder_id):
         template_folder_path=current_service.get_template_folder_path(template_folder_id),
         current_service_id=current_service.id,
         template_folder_id=template_folder_id,
-        template_type="all"
+        template_type="all",
     )
 
 

app/notify_client/template_folder_api_client.py

@@ -42,10 +42,13 @@ class TemplateFolderAPIClient(NotifyAdminAPIClient):
             ))
 
     @cache.delete('service-{service_id}-template-folders')
-    def update_template_folder(self, service_id, template_folder_id, name):
+    def update_template_folder(self, service_id, template_folder_id, name, users_with_permission=None):
+        data = {"name": name}
+        if users_with_permission:
+            data["users_with_permission"] = users_with_permission
         self.post(
             '/service/{}/template-folder/{}'.format(service_id, template_folder_id),
-            {"name": name}
+            data
         )
 
     @cache.delete('service-{service_id}-template-folders')

app/templates/components/checkbox.html

@@ -1,4 +1,4 @@
-{% from "components/select-input.html" import select_nested %}
+{% from "components/select-input.html" import select_nested, select %}
 
 {% macro checkbox(
   field,
@@ -20,7 +20,12 @@
 
 
 {% macro checkboxes_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False) %}
-  {{ select_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False, input="checkbox") }}
+  {{ select_nested(field, child_map, hint, disable, option_hints, hide_legend, input="checkbox") }}
+{% endmacro %}
+
+
+{% macro checkboxes(field, hint=None, disable=[], option_hints={}, hide_legend=False) %}
+  {{ select(field, hint, disable, option_hints, hide_legend, input="checkbox") }}
 {% endmacro %}
 
 
@@ -39,17 +44,3 @@
     <label></label>
   </div>
 {% endmacro %}
-
-{% macro checkbox_group(
-  legend,
-  fields
-) %}
-  <fieldset class="form-group">
-    <legend class="form-label">
-      {{ legend }}
-    </legend>
-    {% for field in fields %}
-      {{ checkbox(field) }}
-    {% endfor %}
-  </fieldset>
-{% endmacro %}

app/templates/views/templates/manage-template-folder.html

@@ -1,6 +1,7 @@
 {% extends "withnav_template.html" %}
 {% from "components/folder-path.html" import folder_path, page_title_folder_path %}
 {% from "components/textbox.html" import textbox %}
+{% from "components/checkbox.html" import checkbox, checkboxes %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/form.html" import form_wrapper %}
 
@@ -23,6 +24,10 @@
 
   {% call form_wrapper(action=url_for('main.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)) %}
     {{ textbox(form.name) }}
+    {% if current_service.has_permission("edit_folder_permissions") %}
+      {{ checkboxes(form.users_with_permission) }}
+    {% endif %}
+
     {{ page_footer(
       'Save',
       delete_link=url_for(

tests/app/main/views/test_template_folders.py

@@ -417,14 +417,18 @@ def test_can_create_email_template_with_parent_folder(
 
 def test_get_manage_folder_page(
     client_request,
+    active_user_with_permissions,
     service_one,
     mock_get_template_folders,
+    mocker
 ):
     folder_id = str(uuid.uuid4())
     mock_get_template_folders.return_value = [
-        {'id': folder_id, 'name': 'folder_two', 'parent_id': None},
+        {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [
+            active_user_with_permissions.id
+        ]},
     ]
-
+    mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions])
     page = client_request.get(
         'main.manage_template_folder',
         service_id=service_one['id'],
@@ -437,10 +441,50 @@ def test_get_manage_folder_page(
     assert page.select_one('input[name=name]')['value'] == 'folder_two'
     delete_link = page.find('a', string="Delete this folder")
     expected_delete_url = "/services/{}/templates/folders/{}/delete".format(service_one['id'], folder_id)
-
     assert expected_delete_url in delete_link["href"]
 
 
+def test_get_manage_folder_viewing_permissions_for_users(
+    client_request,
+    active_user_with_permissions,
+    service_one,
+    mock_get_template_folders,
+    mocker
+):
+    folder_id = str(uuid.uuid4())
+    team_member = active_user_view_permissions(str(uuid.uuid4()))
+    team_member_2 = active_user_view_permissions(str(uuid.uuid4()))
+    service_one["permissions"] += ["edit_folder_permissions"]
+    mock_get_template_folders.return_value = [
+        {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [
+            active_user_with_permissions.id, team_member_2.id
+        ]},
+    ]
+    mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions, team_member, team_member_2])
+
+    page = client_request.get(
+        'main.manage_template_folder',
+        service_id=service_one['id'],
+        template_folder_id=folder_id,
+        _test_page_title=False,
+    )
+    assert normalize_spaces(page.select_one('title').text) == (
+        'folder_two – Templates – service one – GOV.UK Notify'
+    )
+    form_labels = page.select('legend[class=form-label]')
+    assert "Users who can see this folder:" in form_labels[0].text
+    checkboxes = page.select('input[name=users_with_permission]')
+
+    assert len(checkboxes) == 2
+    assert checkboxes[0]['value'] == team_member.id
+    assert "checked" not in checkboxes[0].attrs
+
+    assert checkboxes[1]['value'] == team_member_2.id
+    assert "checked" in checkboxes[1].attrs
+
+    assert "Test User" in page.findAll('label', {'for': 'users_with_permission-0'})[0].text
+
+
 def test_manage_folder_page_404s(client_request, service_one, mock_get_template_folders):
     client_request.get(
         'main.manage_template_folder',
@@ -463,18 +507,21 @@ def test_get_manage_folder_page_no_permissions(
     )
 
 
-def test_rename_folder(client_request, service_one, mock_get_template_folders, mocker):
+def test_rename_folder(client_request, active_user_with_permissions, service_one, mock_get_template_folders, mocker):
     mock_update = mocker.patch('app.template_folder_api_client.update_template_folder')
     folder_id = str(uuid.uuid4())
     mock_get_template_folders.return_value = [
-        {'id': folder_id, 'name': 'folder_two', 'parent_id': None},
+        {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [
+            active_user_with_permissions.id
+        ]}
     ]
+    mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions])
 
     client_request.post(
         'main.manage_template_folder',
         service_id=service_one['id'],
         template_folder_id=folder_id,
-        _data={"name": "new beautiful name"},
+        _data={"name": "new beautiful name", "users_with_permission": []},
         _expected_redirect=url_for("main.choose_template",
                                    service_id=service_one['id'],
                                    template_folder_id=folder_id,
@@ -484,7 +531,40 @@ def test_rename_folder(client_request, service_one, mock_get_template_folders, m
     mock_update.assert_called_once_with(
         service_one['id'],
         folder_id,
-        name="new beautiful name"
+        name="new beautiful name",
+        users_with_permission=[active_user_with_permissions.id]
+    )
+
+
+def test_manage_folder_users(
+    client_request, active_user_with_permissions, service_one, mock_get_template_folders, mocker
+):
+    team_member = active_user_view_permissions(str(uuid.uuid4()))
+    mock_update = mocker.patch('app.template_folder_api_client.update_template_folder')
+    folder_id = str(uuid.uuid4())
+    mock_get_template_folders.return_value = [
+        {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [
+            active_user_with_permissions.id, team_member.id
+        ]}
+    ]
+    mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions, team_member])
+
+    client_request.post(
+        'main.manage_template_folder',
+        service_id=service_one['id'],
+        template_folder_id=folder_id,
+        _data={"name": "new beautiful name", "users_with_permission": []},
+        _expected_redirect=url_for("main.choose_template",
+                                   service_id=service_one['id'],
+                                   template_folder_id=folder_id,
+                                   _external=True)
+    )
+
+    mock_update.assert_called_once_with(
+        service_one['id'],
+        folder_id,
+        name="new beautiful name",
+        users_with_permission=[active_user_with_permissions.id]
     )
 
 

tests/app/notify_client/test_template_folder_client.py

@@ -111,13 +111,13 @@ def test_update_template_folder_calls_correct_api_endpoint(mocker):
     some_service_id = uuid.uuid4()
     template_folder_id = uuid.uuid4()
     expected_url = '/service/{}/template-folder/{}'.format(some_service_id, template_folder_id)
-    data = {'name': 'foo'}
+    data = {'name': 'foo', 'users_with_permission': ['some_id']}
 
     client = TemplateFolderAPIClient()
 
     mock_post = mocker.patch('app.notify_client.template_folder_api_client.TemplateFolderAPIClient.post')
 
-    client.update_template_folder(some_service_id, template_folder_id, name='foo')
+    client.update_template_folder(some_service_id, template_folder_id, name='foo', users_with_permission=['some_id'])
 
     mock_post.assert_called_once_with(expected_url, data)
     mock_redis_delete.assert_called_once_with('service-{}-template-folders'.format(some_service_id))