Merge pull request #2630 from alphagov/nest-destination-folders

Nest destination folders

app/assets/stylesheets/components/pill.scss

@@ -57,11 +57,11 @@
     border: 2px solid $black;
     outline: 1px solid rgba($white, 0.1);
     position: relative;
-    z-index: 1000;
+    z-index: 10;
     color: $text-colour;
 
     &:focus {
-      z-index: 10;
+      z-index: 1000;
       outline: 3px solid $yellow;
     }
 

app/assets/stylesheets/components/radio-select.scss

@@ -1,59 +0,0 @@
-.radio-select {
-
-  min-height: 39px;
-
-  &-column {
-
-    display: inline-block;
-    vertical-align: top;
-
-    .multiple-choice {
-      margin-right: 5px;
-      padding-right: 10px;
-      padding-left: 54px - 10px;
-    }
-
-  }
-
-  .js-reset-button,
-  .js-category-button {
-
-    background: none;
-    text-decoration: underline;
-    color: $link-colour;
-    border: none;
-    display: inline-block;
-    vertical-align: top;
-    width: auto;
-    padding: 7px 20px 7px 10px;
-    margin-right: 5px;
-    cursor: pointer;
-
-    &:hover {
-      color: $link-hover-colour;
-    }
-
-  }
-
-  .js-reset-button-block {
-    display: block;
-    width: 100%;
-    text-align: left;
-    padding: 20px 20px $gutter 57px;
-  }
-
-  .js-enabled & {
-
-    overflow: visible;
-
-    .multiple-choice {
-      display: none;
-    }
-
-    .js-multiple-choice {
-      display: block;
-    }
-
-  }
-
-}

app/assets/stylesheets/components/radios.scss

@@ -0,0 +1,118 @@
+.radio-select {
+
+  min-height: 39px;
+
+  &-column {
+
+    display: inline-block;
+    vertical-align: top;
+
+    .multiple-choice {
+      margin-right: 5px;
+      padding-right: 10px;
+      padding-left: 54px - 10px;
+    }
+
+  }
+
+  .js-reset-button,
+  .js-category-button {
+
+    background: none;
+    text-decoration: underline;
+    color: $link-colour;
+    border: none;
+    display: inline-block;
+    vertical-align: top;
+    width: auto;
+    padding: 7px 20px 7px 10px;
+    margin-right: 5px;
+    cursor: pointer;
+
+    &:hover {
+      color: $link-hover-colour;
+    }
+
+  }
+
+  .js-reset-button-block {
+    display: block;
+    width: 100%;
+    text-align: left;
+    padding: 20px 20px $gutter 57px;
+  }
+
+  .js-enabled & {
+
+    overflow: visible;
+
+    .multiple-choice {
+      display: none;
+    }
+
+    .js-multiple-choice {
+      display: block;
+    }
+
+  }
+
+}
+
+.radios-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=radio]+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;
+    }
+
+    .block-label-hint {
+      &:after {
+        // Adds an little extra segment of line alongside the ‘current folder’
+        // hint so that it extends all the way down to the next radio
+        content: "";
+        position: absolute;
+        top: $circle-diameter + 5px;
+        left: $border-indent;
+        width: $border-thickness;
+        height: 25px;
+        background: $border-colour;
+      }
+    }
+
+  }
+
+}

app/assets/stylesheets/main.scss

@@ -46,7 +46,7 @@ $path: '/static/images/';
 @import 'components/email-message';
 @import 'components/api-key';
 @import 'components/vendor/previous-next-navigation';
-@import 'components/radio-select';
+@import 'components/radios';
 @import 'components/pill';
 @import 'components/secondary-button';
 @import 'components/show-more';

app/main/forms.py

@@ -742,6 +742,33 @@ 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
 
