Merge pull request #2481 from alphagov/label-folders

Label and filter folders depending on their contents

app/main/views/templates.py

@@ -126,7 +126,7 @@ def choose_template(service_id, template_type='all', template_folder_id=None):
         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_folders=current_service.get_template_folders(template_folder_id),
+        template_folders=current_service.get_template_folders(template_type, template_folder_id),
         templates=current_service.get_templates(template_type, template_folder_id),
         show_search_box=(len(current_service.get_templates(template_type)) > 7),
         show_template_nav=(

app/models/service.py

@@ -298,12 +298,31 @@ class Service():
     def all_template_folder_ids(self):
         return {folder['id'] for folder in self.all_template_folders}
 
-    def get_template_folders(self, parent_folder_id=None):
+    def get_template_folders(self, template_type='all', parent_folder_id=None):
         return [
             folder for folder in self.all_template_folders
-            if folder['parent_id'] == parent_folder_id
+            if (
+                folder['parent_id'] == parent_folder_id and
+                self.is_folder_visible(folder['id'], template_type)
+            )
         ]
 
+    def is_folder_visible(self, template_folder_id, template_type='all'):
+
+        if template_type == 'all':
+            return True
+
+        if self.get_templates(template_type, template_folder_id):
+            return True
+
+        if any(
+            self.is_folder_visible(child_folder['id'], template_type)
+            for child_folder in self.get_template_folders(template_type, template_folder_id)
+        ):
+            return True
+
+        return False
+
     def get_template_folder_path(self, template_folder_id):
         if template_folder_id is None:
             return []
@@ -322,7 +341,7 @@ class Service():
     def get_template_folders_and_templates(self, template_type, template_folder_id):
         return (
             self.get_templates(template_type, template_folder_id) +
-            self.get_template_folders(template_folder_id)
+            self.get_template_folders(template_type, template_folder_id)
         )
 
     def move_to_folder(self, ids_to_move, move_to):

app/templates/components/message-count-label.html

@@ -55,3 +55,26 @@
     {%- endif -%}
   {%- endif %}
 {%- endmacro %}
+
+
+{% macro folder_contents_count(number_of_folders, number_of_templates) %}
+
+  {% if number_of_folders == number_of_templates == 0 %}
+    Empty
+  {% endif %}
+
+  {% if number_of_templates == 1 %}
+      {{ number_of_templates }} template
+  {%- elif number_of_templates > 1 -%}
+      {{ number_of_templates }} templates
+  {%- endif -%}
+
+  {%- if number_of_folders and number_of_templates %}, {% endif -%}
+
+  {%- if number_of_folders == 1 -%}
+      {{ number_of_folders }} folder
+  {%- elif number_of_folders > 1 -%}
+      {{ number_of_folders }} folders
+  {% endif %}
+
+{%- endmacro %}

app/templates/views/templates/_template_list.html

@@ -1,4 +1,5 @@
 {% from "components/checkbox.html" import unlabelled_checkbox %}
+{% from "components/message-count-label.html" import folder_contents_count %}
 
 <nav id=template-list>
   {% for template_folder in template_folders %}
@@ -15,7 +16,12 @@
           {{ template_folder.name }}
         </a>
       </h2>
-      <p class="message-type">Folder containing {{ template_count }} template{% if template_count != 1 %}s{% endif %}</p>
+      <p class="message-type">
+        {{ folder_contents_count(
+          current_service.get_template_folders(template_type, template_folder.id)|length,
+          current_service.get_templates(template_type, template_folder.id)|length,
+        ) }}
+      </p>
     </div>
   {% endfor %}
   {% for template in templates %}

tests/app/main/views/test_template_folders.py

@@ -7,6 +7,24 @@ from tests.conftest import SERVICE_ONE_ID, normalize_spaces
 
 PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0'
 CHILD_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b'
+GRANDCHILD_FOLDER_ID = 'fafe723f-1d39-4a10-865f-e551e03d8886'
+
+
+def _folder(name, folder_id=None, parent=None):
+    return {
+        'name': name,
+        'id': folder_id or str(uuid.uuid4()),
+        'parent_id': parent,
+    }
+
+
+def _template(template_type, name, parent=None):
+    return {
+        'id': str(uuid.uuid4()),
+        'name': name,
+        'template_type': template_type,
+        'folder': parent,
+    }
 
 
 @pytest.mark.parametrize('parent_folder_id', [None, PARENT_FOLDER_ID])
@@ -81,46 +99,77 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
 
 
 @pytest.mark.parametrize(
-    'expected_page_title, extra_args, expected_nav_links, expected_links',
+    'expected_page_title, extra_args, expected_nav_links, expected_items',
     [
         (
             'Templates',
             {},
             ['Text message', 'Email', 'Letter'],
             [
-                'folder_one',
+                'folder_one 2 folders',
-                'folder_two',
+                'folder_two Empty',
-                'sms_template_one',
+                'sms_template_one Text message template',
-                'sms_template_two',
+                'sms_template_two Text message template',
-                'email_template_one',
+                'email_template_one Email template',
-                'email_template_two',
+                'email_template_two Email template',
-                'letter_template_one',
+                'letter_template_one Letter template',
-                'letter_template_two',
+                'letter_template_two Letter template',
             ]
         ),
         (
             'Templates',
             {'template_type': 'sms'},
             ['All', 'Email', 'Letter'],
-            ['folder_one', 'folder_two', 'sms_template_one', 'sms_template_two'],
+            [
+                'folder_one 1 folder',
+                'sms_template_one Text message template',
+                'sms_template_two Text message template',
+            ],
+        ),
+        (
+            'Templates / folder_one',
+            {'template_folder_id': PARENT_FOLDER_ID},
+            ['Text message', 'Email', 'Letter'],
+            [
+                'folder_one_one 1 template, 1 folder',
+                'folder_one_two Empty',
+            ],
         ),
         (
             'Templates / folder_one',
             {'template_type': 'sms', 'template_folder_id': PARENT_FOLDER_ID},
             ['All', 'Email', 'Letter'],
-            ['folder_one_one', 'folder_one_two'],
+            [
+                'folder_one_one 1 folder',
+            ],
+        ),
+        (
+            'Templates / folder_one',
+            {'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID},
+            ['All', 'Text message', 'Letter'],
+            [],
         ),
         (
             'Templates / folder_one / folder_one_one',
             {'template_folder_id': CHILD_FOLDER_ID},
             ['Text message', 'Email', 'Letter'],
-            [],
+            [
+                'folder_one_one_one 1 template',
+                'letter_template_nested Letter template',
+            ],
+        ),
+        (
+            'Templates / folder_one / folder_one_one / folder_one_one_one',
+            {'template_folder_id': GRANDCHILD_FOLDER_ID},
+            ['Text message', 'Email', 'Letter'],
+            [
+                'sms_template_nested Text message template',
+            ],
         ),
     ]
 )
 def test_should_show_templates_folder_page(
     client_request,
-    mock_get_service_templates,
     mock_get_template_folders,
     mock_has_no_jobs,
     service_one,
@@ -129,15 +178,28 @@ def test_should_show_templates_folder_page(
     expected_page_title,
     extra_args,
     expected_nav_links,
-    expected_links,
+    expected_items,
 ):
-
     mock_get_template_folders.return_value = [
-        {'id': str(uuid.uuid4()), 'name': 'folder_two', 'parent_id': None},
+        _folder('folder_two'),
-        {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None},
+        _folder('folder_one', PARENT_FOLDER_ID),
-        {'id': str(uuid.uuid4()), 'name': 'folder_one_two', 'parent_id': PARENT_FOLDER_ID},
+        _folder('folder_one_two', parent=PARENT_FOLDER_ID),
-        {'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID},
+        _folder('folder_one_one', CHILD_FOLDER_ID, parent=PARENT_FOLDER_ID),
+        _folder('folder_one_one_one', GRANDCHILD_FOLDER_ID, parent=CHILD_FOLDER_ID),
     ]
+    mock_get_service_templates = mocker.patch(
+        'app.service_api_client.get_service_templates',
+        return_value={'data': [
+            _template('sms', 'sms_template_one'),
+            _template('sms', 'sms_template_two'),
+            _template('email', 'email_template_one'),
+            _template('email', 'email_template_two'),
+            _template('letter', 'letter_template_one'),
+            _template('letter', 'letter_template_two'),
+            _template('letter', 'letter_template_nested', parent=CHILD_FOLDER_ID),
+            _template('sms', 'sms_template_nested', parent=GRANDCHILD_FOLDER_ID),
+        ]}
+    )
 
     service_one['permissions'] += ['letter', 'edit_folders']
 
@@ -156,12 +218,12 @@ def test_should_show_templates_folder_page(
     for index, expected_link in enumerate(expected_nav_links):
         assert links_in_page[index].text.strip() == expected_link
 
-    page_links = page.select('.message-name a')
+    page_items = page.select('.template-list-item')
 
-    assert len(page_links) == len(expected_links)
+    assert len(page_items) == len(expected_items)
 
-    for index, expected_link in enumerate(expected_links):
+    for index, expected_item in enumerate(expected_items):
-        assert page_links[index].text.strip() == expected_link
+        assert normalize_spaces(page_items[index].text) == expected_item
 
     mock_get_service_templates.assert_called_once_with(SERVICE_ONE_ID)