darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-17 03:40:04 -04:00
Code Issues Packages Projects Releases Wiki Activity

Put templates on service model

Browse Source 
We do a lot of logic around choosing which templates to show. This logic
is all inside one view method.

It makes it cleaner to break this logic up into functions. But this
would mean passing around variables from one function to another.
Putting these methods onto a class (the service model) means that
there’s a place to store this data (rather than having to pass it around
a lot).

Making this code more manageable is important so that when we have
templates and folders it’s easy to encapsulate the logic around
combining the two.
This commit is contained in:
Chris Hill-Scott
2018-10-25 07:59:50 +01:00
parent d69e8b50cd
commit 1e6b79a546
6 changed files with 86 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -4,7 +4,7 @@ from notifications_python_client.errors import HTTPError
from notifications_utils.recipients import format_phone_number_human_readable
from notifications_utils.template import SMSPreviewTemplate
from app import notification_api_client, service_api_client
from app import current_service, notification_api_client, service_api_client
from app.main import main
from app.main.forms import SearchTemplatesForm
from app.utils import user_has_permissions
@@ -44,13 +44,7 @@ def conversation_reply(
service_id,
notification_id,
):
templates = [
template
for template in service_api_client.get_service_templates(service_id)['data']
if template['template_type'] == 'sms'
]
templates = current_service.templates_by_type('sms')
return render_template(
'views/templates/choose-reply.html',
templates=templates,

1
app/main/views/dashboard.py
View File

@@ -72,7 +72,6 @@ def service_dashboard(service_id):
return render_template(
'views/dashboard/dashboard.html',
updates_url=url_for(".service_dashboard_updates", service_id=service_id),
templates=service_api_client.get_service_templates(service_id)['data'],
partials=get_dashboard_partials(service_id)
)

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

@@ -101,24 +101,6 @@ def start_tour(service_id, template_id):
@login_required
@user_has_permissions()
def choose_template(service_id, template_type='all'):
templates = service_api_client.get_service_templates(service_id)['data']
letters_available = current_service.has_permission('letter')
available_template_types = list(filter(None, (
'email',
'sms',
'letter' if letters_available else None,
)))
templates = [
template for template in templates
if template['template_type'] in available_template_types
]
has_multiple_template_types = len({
template['template_type'] for template in templates
}) > 1
template_nav_items = [
(label, key, url_for('.choose_template', service_id=current_service.id, template_type=key), '')
@@ -126,23 +108,18 @@ def choose_template(service_id, template_type='all'):
('All', 'all'),
('Text message', 'sms'),
('Email', 'email'),
('Letter', 'letter') if letters_available else None,
('Letter', 'letter') if current_service.has_permission('letter') else None,
])
]
templates_on_page = [
template for template in templates
if (
template_type in ['all', template['template_type']] and
template['template_type'] in available_template_types
)
]
return render_template(
'views/templates/choose.html',
templates=templates_on_page,
show_search_box=(len(templates_on_page) > 7),
show_template_nav=has_multiple_template_types and (len(templates) > 2),
templates=current_service.templates_by_type(template_type),
show_search_box=(len(current_service.templates_by_type(template_type)) > 7),
show_template_nav=(
current_service.has_multiple_template_types
and (len(current_service.templates) > 2)
),
template_nav_items=template_nav_items,
template_type=template_type,
search_form=SearchTemplatesForm(),

46
app/notify_client/models.py
View File

@@ -319,25 +319,47 @@ class Service(dict):
) > 1
@property
def has_templates(self):
def templates(self):
from app import service_api_client
return service_api_client.count_service_templates(
self.id
) > 0
templates = service_api_client.get_service_templates(self.id)['data']
return [
template for template in templates
if template['template_type'] in self.available_template_types
]
def templates_by_type(self, template_type):
return [
template for template in self.templates
if template_type in {'all', template['template_type']}
]
@property
def available_template_types(self):
return [
channel for channel in ('email', 'sms', 'letter')
if self.has_permission(channel)
]
@property
def has_templates(self):
return len(self.templates) > 0
@property
def has_multiple_template_types(self):
return len({
template['template_type'] for template in self.templates
}) > 1
@property
def has_email_templates(self):
from app import service_api_client
return service_api_client.count_service_templates(
self.id, template_type='email'
) > 0
return len(self.templates_by_type('email')) > 0
@property
def has_sms_templates(self):
from app import service_api_client
return service_api_client.count_service_templates(
self.id, template_type='sms'
) > 0
return len(self.templates_by_type('sms')) > 0
@property
def has_email_reply_to_address(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 templates %}
{% if current_user.has_permissions('manage_templates') and not current_service.templates %}
{% include 'views/dashboard/write-first-messages.html' %}
{% endif %}

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

