darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-05-28 01:50:12 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2531 from alphagov/search-within-folders

Browse Source 
Make template search work across folders
This commit is contained in:
Chris Hill-Scott
2018-11-26 16:30:54 +00:00
committed by GitHub
parent 6e56a4d517 7d20846d94
commit d5c4c7590c
11 changed files with 371 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
app/assets/javascripts/liveSearch.js
View File

@@ -9,7 +9,12 @@
$targets.each(function() {
let content = $(this).text();
let content = $('.live-search-relevant', this).text() || $(this).text();
if (query == '') {
$(this).css('display', '');
return;
}
$(this).toggle(
normalize(content).indexOf(normalize(query)) > -1

47
app/assets/stylesheets/components/message.scss
View File

@@ -5,9 +5,31 @@
margin: 0;
a {
display: block;
display: inline;
margin-bottom: -$gutter;
padding-bottom: $gutter;
&:focus {
// Use box shadow instead of outline to avoid buggy outline
// rendering in Firefox
outline: none;
box-shadow: 0 0 0 3px $yellow;
@include ie-lte(8) {
// Box shadow is not supported in IE8
outline: 3px solid $yellow;
}
}
}
&-separator {
display: inline-block;
vertical-align: top;
color: $secondary-text-colour;
padding: 0 4px 0 5px;
font-weight: normal;
}
}
@@ -40,11 +62,32 @@
}
&-hidden-by-default {
display: none;
}
&-without-ancestors {
.message-name {
a {
display: block;
&:first-child {
display: block;
}
}
}
}
}
&-folder {
a {
a:first-child {
display: inline-block;
text-indent: 40px;
background-image: file-url('folder-blue-bold.svg');
background-repeat: no-repeat;

22
app/main/forms.py
View File

@@ -1143,8 +1143,8 @@ class TemplateAndFoldersSelectionForm(Form):
def __init__(
self,
service,
template_type,
all_template_folders,
template_list,
current_folder_id,
*args,
**kwargs
@@ -1152,20 +1152,12 @@ class TemplateAndFoldersSelectionForm(Form):
super().__init__(*args, **kwargs)
self.templates_and_folders.choices = self.ids_and_names(
service.get_template_folders_and_templates(template_type, current_folder_id)
)
self.templates_and_folders.choices = template_list.as_id_and_name
self.move_to.choices = self.ids_and_names(
[self.ALL_TEMPLATES_FOLDER] + service.all_template_folders,
exclude=current_folder_id,
)
@staticmethod
def ids_and_names(items, exclude=None):
return [
(item['id'], item['name']) for item in items
if item['id'] != str(exclude)
self.move_to.choices = [
(item['id'], item['name'])
for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders)
if item['id'] != str(current_folder_id)
]
templates_and_folders = MultiCheckboxField('Choose templates or folders')

10
app/main/views/templates.py
View File

@@ -29,6 +29,7 @@ from app.main.forms import (
)
from app.main.views.send import get_example_csv_rows, get_sender_details
from app.models.service import Service
from app.models.template_list import TemplateList
from app.template_previews import TemplatePreview, get_page_count_for_letter
from app.utils import (
email_or_sms_not_enabled,
@@ -108,8 +109,11 @@ def start_tour(service_id, template_id):
@user_has_permissions()
def choose_template(service_id, template_type='all', template_folder_id=None):
template_list = TemplateList(current_service, template_type, template_folder_id)
templates_and_folders_form = TemplateAndFoldersSelectionForm(
service=current_service,
all_template_folders=current_service.all_template_folders,
template_list=template_list,
template_type=template_type,
current_folder_id=template_folder_id,
)
@@ -126,9 +130,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_folder_has_contents=current_service.get_template_folders_and_templates('all', 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),
template_list=template_list,
show_search_box=current_service.count_of_templates_and_folders > 7,
show_template_nav=(
current_service.has_multiple_template_types

124
app/models/template_list.py Normal file
View File

@@ -0,0 +1,124 @@
class TemplateList():
def __init__(
self,
service,
template_type='all',
template_folder_id=None,
):
self.service = service
self.template_type = template_type
self.template_folder_id = template_folder_id
def __iter__(self):
for item in self.get_templates_and_folders(
self.template_type, self.template_folder_id, ancestors=[]
):
yield item
def get_templates_and_folders(self, template_type, template_folder_id, ancestors):
for item in self.service.get_template_folders(
template_type, template_folder_id
):
yield TemplateListFolder(
item,
folders=self.service.get_template_folders(
self.template_type, item['id']
),
templates=self.service.get_templates(
self.template_type, item['id']
),
ancestors=ancestors,
)
for sub_item in self.get_templates_and_folders(
template_type, item['id'], ancestors + [item]
):
yield sub_item
for item in self.service.get_templates(
self.template_type, template_folder_id
):
yield TemplateListTemplate(
item,
ancestors=ancestors,
)
@property
def as_id_and_name(self):
return [(item.id, item.name) for item in self]
@property
def templates_to_show(self):
return any(self)
@property
def folder_is_empty(self):
return not any(self.get_templates_and_folders(
'all', self.template_folder_id, []
))
class TemplateListItem():
def __init__(
self,
template_or_folder,
ancestors,
):
self.id = template_or_folder['id']
self.name = template_or_folder['name']
self.ancestors = ancestors
class TemplateListTemplate(TemplateListItem):
is_folder = False
def __init__(
self,
template,
ancestors,
):
super().__init__(template, ancestors)
self.hint = {
'email': 'Email template',
'sms': 'Text message template',
'letter': 'Letter template',
}.get(template['template_type'])
class TemplateListFolder(TemplateListItem):
is_folder = True
def __init__(
self,
folder,
templates,
folders,
ancestors,
):
super().__init__(folder, ancestors)
self.number_of_templates = len(templates)
self.number_of_folders = len(folders)
@property
def _hint_parts(self):
if self.number_of_folders == self.number_of_templates == 0:
yield 'Empty'
if self.number_of_templates == 1:
yield '1 template'
elif self.number_of_templates > 1:
yield '{} templates'.format(self.number_of_templates)
if self.number_of_folders == 1:
yield '1 folder'
elif self.number_of_folders > 1:
yield '{} folders'.format(self.number_of_folders)
@property
def hint(self):
return ', '.join(self._hint_parts)

23
app/templates/components/message-count-label.html
View File

@@ -55,26 +55,3 @@
{%- 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 %}

2
app/templates/views/templates/_move_to.html
View File

@@ -1,7 +1,7 @@
{% from "components/radios.html" import radios %}
{% from "components/page-footer.html" import page_footer %}
{% if templates_and_folders_form.move_to.choices and (templates or template_folders) %}
{% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %}
{{ radios(templates_and_folders_form.move_to) }}
{{ page_footer('Move selected', button_name='operation', button_value='move') }}
{% endif %}

57
app/templates/views/templates/_template_list.html
View File

@@ -1,52 +1,43 @@
{% from "components/checkbox.html" import unlabelled_checkbox %}
{% from "components/message-count-label.html" import folder_contents_count, message_count_label %}
{% if service_has_templates_or_folders and not templates and not template_folders %}
{% if not template_list.templates_to_show %}
<p class="template-list-empty">
{% if template_folder_has_contents %}
There are no {{ message_count_label(1, template_type, suffix='') }} templates in this folder
{% else %}
{% if template_list.folder_is_empty %}
This folder is empty
{% else %}
There are no {{ message_count_label(1, template_type, suffix='') }} templates in this folder
{% endif %}
</p>
{% else %}
<nav id=template-list>
{% for template_folder in template_folders %}
<div class="template-list-item {% if can_manage_folders %}template-list-item-with-checkbox{% endif %}">
{% 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 %}
{{ unlabelled_checkbox(
id='templates-or-folder-{}'.format(template_folder.id),
id='templates-or-folder-{}'.format(item.id),
name='templates_and_folders',
value=template_folder.id,
value=item.id,
) }}
{% endif %}
<h2 class="message-name template-list-folder">
<a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=template_folder.id) }}">
{{ template_folder.name }}
</a>
<h2 class="message-name {% if item.is_folder or (item.ancestors and not item.is_folder) %}template-list-folder{% endif %}">
{% for ancestor in item.ancestors %}
<a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=ancestor.id) }}">
{{ ancestor.name }}
</a> <span class="message-name-separator">/</span>
{% endfor %}
{% if item.is_folder %}
<a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=item.id) }}">
<span class="live-search-relevant">{{ item.name }}</span>
</a>
{% else %}
<a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.id) }}">
<span class="live-search-relevant">{{ item.name }}</span>
</a>
{% endif %}
</h2>
<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 %}
<div class="template-list-item {% if can_manage_folders %}template-list-item-with-checkbox{% endif %}">
{% if can_manage_folders %}
{{ unlabelled_checkbox(
id='templates-or-folder-{}'.format(template.id),
name='templates_and_folders',
value=template.id,
) }}
{% endif %}
<h2 class="message-name">
<a href="{{ url_for('.view_template', service_id=current_service.id, template_id=template.id) }}">{{ template.name }}</a>
</h2>
<p class="message-type">
{{ message_count_label(1, template.template_type, suffix='')|capitalize }} template
{{ item.hint }}
</p>
</div>
{% endfor %}

