mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-02-23 11:51:05 -05:00
Make choose template a list of template names
When a team has lots of templates the choose template page gets very long. It gets hard to find the template that you are looking for. Our initial reckon was that teams would not be giving their templates very useful names, and therefore a preview would be helpful. What we have found is that: - teams actually do give their templates useful names, and refer to these template names elsewhere - the previews are less useful for emails and text messages, because they have so much content (which for emails also makes it harder to `ctrl` + `f` the template name) The other problem we found was that this page presented the user with a _lot_ of options. For each template there were 4 actions, plus the click-to-preview action for letters, plus the ‘see previous version’ action for templates that had been edited multiple times. It was a very busy page. And the final problem (that we recently introduced) was that there was no way, other than the visual cues, to know whether a template was a letter, email, or text message. So this commit strips back the choose template page to be very focused on finding the right template, by only showing the template name and type. The user can then click through to a page that shows just a single template, and perform actions relevant to that template from that page.
This commit is contained in:
@@ -44,15 +44,7 @@ page_headings = {
|
||||
def choose_template(service_id):
|
||||
return render_template(
|
||||
'views/templates/choose.html',
|
||||
templates=[
|
||||
get_template(
|
||||
template,
|
||||
current_service,
|
||||
letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template['id']),
|
||||
)
|
||||
for template in service_api_client.get_service_templates(service_id)['data']
|
||||
if should_show_template(template['template_type'])
|
||||
],
|
||||
templates=service_api_client.get_service_templates(service_id)['data']
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
{% from "components/message-count-label.html" import message_count_label %}
|
||||
|
||||
{% extends "withnav_template.html" %}
|
||||
|
||||
{% block service_page_title %}
|
||||
@@ -44,19 +46,22 @@
|
||||
{% endif %}
|
||||
</div>
|
||||
|
||||
<div class="grid-row">
|
||||
<nav class="grid-row">
|
||||
{% for template in templates %}
|
||||
<div class="column-whole">
|
||||
<h2 class="message-name">{{ template.name }}</h2>
|
||||
{% if template.get_raw('updated_at', None) %}
|
||||
<p class="message-updated-at">
|
||||
Edited {{ template.get_raw('updated_at', None)|format_date_short }} <a href="{{ url_for('.view_template_versions', service_id=current_service.id, template_id=template.id) }}">see previous versions</a>
|
||||
</p>
|
||||
{% 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="hint">
|
||||
{{ message_count_label(1, template.template_type, suffix='')|capitalize }} template
|
||||
{% if template.get('updated_at', None) %}
|
||||
 Last edited {{ template.get('updated_at', None)|format_date_short }}
|
||||
 <a href="{{ url_for('.view_template_versions', service_id=current_service.id, template_id=template.id) }}">See previous versions</a>
|
||||
{% endif %}
|
||||
</p>
|
||||
</div>
|
||||
{% include 'views/templates/_template.html' %}
|
||||
{% endfor %}
|
||||
</div>
|
||||
</nav>
|
||||
{% endif %}
|
||||
|
||||
{% endblock %}
|
||||
|
||||
@@ -664,132 +664,6 @@ def test_route_invalid_permissions(
|
||||
service_one)
|
||||
|
||||
|
||||
def test_route_choose_template_manage_service_permissions(
|
||||
mocker,
|
||||
app_,
|
||||
client,
|
||||
api_user_active,
|
||||
service_one,
|
||||
mock_login,
|
||||
mock_get_user,
|
||||
mock_get_service,
|
||||
mock_check_verify_code,
|
||||
mock_get_service_templates,
|
||||
mock_get_jobs,
|
||||
):
|
||||
template_id = mock_get_service_templates(service_one['id'])['data'][0]['id']
|
||||
resp = validate_route_permission(
|
||||
mocker,
|
||||
app_,
|
||||
"GET",
|
||||
200,
|
||||
url_for(
|
||||
'main.choose_template',
|
||||
service_id=service_one['id'],
|
||||
),
|
||||
['manage_users', 'manage_templates', 'manage_settings'],
|
||||
api_user_active,
|
||||
service_one)
|
||||
page = resp.get_data(as_text=True)
|
||||
assert url_for(
|
||||
"main.send_messages",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) not in page
|
||||
assert url_for(
|
||||
"main.send_test",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) not in page
|
||||
assert url_for(
|
||||
"main.edit_service_template",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) in page
|
||||
|
||||
|
||||
def test_route_choose_template_send_messages_permissions(
|
||||
mocker,
|
||||
app_,
|
||||
client,
|
||||
active_user_with_permissions,
|
||||
service_one,
|
||||
mock_get_service,
|
||||
mock_check_verify_code,
|
||||
mock_get_service_templates,
|
||||
mock_get_jobs,
|
||||
):
|
||||
template_id = None
|
||||
for temp in mock_get_service_templates(service_one['id'])['data']:
|
||||
if temp['template_type'] == 'sms':
|
||||
template_id = temp['id']
|
||||
assert template_id
|
||||
resp = validate_route_permission(
|
||||
mocker,
|
||||
app_,
|
||||
"GET",
|
||||
200,
|
||||
url_for(
|
||||
'main.choose_template',
|
||||
service_id=service_one['id'],
|
||||
template_type='sms'),
|
||||
['send_texts', 'send_emails', 'send_letters'],
|
||||
active_user_with_permissions,
|
||||
service_one)
|
||||
page = resp.get_data(as_text=True)
|
||||
assert url_for(
|
||||
"main.send_messages",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) in page
|
||||
assert url_for(
|
||||
"main.edit_service_template",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) not in page
|
||||
|
||||
|
||||
def test_route_choose_template_manage_api_keys_permissions(
|
||||
mocker,
|
||||
app_,
|
||||
client,
|
||||
api_user_active,
|
||||
service_one,
|
||||
mock_get_user,
|
||||
mock_get_service,
|
||||
mock_check_verify_code,
|
||||
mock_get_service_templates,
|
||||
mock_get_jobs,
|
||||
):
|
||||
template_id = None
|
||||
for temp in mock_get_service_templates(service_one['id'])['data']:
|
||||
if temp['template_type'] == 'sms':
|
||||
template_id = temp['id']
|
||||
assert template_id
|
||||
resp = validate_route_permission(
|
||||
mocker,
|
||||
app_,
|
||||
"GET",
|
||||
200,
|
||||
url_for(
|
||||
'main.choose_template',
|
||||
service_id=service_one['id'],
|
||||
template_type='sms'),
|
||||
['manage_api_keys'],
|
||||
api_user_active,
|
||||
service_one)
|
||||
page = resp.get_data(as_text=True)
|
||||
assert url_for(
|
||||
"main.send_test",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) not in page
|
||||
assert url_for(
|
||||
"main.edit_service_template",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id) not in page
|
||||
page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser')
|
||||
links = page.findAll('a', href=re.compile('^' + url_for(
|
||||
"main.send_from_api",
|
||||
service_id=service_one['id'],
|
||||
template_id=template_id)))
|
||||
assert len(links) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'extra_args,expected_url',
|
||||
[
|
||||
|
||||
Reference in New Issue
Block a user