@@ -551,20 +551,28 @@ def test_should_show_request_to_go_live_checklist(
expected_reply_to_checklist_item,
):
def _count_templates(service_id, template_type=None):
def _templates_by_type(template_type):
return {
'email': count_of_email_templates,
'sms': 0,
}.get(template_type, count_of_templates)
'email': list(range(0, count_of_email_templates)),
'sms': [],
}.get(template_type)
mock_count_users = mocker.patch(
'app.main.views.service_settings.user_api_client.get_count_of_users_with_permission',
return_value=count_of_users_with_manage_service
)
mock_count_templates = mocker.patch(
'app.main.views.service_settings.service_api_client.count_service_templates',
side_effect=_count_templates
mock_templates = mocker.patch(
'app.notify_client.models.Service.templates',
new_callable=PropertyMock,
return_value=list(range(0, count_of_templates)),
)
mock_templates_by_type = mocker.patch(
'app.notify_client.models.Service.templates_by_type',
side_effect=_templates_by_type,
)
mock_get_reply_to_email_addresses = mocker.patch(
'app.main.views.service_settings.service_api_client.get_reply_to_email_addresses',
return_value=reply_to_email_addresses
@@ -587,10 +595,12 @@ def test_should_show_request_to_go_live_checklist(
)
mock_count_users.assert_called_once_with(SERVICE_ONE_ID, 'manage_service')
assert mock_count_templates.call_args_list == [
call(SERVICE_ONE_ID),
call(SERVICE_ONE_ID, template_type='email'),
call(SERVICE_ONE_ID, template_type='sms'),
assert mock_templates.call_args_list == [
call(),
]
assert mock_templates_by_type.call_args_list == [
call('email'),
call('sms'),
]
if count_of_email_templates:
@@ -662,20 +672,26 @@ def test_should_check_for_sms_sender_on_go_live(
service_one['organisation_type'] = organisation_type
def _count_templates(service_id, template_type=None):
return {
def _templates_by_type(template_type):
return list(range(0, {
'email': 0,
'sms': count_of_sms_templates,
}.get(template_type, count_of_sms_templates)
}.get(template_type, count_of_sms_templates)))
mocker.patch(
'app.main.views.service_settings.user_api_client.get_count_of_users_with_permission',
return_value=99,
)
mock_count_templates = mocker.patch(
'app.main.views.service_settings.service_api_client.count_service_templates',
side_effect=_count_templates,
mock_templates = mocker.patch(
'app.notify_client.models.Service.templates',
new_callable=PropertyMock,
side_effect=partial(_templates_by_type, 'all'),
)
mock_templates_by_type = mocker.patch(
'app.notify_client.models.Service.templates_by_type',
side_effect=_templates_by_type,
)
mock_get_sms_senders = mocker.patch(
'app.main.views.service_settings.service_api_client.get_sms_senders',
return_value=sms_senders,
@@ -693,10 +709,12 @@ def test_should_check_for_sms_sender_on_go_live(
checklist_items = page.select('.task-list .task-list-item')
assert normalize_spaces(checklist_items[2].text) == expected_sms_sender_checklist_item
assert mock_count_templates.call_args_list == [
call(SERVICE_ONE_ID),
call(SERVICE_ONE_ID, template_type='email'),
call(SERVICE_ONE_ID, template_type='sms'),
assert mock_templates.call_args_list == [
call(),
]
assert mock_templates_by_type.call_args_list == [
call('email'),
call('sms'),
]
mock_get_sms_senders.assert_called_once_with(SERVICE_ONE_ID)
@@ -729,8 +747,9 @@ def test_should_check_for_mou_on_request_to_go_live(
return_value=0,
)
mocker.patch(
'app.main.views.service_settings.service_api_client.count_service_templates',
return_value=0,
'app.notify_client.models.Service.templates',
new_callable=PropertyMock,
return_value=[],
)
mocker.patch(
'app.main.views.service_settings.service_api_client.get_sms_senders',