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

Add Service.get_templates method with filters by type and folder

Browse Source 
With the addition of template folders we need to filter templates
based on a combination of type and parent folder ID.

This replaces the existing `templates_by_type` method with
`get_templates`, which supports both type and parent folder filters,
avoiding a need to create specific methods for each use case.

We still need the templates property to exist in some way in order
to cache it, but it needs to be clear that it's different from
`.get_templates`. One option was to make it "private" (i.e. `_templates`),
and always use `.get_templates` in the rest of the code, but this requires
adding "include all folders" to `.get_templates`, which doesn't have an
obvious interface since `parent_folder_id=None` already means "top-level
only".

This will probably come up again when we need to look into adding
templates from nested folders into the page for live search, but
for now renaming `Service.templates` to `.all_templates` makes it
clear what the property contains.
This commit is contained in:
Alexey Bezhan
2018-11-05 15:26:59 +00:00
parent 078595da9d
commit 29bed8ba55
6 changed files with 25 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/main/views/conversation.py
View File

@@ -44,7 +44,7 @@ def conversation_reply(
service_id,
notification_id,
):
templates = current_service.templates_by_type('sms')
templates = current_service.get_templates('sms')
return render_template(
'views/templates/choose-reply.html',
templates=templates,

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

@@ -118,11 +118,11 @@ def choose_template(service_id, template_type='all'):
return render_template(
'views/templates/choose.html',
template_folders=current_service.template_folders,
templates=current_service.templates_by_type(template_type),
show_search_box=(len(current_service.templates_by_type(template_type)) > 7),
templates=current_service.get_templates(template_type),
show_search_box=(len(current_service.get_templates(template_type)) > 7),
show_template_nav=(
current_service.has_multiple_template_types
and (len(current_service.templates) > 2)
and (len(current_service.all_templates) > 2)
),
template_nav_items=template_nav_items,
template_type=template_type,

18
app/models/service.py
View File

@@ -68,7 +68,7 @@ class Service():
) > 1
@cached_property
def templates(self):
def all_templates(self):
templates = service_api_client.get_service_templates(self.id)['data']
@@ -77,12 +77,14 @@ class Service():
if template['template_type'] in self.available_template_types
]
def templates_by_type(self, template_type):
def get_templates(self, template_type='all', template_folder_id=None):
if isinstance(template_type, str):
template_type = [template_type]
return [
template for template in self.templates
if set(template_type) & {'all', template['template_type']}
template for template in self.all_templates
if (set(template_type) & {'all', template['template_type']})
and template.get('folder_id') == template_folder_id
]
@property
@@ -94,21 +96,21 @@ class Service():
@property
def has_templates(self):
return len(self.templates) > 0
return len(self.all_templates) > 0
@property
def has_multiple_template_types(self):
return len({
template['template_type'] for template in self.templates
template['template_type'] for template in self.all_templates
}) > 1
@property
def has_email_templates(self):
return len(self.templates_by_type('email')) > 0
return len(self.get_templates('email')) > 0
@property
def has_sms_templates(self):
return len(self.templates_by_type('sms')) > 0
return len(self.get_templates('sms')) > 0
@cached_property
def email_reply_to_addresses(self):

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

@@ -15,7 +15,7 @@
<div class="dashboard">
<h1 class="visuallyhidden">Dashboard</h1>
{% if current_user.has_permissions('manage_templates') and not current_service.templates %}
{% if current_user.has_permissions('manage_templates') and not current_service.all_templates %}
{% include 'views/dashboard/write-first-messages.html' %}
{% endif %}

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

@@ -13,7 +13,7 @@
</div>
<nav>
{% for service in services %}
{% set templates = service.templates_by_type(current_service.permissions) %}
{% set templates = service.get_templates(current_service.permissions) %}
{% if templates and services|length > 1 %}
<h2 class="">
{{ service.name }}

18
tests/app/main/views/test_service_settings.py
View File

@@ -563,13 +563,13 @@ def test_should_show_request_to_go_live_checklist(
)
mock_templates = mocker.patch(
'app.models.service.Service.templates',
'app.models.service.Service.all_templates',
new_callable=PropertyMock,
return_value=list(range(0, count_of_templates)),
)
mock_templates_by_type = mocker.patch(
'app.models.service.Service.templates_by_type',
mock_get_templates = mocker.patch(
'app.models.service.Service.get_templates',
side_effect=_templates_by_type,
)
@@ -598,7 +598,7 @@ def test_should_show_request_to_go_live_checklist(
assert mock_templates.call_args_list == [
call(),
]
assert mock_templates_by_type.call_args_list == [
assert mock_get_templates.call_args_list == [
call('email'),
call('sms'),
]
@@ -683,12 +683,12 @@ def test_should_check_for_sms_sender_on_go_live(
return_value=99,
)
mock_templates = mocker.patch(
'app.models.service.Service.templates',
'app.models.service.Service.all_templates',
new_callable=PropertyMock,
side_effect=partial(_templates_by_type, 'all'),
)
mock_templates_by_type = mocker.patch(
'app.models.service.Service.templates_by_type',
mock_get_templates = mocker.patch(
'app.models.service.Service.get_templates',
side_effect=_templates_by_type,
)
@@ -712,7 +712,7 @@ def test_should_check_for_sms_sender_on_go_live(
assert mock_templates.call_args_list == [
call(),
]
assert mock_templates_by_type.call_args_list == [
assert mock_get_templates.call_args_list == [
call('email'),
call('sms'),
]
@@ -747,7 +747,7 @@ def test_should_check_for_mou_on_request_to_go_live(
return_value=0,
)
mocker.patch(
'app.models.service.Service.templates',
'app.models.service.Service.all_templates',
new_callable=PropertyMock,
return_value=[],
)