Merge pull request #2819 from alphagov/edit-folder-permissions

Edit user folder permissions

app/assets/stylesheets/components/checkboxes.scss

@@ -0,0 +1,44 @@
+.checkboxes-nested {
+
+  margin-bottom: 10px;
+
+  .multiple-choice {
+
+    $circle-diameter: 39px;
+    $border-thickness: 4px;
+    $border-indent: ($circle-diameter / 2) - ($border-thickness / 2);
+    $border-colour: $border-colour;
+
+    float: none;
+    position: relative;
+
+    &:before {
+      content: "";
+      position: absolute;
+      bottom: 0;
+      left: $border-indent;
+      width: $border-thickness;
+      height: 100%;
+      background: $border-colour;
+    }
+
+    label {
+      float: none;
+    }
+
+    [type=checkbox]+label::before {
+      // To overlap the grey inset line
+      background: $white;
+    }
+
+    ul {
+      // To equalise the spacing between the line and the top/bottom of
+      // the radio
+      margin-top: 5px;
+      margin-bottom: -5px;
+      padding-left: 12px;
+    }
+
+  }
+
+}

app/assets/stylesheets/main.scss

@@ -47,6 +47,7 @@ $path: '/static/images/';
 @import 'components/api-key';
 @import 'components/vendor/previous-next-navigation';
 @import 'components/radios';
+@import 'components/checkboxes';
 @import 'components/pill';
 @import 'components/secondary-button';
 @import 'components/show-more';

app/main/forms.py

@@ -255,6 +255,78 @@ def organisation_type():
     )
 
 
+class FieldWithNoneOption():
+
+    # This is a special value that is specific to our forms. This is
+    # more expicit than casting `None` to a string `'None'` which can
+    # have unexpected edge cases
+    NONE_OPTION_VALUE = '__NONE__'
+
+    # When receiving Python data, eg when instantiating the form object
+    # we want to convert that data to our special value, so that it gets
+    # recognised as being one of the valid choices
+    def process_data(self, value):
+        self.data = self.NONE_OPTION_VALUE if value is None else value
+
+    # After validation we want to convert it back to a Python `None` for
+    # use elsewhere, eg posting to the API
+    def post_validate(self, form, validation_stopped):
+        if self.data == self.NONE_OPTION_VALUE and not validation_stopped:
+            self.data = None
+
+
+class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField):
+    pass
+
+
+class NestedFieldMixin:
+    def children(self):
+        # start map with root option as a single child entry
+        child_map = {None: [option for option in self
+                            if option.data == self.NONE_OPTION_VALUE]}
+
+        # add entries for all other children
+        for option in self:
+            if option.data == self.NONE_OPTION_VALUE:
+                child_ids = [
+                    folder['id'] for folder in self.all_template_folders
+                    if folder['parent_id'] is None]
+                key = self.NONE_OPTION_VALUE
+            else:
+                child_ids = [
+                    folder['id'] for folder in self.all_template_folders
+                    if folder['parent_id'] == option.data]
+                key = option.data
+
+            child_map[key] = [option for option in self if option.data in child_ids]
+
+        return child_map
+
+
+class NestedRadioField(RadioFieldWithNoneOption, NestedFieldMixin):
+    pass
+
+
+class NestedCheckboxesField(SelectMultipleField, NestedFieldMixin):
+    NONE_OPTION_VALUE = None
+
+
+class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField):
+    pass
+
+
+class RadioFieldWithRequiredMessage(RadioField):
+    def __init__(self, *args, required_message='Not a valid choice', **kwargs):
+        self.required_message = required_message
+        super().__init__(*args, **kwargs)
+
+    def pre_validate(self, form):
+        try:
+            return super().pre_validate(form)
+        except ValueError:
+            raise ValueError(self.required_message)
+
+
 class StripWhitespaceForm(Form):
     class Meta:
         def bind_field(self, form, unbound_field, options):