@@ -1199,7 +1226,7 @@ class TemplateAndFoldersSelectionForm(Form):
     """
 
     ALL_TEMPLATES_FOLDER = {
-        'name': 'All templates',
+        'name': 'Templates',
         'id': RadioFieldWithNoneOption.NONE_OPTION_VALUE,
     }
 
@@ -1207,7 +1234,6 @@ class TemplateAndFoldersSelectionForm(Form):
         self,
         all_template_folders,
         template_list,
-        current_folder_id,
         allow_adding_letter_template,
         allow_adding_copy_of_template,
         *args,
@@ -1221,13 +1247,10 @@ class TemplateAndFoldersSelectionForm(Form):
         self.op = None
         self.is_move_op = self.is_add_folder_op = self.is_add_template_op = False
 
-        if current_folder_id is None:
+        self.move_to.all_template_folders = all_template_folders
-            current_folder_id = RadioFieldWithNoneOption.NONE_OPTION_VALUE
-
         self.move_to.choices = [
             (item['id'], item['name'])
             for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders)
-            if item['id'] != current_folder_id
         ]
 
         self.add_template_by_template_type.choices = list(filter(None, [
@@ -1262,10 +1285,16 @@ class TemplateAndFoldersSelectionForm(Form):
     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=[
+    # if no default set, it is set to None, which process_data transforms to '__NONE__'
-        Optional(),
+    # this means '__NONE__' (self.ALL_TEMPLATES option) is selected when no form data has been submitted
-        required_for_ops('move-to-new-folder', 'move-to-existing-folder')
+    # set default to empty string so process_data method doesn't perform any transformation
-    ])
+    move_to = NestedRadioField(
+        'Choose a folder',
+        default='',
+        validators=[
+            required_for_ops('move-to-existing-folder'),
+            Optional()
+        ])
     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

@@ -116,13 +116,14 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
         all_template_folders=current_service.all_template_folders,
         template_list=template_list,
         template_type=template_type,
-        current_folder_id=template_folder_id,
         allow_adding_letter_template=current_service.has_permission('letter'),
         allow_adding_copy_of_template=(
             current_service.all_templates or
             len(user_api_client.get_service_ids_for_user(current_user)) > 1
         ),
     )
+    option_hints = {template_folder_id: 'current folder'}
+
     if request.method == 'POST' and templates_and_folders_form.validate_on_submit():
         if not can_manage_folders():
             abort(403)
@@ -149,6 +150,8 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
         template_type=template_type,
         search_form=SearchTemplatesForm(),
         templates_and_folders_form=templates_and_folders_form,
+        move_to_children=templates_and_folders_form.move_to.children(),
+        option_hints=option_hints
     )
 
 

app/templates/components/radios.html

@@ -14,6 +14,44 @@
   {% endcall %}
 {% 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>
+{% 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 %}
+{% 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>
@@ -37,8 +75,12 @@
   </div>
 {% endmacro %}
 
-{% macro radio(option, disable=[], option_hints={}, data_target=None) %}
+{% 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 %}
@@ -56,7 +98,14 @@
         </div>
       {% endif %}
     </label>
+    {% if caller %}
+        {{ caller() }}
+    {% endif %}
+  {% if as_list_item %}
+  </li>
+  {% else %}
   </div>
+  {% endif %}
 {% endmacro %}
 
 

app/templates/views/templates/_move_to.html

@@ -1,4 +1,4 @@
-{% from "components/radios.html" import radios %}
+{% from "components/radios.html" import radios, radios_nested %}
 {% from "components/page-footer.html" import page_footer %}
 
 <div id="sticky_template_forms" class="js-stick-at-bottom-when-scrolling">
@@ -6,7 +6,7 @@
   <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
   {% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %}
     <div id="move_to_folder_radios">
-      {{ radios(templates_and_folders_form.move_to) }}
+      {{ radios_nested(templates_and_folders_form.move_to, move_to_children, option_hints=option_hints) }}
       {{ page_footer('Move', button_name='operation', button_value='move-to-existing-folder') }}
     </div>
     <div id="move_to_new_folder_form">

tests/app/main/views/test_template_folders.py

@@ -13,6 +13,7 @@ from tests.conftest import (
     normalize_spaces,
 )
 
+ROOT_FOLDER_ID = '__NONE__'
 PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0'
 CHILD_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b'
 GRANDCHILD_FOLDER_ID = 'fafe723f-1d39-4a10-865f-e551e03d8886'
@@ -791,10 +792,10 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi
     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,
+        ROOT_FOLDER_ID, 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',
+        'Templates', 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two',
     ]
     assert set(x['value'] for x in page.find_all('button', {'name': 'operation'})) == {
         'unknown',
@@ -805,6 +806,33 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi
     }
 
 
+def test_move_to_shouldnt_select_a_folder_by_default(
+    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())
+    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},
+    ]
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+    )
+    checked_radio = page.find('input', attrs={'name': 'move_to', 'checked': 'checked'})
+    assert checked_radio is None
+
+
 def test_should_be_able_to_move_to_existing_folder(
     client_request,
     service_one,
@@ -881,6 +909,61 @@ def test_should_not_be_able_to_move_to_existing_folder_if_dont_have_permission(
     assert mock_move_to_template_folder.called is False
 
 
+def test_move_folder_form_shows_current_folder_hint_when_in_a_folder(
+    client_request,
+    service_one,
+    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},
+    ]
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+        template_folder_id=PARENT_FOLDER_ID,
+        _test_page_title=False
+    )
+
+    page.find("input", attrs={"name": "move_to", "value": PARENT_FOLDER_ID})
+
+    move_form_labels = page.find('div', id='move_to_folder_radios').find_all('label')
+
+    assert len(move_form_labels) == 3
+    assert normalize_spaces(move_form_labels[0].text) == 'Templates'
+    assert normalize_spaces(move_form_labels[1].text) == 'parent_folder current folder'
+    assert normalize_spaces(move_form_labels[2].text) == 'child_folder'
+
+
+def test_move_folder_form_does_not_show_current_folder_hint_at_the_top_level(
+    client_request,
+    service_one,
+    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},
+    ]
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+        _test_page_title=False
+    )
+
+    page.find("input", attrs={"name": "move_to", "value": PARENT_FOLDER_ID})
+
+    move_form_labels = page.find('div', id='move_to_folder_radios').find_all('label')
+
+    assert len(move_form_labels) == 3
+    assert normalize_spaces(move_form_labels[0].text) == 'Templates'
+    assert normalize_spaces(move_form_labels[1].text) == 'parent_folder'
+    assert normalize_spaces(move_form_labels[2].text) == 'child_folder'
+
+
 def test_should_be_able_to_move_a_sub_item(
     client_request,
     service_one,
@@ -902,7 +985,7 @@ def test_should_be_able_to_move_a_sub_item(
         template_folder_id=PARENT_FOLDER_ID,
         _data={
             'operation': 'move-to-existing-folder',
-            'move_to': '__NONE__',
+            'move_to': ROOT_FOLDER_ID,
             'templates_and_folders': [GRANDCHILD_FOLDER_ID],
         },
         _expected_status=302,
@@ -957,7 +1040,7 @@ def test_should_be_able_to_move_a_sub_item(
         'operation': 'add-new-template',
         'templates_and_folders': [],
         'move_to_new_folder_name': '',
-        'move_to': '__NONE__',
+        'move_to': ROOT_FOLDER_ID,
         'add_template_by_template_type': 'email',
     },
     # add a new template, but don't select anything
@@ -1005,6 +1088,7 @@ def test_no_action_if_user_fills_in_ambiguous_fields(
     ]
 
     assert [
+        ROOT_FOLDER_ID,
         FOLDER_TWO_ID,
         PARENT_FOLDER_ID,
     ] == [