24
app/templates/views/templates/choose.html
View File

@@ -77,29 +77,11 @@
{% if can_manage_folders %}
<form method="post">
{% with
templates=templates,
template_folders=template_folders,
service_has_templates_or_folders=(current_service.has_templates or current_service.has_folders),
template_type=template_type,
template_folder_has_contents=template_folder_has_contents
%}
{% include 'views/templates/_template_list.html' %}
{% endwith %}
{% with templates=templates, template_folders=template_folders, templates_and_folders_form=templates_and_folders_form %}
{% include 'views/templates/_move_to.html' %}
{% endwith %}
{% include 'views/templates/_template_list.html' %}
{% include 'views/templates/_move_to.html' %}
</form>
{% else %}
{% with
templates=templates,
template_folders=template_folders,
service_has_templates_or_folders=(current_service.has_templates or current_service.has_folders),
template_type=template_type,
template_folder_has_contents=template_folder_has_contents
%}
{% include 'views/templates/_template_list.html' %}
{% endwith %}
{% include 'views/templates/_template_list.html' %}
{% endif %}
{% endif %}

124
tests/app/main/views/test_template_folders.py
View File

@@ -107,6 +107,8 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
'extra_args,'
'expected_nav_links,'
'expected_items, '
'expected_displayed_items, '
'expected_searchable_text, '
'expected_empty_message '
),
[
@@ -118,6 +120,11 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
['Text message', 'Email', 'Letter'],
[
'folder_one 2 folders',
'folder_one / folder_one_one 1 template, 1 folder',
'folder_one / folder_one_one / folder_one_one_one 1 template',
'folder_one / folder_one_one / folder_one_one_one / sms_template_nested Text message template',
'folder_one / folder_one_one / letter_template_nested Letter template',
'folder_one / folder_one_two Empty',
'folder_two Empty',
'sms_template_one Text message template',
'sms_template_two Text message template',
@@ -126,6 +133,31 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
'letter_template_one Letter template',
'letter_template_two Letter template',
],
[
'folder_one 2 folders',
'folder_two Empty',
'sms_template_one Text message template',
'sms_template_two Text message template',
'email_template_one Email template',
'email_template_two Email template',
'letter_template_one Letter template',
'letter_template_two Letter template',
],
[
'folder_one',
'folder_one_one',
'folder_one_one_one',
'sms_template_nested',
'letter_template_nested',
'folder_one_two',
'folder_two',
'sms_template_one',
'sms_template_two',
'email_template_one',
'email_template_two',
'letter_template_one',
'letter_template_two',
],
None,
),
(
@@ -136,9 +168,25 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
['All', 'Email', 'Letter'],
[
'folder_one 1 folder',
'folder_one / folder_one_one 1 folder',
'folder_one / folder_one_one / folder_one_one_one 1 template',
'folder_one / folder_one_one / folder_one_one_one / sms_template_nested Text message template',
'sms_template_one Text message template',
'sms_template_two Text message template',
],
[
'folder_one 1 folder',
'sms_template_one Text message template',
'sms_template_two Text message template',
],
[
'folder_one',
'folder_one_one',
'folder_one_one_one',
'sms_template_nested',
'sms_template_one',
'sms_template_two',
],
None,
),
(
@@ -149,8 +197,22 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
['Text message', 'Email', 'Letter'],
[
'folder_one_one 1 template, 1 folder',
'folder_one_one / folder_one_one_one 1 template',
'folder_one_one / folder_one_one_one / sms_template_nested Text message template',
'folder_one_one / letter_template_nested Letter template',
'folder_one_two Empty',
],
[
'folder_one_one 1 template, 1 folder',
'folder_one_two Empty',
],
[
'folder_one_one',
'folder_one_one_one',
'sms_template_nested',
'letter_template_nested',
'folder_one_two',
],
None,
),
(
@@ -161,6 +223,16 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
['All', 'Email', 'Letter'],
[
'folder_one_one 1 folder',
'folder_one_one / folder_one_one_one 1 template',
'folder_one_one / folder_one_one_one / sms_template_nested Text message template',
],
[
'folder_one_one 1 folder',
],
[
'folder_one_one',
'folder_one_one_one',
'sms_template_nested',
],
None,
),
@@ -171,6 +243,8 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
{'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID},
['All', 'Text message', 'Letter'],
[],
[],
[],
'There are no email templates in this folder',
),
(
@@ -181,8 +255,18 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
['Text message', 'Email', 'Letter'],
[
'folder_one_one_one 1 template',
'folder_one_one_one / sms_template_nested Text message template',
'letter_template_nested Letter template',
],
[
'folder_one_one_one 1 template',
'letter_template_nested Letter template',
],
[
'folder_one_one_one',
'sms_template_nested',
'letter_template_nested',
],
None,
),
(
@@ -194,6 +278,12 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
[
'sms_template_nested Text message template',
],
[
'sms_template_nested Text message template',
],
[
'sms_template_nested',
],
None,
),
(
@@ -203,6 +293,8 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
{'template_folder_id': FOLDER_TWO_ID},
['Text message', 'Email', 'Letter'],
[],
[],
[],
'This folder is empty',
),
(
@@ -212,6 +304,8 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
{'template_folder_id': FOLDER_TWO_ID, 'template_type': 'sms'},
['All', 'Email', 'Letter'],
[],
[],
[],
'This folder is empty',
),
]
@@ -229,6 +323,8 @@ def test_should_show_templates_folder_page(
extra_args,
expected_nav_links,
expected_items,
expected_displayed_items,
expected_searchable_text,
expected_empty_message,
):
mock_get_template_folders.return_value = [
@@ -281,12 +377,32 @@ 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_items = page.select('.template-list-item')
assert len(page_items) == len(expected_items)
all_page_items = page.select('.template-list-item')
checkboxes = page.select('input[name=templates_and_folders]')
unique_checkbox_values = set(item['value'] for item in checkboxes)
assert len(all_page_items) == len(expected_items)
assert len(checkboxes) == len(expected_items)
assert len(unique_checkbox_values) == len(expected_items)
for index, expected_item in enumerate(expected_items):
assert normalize_spaces(page_items[index].text) == expected_item
assert normalize_spaces(all_page_items[index].text) == expected_item
displayed_page_items = page.find_all(lambda tag: (
tag.has_attr('class')
and 'template-list-item' in tag['class']
and 'template-list-item-hidden-by-default' not in tag['class']
))
assert len(displayed_page_items) == len(expected_displayed_items)
for index, expected_item in enumerate(expected_displayed_items):
assert '/' not in expected_item # Yo dawg I heard you like tests…
assert normalize_spaces(displayed_page_items[index].text) == expected_item
all_searchable_text = page.select('#template-list .template-list-item .live-search-relevant')
assert len(all_searchable_text) == len(expected_searchable_text)
for index, expected_item in enumerate(expected_searchable_text):
assert normalize_spaces(all_searchable_text[index].text) == expected_item
if expected_empty_message:
assert normalize_spaces(page.select_one('.template-list-empty').text) == (

36
tests/app/main/views/test_templates.py
View File

@@ -365,6 +365,40 @@ def test_should_post_move_to_api(
)
def test_should_be_able_to_move_a_sub_item(
client_request,
service_one,
fake_uuid,
mock_get_service_templates,
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},
{'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID},
{'id': GRANDCHILD_FOLDER_ID, 'name': 'folder_one_one_one', 'parent_id': CHILD_FOLDER_ID},
]
client_request.post(
'main.choose_template',
service_id=SERVICE_ONE_ID,
template_folder_id=PARENT_FOLDER_ID,
_data={
'operation': 'move',
'move_to': 'None',
'templates_and_folders': [GRANDCHILD_FOLDER_ID],
},
_expected_status=302,
)
mock_move_to_template_folder.assert_called_once_with(
service_id=SERVICE_ONE_ID,
folder_id=None,
folder_ids={GRANDCHILD_FOLDER_ID},
template_ids=set(),
)
@pytest.mark.parametrize('thing_to_move', [
PARENT_FOLDER_ID, # Can’t move a folder inside itself
CHILD_FOLDER_ID, # Can’t move a folder which doesn’t belong to the service
@@ -470,7 +504,7 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
count_of_templates = count_of_templates_and_folders - count_of_folders
assert len(page.select('.live-search')) == 1
assert count_of_folders == 1
assert count_of_folders == 4
assert count_of_templates == 4