@@ -346,6 +418,15 @@ PermissionsAbstract = type("PermissionsAbstract", (StripWhitespaceForm,), {
 
 
 class PermissionsForm(PermissionsAbstract):
+    def __init__(self, all_template_folders=None, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if all_template_folders is not None:
+            self.folder_permissions.all_template_folders = all_template_folders
+            self.folder_permissions.choices = [
+                (item['id'], item['name']) for item in ([{'name': 'Templates', 'id': None}] + all_template_folders)
+            ]
+
+    folder_permissions = NestedCheckboxesField('Folders this team member can see')
 
     login_authentication = RadioField(
         'Sign in using',
@@ -365,8 +446,9 @@ class PermissionsForm(PermissionsAbstract):
         return (getattr(self, permission) for permission, _ in permissions)
 
     @classmethod
-    def from_user(cls, user, service_id):
+    def from_user(cls, user, service_id, **kwargs):
         return cls(
+            **kwargs,
             **{
                 role: user.has_permission_for_service(service_id, role)
                 for role in roles.keys()
@@ -813,73 +895,6 @@ class ServiceSwitchChannelForm(ServiceOnOffSettingForm):
         super().__init__(name, *args, **kwargs)
 
 
-class FieldWithNoneOption():
-
-    # This is a special value that is specific to our forms. This is
-    # more expicit than casting `None` to a string `'None'` which can
-    # have unexpected edge cases
-    NONE_OPTION_VALUE = '__NONE__'
-
-    # When receiving Python data, eg when instantiating the form object
-    # we want to convert that data to our special value, so that it gets
-    # recognised as being one of the valid choices
-    def process_data(self, value):
-        self.data = self.NONE_OPTION_VALUE if value is None else value
-
-    # After validation we want to convert it back to a Python `None` for
-    # use elsewhere, eg posting to the API
-    def post_validate(self, form, validation_stopped):
-        if self.data == self.NONE_OPTION_VALUE and not validation_stopped:
-            self.data = None
-
-
-class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField):
-    pass
-
-
-class NestedRadioField(RadioFieldWithNoneOption):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-    def children(self):
-        # start map with root option as a single child entry
-        child_map = {None: [option for option in self
-                            if option.data == self.NONE_OPTION_VALUE]}
-
-        # add entries for all other children
-        for option in self:
-            if option.data == self.NONE_OPTION_VALUE:
-                child_ids = [
-                    folder['id'] for folder in self.all_template_folders
-                    if folder['parent_id'] is None]
-                key = self.NONE_OPTION_VALUE
-            else:
-                child_ids = [
-                    folder['id'] for folder in self.all_template_folders
-                    if folder['parent_id'] == option.data]
-                key = option.data
-
-            child_map[key] = [option for option in self if option.data in child_ids]
-
-        return child_map
-
-
-class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField):
-    pass
-
-
-class RadioFieldWithRequiredMessage(RadioField):
-    def __init__(self, *args, required_message='Not a valid choice', **kwargs):
-        self.required_message = required_message
-        super().__init__(*args, **kwargs)
-
-    def pre_validate(self, form):
-        try:
-            return super().pre_validate(form)
-        except ValueError:
-            raise ValueError(self.required_message)
-
-
 class ServiceSetEmailBranding(StripWhitespaceForm):
 
     branding_style = RadioFieldWithNoneOption(

app/main/views/manage_users.py

@@ -84,12 +84,24 @@ def edit_user_permissions(service_id, user_id):
     if user.mobile_number:
         mobile_number = redact_mobile_number(user.mobile_number, " ")
 
-    form = PermissionsForm.from_user(user, service_id)
+    form = PermissionsForm.from_user(
+        user,
+        service_id,
+        folder_permissions=[
+            f['id'] for f in current_service.all_template_folders
+            if user_id in f.get('users_with_permission', [])
+        ],
+        all_template_folders=current_service.all_template_folders
+    )
 
     if form.validate_on_submit():
         user_api_client.set_user_permissions(
             user_id, service_id,
             permissions=form.permissions,
+            folder_permissions=(
+                form.folder_permissions.data
+                if current_service.has_permission('edit_folder_permissions') else None
+            ),
         )
         if service_has_email_auth:
             user_api_client.update_user_attribute(user_id, auth_type=form.login_authentication.data)

app/notify_client/user_api_client.py

@@ -161,13 +161,17 @@ class UserApiClient(NotifyAdminAPIClient):
         resp = self.post('/organisations/{}/users/{}'.format(org_id, user_id), data={})
         return User(resp['data'], max_failed_login_count=self.max_failed_login_count)
 
+    @cache.delete('service-{service_id}-template-folders')
     @cache.delete('user-{user_id}')
-    def set_user_permissions(self, user_id, service_id, permissions):
+    def set_user_permissions(self, user_id, service_id, permissions, folder_permissions=None):
         # permissions passed in are the combined admin roles, not db permissions
         data = {
-            'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)]
+            'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)],
         }
 
+        if folder_permissions is not None:
+            data['folder_permissions'] = folder_permissions
+
         endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id)
         self.post(endpoint, data=data)
 

app/templates/components/checkbox.html

@@ -1,3 +1,5 @@
+{% from "components/select-input.html" import select_nested %}
+
 {% macro checkbox(
   field,
   hint=False,
@@ -17,6 +19,11 @@
 {% endmacro %}
 
 
+{% 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") }}
+{% endmacro %}
+
+
 {% macro checkbox_input(id, name, data=None, value="y") %}
   <input
     id="{{ id }}" name="{{ name }}" type="checkbox" value="{{ value }}"

app/templates/components/radios.html

@@ -1,111 +1,22 @@
-{% macro radios(
-  field,
-  hint=None,
-  disable=[],
-  option_hints={},
-  hide_legend=False
-) %}
-  {% call radios_wrapper(
-    field, hint, disable, option_hints, hide_legend
-  ) %}
-    {% for option in field %}
-      {{ radio(option, disable, option_hints) }}
-    {% endfor %}
-  {% endcall %}
+{% from "components/select-input.html" import select, select_list, select_nested, select_wrapper, select_input %}
+
+{% macro radios(field, hint=None, disable=[], option_hints={}, hide_legend=False) %}
+  {{ select(field, hint, disable, option_hints, hide_legend, input="radio") }}
 {% endmacro %}
 
 
-{% macro radio_list(
-    options,
-    child_map,
-    disable=[],
-    option_hints={}
-) %}
-    <ul>
-    {% for option in options %}
-      {% if child_map[option.data] %}
-        {% call radio(option, disable, option_hints, as_list_item=True) %}
-          {{ radio_list(child_map[option.data], child_map, disable, option_hints) }}
-        {% endcall %}
-      {% else %}
-          {{ radio(option, disable, option_hints, as_list_item=True) }}
-      {% endif %}
-    {% endfor %}
-    </ul>
+{% macro radio_list(options, child_map, disable=[], option_hints={}) %}
+  {{ select_list(options, child_map, disable, option_hints, input="radio") }}
 {% endmacro %}
 
 
-{% macro radios_nested(
-  field,
-  child_map,
-  hint=None,
-  disable=[],
-  option_hints={},
-  hide_legend=False
-) %}
-  {% call radios_wrapper(
-    field, hint, disable, option_hints, hide_legend
-  ) %}
-    <div class="radios-nested">
-      {{ radio_list(child_map[None], child_map, disable, option_hints) }}
-    </div>
-  {% endcall %}
+{% macro radios_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False) %}
+  {{ select_nested(field, child_map, hint, disable, option_hints, hide_legend, input="radio") }}
 {% endmacro %}
 
-{% macro radios_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False) %}
-  <div class="form-group {% if field.errors %} form-group-error{% endif %}">
-    <fieldset>
-      <legend class="{{ 'form-label' if not hide_legend else '' }}">
-        {% if hide_legend %}<span class="visually-hidden">{% endif %}
-          {{ field.label.text|safe }}
-        {% if hide_legend %}</span>{% endif %}
-        {% if hint %}
-          <span class="form-hint">
-            {{ hint }}
-          </span>
-        {% endif %}
-        {% if field.errors %}
-          <span class="error-message" data-module="track-error" data-error-type="{{ field.errors[0] }}" data-error-label="{{ field.name }}">
-            {{ field.errors[0] }}
-          </span>
-        {% endif %}
-      </legend>
-      {{ caller() }}
-    </fieldset>
-  </div>
-{% endmacro %}
 
 {% macro radio(option, disable=[], option_hints={}, data_target=None, as_list_item=False) %}
-  {% if as_list_item %}
-  <li class="multiple-choice" {% if data_target %}data-target="{{ data_target }}"{% endif %}>
-  {% else %}
-  <div class="multiple-choice" {% if data_target %}data-target="{{ data_target }}"{% endif %}>
-  {% endif %}
-    <input
-      id="{{ option.id }}" name="{{ option.name }}" type="radio" value="{{ option.data }}"
-      {% if option.data in disable %}
-        disabled
-      {% endif %}
-      {% if option.checked %}
-        checked
-      {% endif %}
-    >
-    <label class="block-label" for="{{ option.id }}">
-      {{ option.label.text }}
-      {% if option_hints[option.data] %}
-        <div class="block-label-hint">
-          {{ option_hints[option.data] }}
-        </div>
-      {% endif %}
-    </label>
-    {% if caller %}
-        {{ caller() }}
-    {% endif %}
-  {% if as_list_item %}
-  </li>
-  {% else %}
-  </div>
-  {% endif %}
+  {{ select_input(option, disable, option_hints, data_target, as_list_item, input="radio") }}
 {% endmacro %}
 
 

app/templates/components/select-input.html

@@ -0,0 +1,92 @@
+{% macro select(field, hint=None, disable=[], option_hints={}, hide_legend=False, input="radio") %}
+  {% call select_wrapper(
+    field, hint, disable, option_hints, hide_legend
+  ) %}
+    {% for option in field %}
+      {{ select_input(option, disable, option_hints, input=input) }}
+    {% endfor %}
+  {% endcall %}
+{% endmacro %}
+
+
+{% macro select_list(options, child_map, disable=[], option_hints={}, input="radio") %}
+    <ul>
+    {% for option in options %}
+      {% if child_map[option.data] %}
+        {% call select_input(option, disable, option_hints, as_list_item=True, input=input) %}
+          {{ select_list(child_map[option.data], child_map, disable, option_hints, input=input) }}
+        {% endcall %}
+      {% else %}
+          {{ select_input(option, disable, option_hints, as_list_item=True, input=input) }}
+      {% endif %}
+    {% endfor %}
+    </ul>
+{% endmacro %}
+
+
+{% macro select_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False, input="radio") %}
+  {% call select_wrapper(
+    field, hint, disable, option_hints, hide_legend
+  ) %}
+  <div class="{{ "radios" if input == "radio" else "checkboxes" }}-nested">
+      {{ select_list(child_map[None], child_map, disable, option_hints, input=input) }}
+    </div>
+  {% endcall %}
+{% endmacro %}
+
+
+{% macro select_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False) %}
+  <div class="form-group {% if field.errors %} form-group-error{% endif %}">
+    <fieldset>
+      <legend class="{{ 'form-label' if not hide_legend else '' }}">
+        {% if hide_legend %}<span class="visually-hidden">{% endif %}
+          {{ field.label.text|safe }}
+        {% if hide_legend %}</span>{% endif %}
+        {% if hint %}
+          <span class="form-hint">
+            {{ hint }}
+          </span>
+        {% endif %}
+        {% if field.errors %}
+          <span class="error-message" data-module="track-error" data-error-type="{{ field.errors[0] }}" data-error-label="{{ field.name }}">
+            {{ field.errors[0] }}
+          </span>
+        {% endif %}
+      </legend>
+      {{ caller() }}
+    </fieldset>
+  </div>
+{% endmacro %}
+
+{% macro select_input(option, disable=[], option_hints={}, data_target=None, as_list_item=False, input="radio") %}
+  {% if as_list_item %}
+  <li class="multiple-choice" {% if data_target %}data-target="{{ data_target }}"{% endif %}>
+  {% else %}
+  <div class="multiple-choice" {% if data_target %}data-target="{{ data_target }}"{% endif %}>
+  {% endif %}
+    <input
+      id="{{ option.id }}" name="{{ option.name }}" type="{{ input }}" value="{{ option.data }}"
+      {% if option.data in disable %}
+        disabled
+      {% endif %}
+      {% if option.checked %}
+        checked
+      {% endif %}
+    >
+    <label class="block-label" for="{{ option.id }}">
+      {{ option.label.text }}
+      {% if option_hints[option.data] %}
+        <div class="block-label-hint">
+          {{ option_hints[option.data] }}
+        </div>
+      {% endif %}
+    </label>
+    {% if caller %}
+        {{ caller() }}
+    {% endif %}
+  {% if as_list_item %}
+  </li>
+  {% else %}
+  </div>
+  {% endif %}
+{% endmacro %}

app/templates/views/manage-users/permissions.html

@@ -1,5 +1,5 @@
-{% from "components/checkbox.html" import checkbox %}
-{% from "components/radios.html" import radio, radios, radios_wrapper, conditional_radio_panel %}
+{% from "components/checkbox.html" import checkbox, checkboxes_nested %}
+{% from "components/radios.html" import radio, radios, conditional_radio_panel %}
 
 <fieldset class="form-group">
   <legend class="form-label">
@@ -14,6 +14,10 @@
   All team members can see sent messages.
 </p>
 
+{% if current_service.has_permission("edit_folder_permissions") and form.folder_permissions.all_template_folders %}
+  {{ checkboxes_nested(form.folder_permissions, form.folder_permissions.children()) }}
+{% endif %}
+
 {% if service_has_email_auth %}
   {% if not mobile_number %}
     {{ radios(

app/templates/views/service-settings/contact_link.html

@@ -1,7 +1,8 @@
 {% extends "withnav_template.html" %}
 {% from "components/textbox.html" import textbox %}
 {% from "components/page-footer.html" import page_footer %}
-{% from "components/radios.html" import radio, radios_wrapper %}
+{% from "components/radios.html" import radio %}
+{% from "components/select-input.html" import select_wrapper %}
 {% from "components/form.html" import form_wrapper %}
 
 {% block service_page_title %}
@@ -21,7 +22,7 @@
       </p>
       {% call form_wrapper() %}
 
-        {% call radios_wrapper(form.contact_details_type, hide_legend=true) %}
+        {% call select_wrapper(form.contact_details_type, hide_legend=true) %}
           {% for option in form.contact_details_type %}
             {% set data_target = option.data.replace('_', '-') ~ "-type" %}
 

tests/app/main/views/test_manage_users.py

@@ -228,6 +228,7 @@ def test_service_with_no_email_auth_hides_auth_type_options(
     auth_options_hidden,
     service_one,
     mock_get_users_by_service,
+    mock_get_template_folders
 ):
     if service_has_email_auth:
         service_one['permissions'].append('email_auth')
@@ -249,6 +250,7 @@ def test_service_with_no_email_auth_hides_auth_type_options(
 def test_service_without_caseworking_doesnt_show_admin_vs_caseworker(
     client_request,
     mock_get_users_by_service,
+    mock_get_template_folders,
     endpoint,
     service_has_caseworking,
     extra_args,
@@ -304,6 +306,7 @@ def test_manage_users_page_shows_member_auth_type_if_service_has_email_auth_acti
 def test_user_with_no_mobile_number_cant_be_set_to_sms_auth(
     client_request,
     mock_get_users_by_service,
+    mock_get_template_folders,
     user,
     sms_option_disabled,
     expected_label,
@@ -353,6 +356,7 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth(
 def test_should_show_page_for_one_user(
     client_request,
     mock_get_users_by_service,
+    mock_get_template_folders,
     endpoint,
     extra_args,
     expected_checkboxes,
@@ -419,6 +423,7 @@ def test_edit_user_permissions(
     mock_get_users_by_service,
     mock_get_invites_for_service,
     mock_set_user_permissions,
+    mock_get_template_folders,
     fake_uuid,
     submitted_permissions,
     permissions_sent_to_api,
@@ -442,6 +447,45 @@ def test_edit_user_permissions(
         fake_uuid,
         SERVICE_ONE_ID,
         permissions=permissions_sent_to_api,
+        folder_permissions=None
+    )
+
+
+def test_edit_user_folder_permissions(
+    client_request,
+    mocker,
+    service_one,
+    mock_get_users_by_service,
+    mock_get_invites_for_service,
+    mock_set_user_permissions,
+    mock_get_template_folders,
+    fake_uuid,
+):
+    service_one['permissions'] = ['edit_folder_permissions']
+    mock_get_template_folders.return_value = [
+        {'id': 'folder-id-1', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []},
+        {'id': 'folder-id-2', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []},
+        {'id': 'folder-id-3', 'name': 'folder_one', 'parent_id': 'folder-id-1', 'users_with_permission': []},
+    ]
+    client_request.post(
+        'main.edit_user_permissions',
+        service_id=SERVICE_ONE_ID,
+        user_id=fake_uuid,
+        _data=dict(
+            folder_permissions=['folder-id-1', 'folder-id-3']
+        ),
+        _expected_status=302,
+        _expected_redirect=url_for(
+            'main.manage_users',
+            service_id=SERVICE_ONE_ID,
+            _external=True,
+        ),
+    )
+    mock_set_user_permissions.assert_called_with(
+        fake_uuid,
+        SERVICE_ONE_ID,
+        permissions=set(),
+        folder_permissions=['folder-id-1', 'folder-id-3']
     )
 
 
@@ -474,7 +518,8 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service(
     mock_set_user_permissions,
     mock_update_user_attribute,
     service_one,
-    auth_type
+    auth_type,
+    mock_get_template_folders
 ):
     service_one['permissions'].append('email_auth')
 
@@ -502,7 +547,8 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service(
             'manage_templates',
             'manage_service',
             'manage_api_keys',
-        }
+        },
+        folder_permissions=None
     )
     mock_update_user_attribute.assert_called_with(
         str(active_user_with_permissions.id),
@@ -1015,6 +1061,7 @@ def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_l
     client_request,
     active_user_with_permissions,
     mock_get_users_by_service,
+    mock_get_template_folders,
     service_one,
     mocker
 ):