Merge pull request #2174 from alphagov/copy-template

Add option to copy existing template when adding
This commit is contained in:
Chris Hill-Scott
2018-07-30 13:02:46 +01:00
committed by GitHub
14 changed files with 296 additions and 13 deletions

View File

@@ -81,6 +81,14 @@
margin-bottom: $gutter * 2;
}
.left-gutter {
padding-left: $gutter;
}
.left-gutter-4-3 {
padding-left: $gutter * 4 / 3;
}
.align-with-heading {
display: block;
text-align: center;

View File

@@ -837,14 +837,15 @@ class ChooseTemplateType(StripWhitespaceForm):
]
)
def __init__(self, include_letters=False, *args, **kwargs):
def __init__(self, include_letters=False, include_copy=False, *args, **kwargs):
super().__init__(*args, **kwargs)
self.template_type.choices = filter(None, [
('email', 'Email'),
('sms', 'Text message'),
('letter', 'Letter') if include_letters else None
('letter', 'Letter') if include_letters else None,
('copy-existing', 'Copy of an existing template') if include_copy else None,
])

View File

@@ -30,9 +30,9 @@ def find_users_by_email():
@user_is_platform_admin
def user_information(user_id):
user = user_api_client.get_user(user_id)
services = user_api_client.get_organisations_and_services_for_user(user)
services = user_api_client.get_services_for_user(user)
return render_template(
'views/find-users/user-information.html',
user=user,
services=services['services_without_organisations'],
services=services,
)

View File

@@ -9,7 +9,12 @@ from notifications_python_client.errors import HTTPError
from notifications_utils.formatters import nl2br
from notifications_utils.recipients import first_column_headings
from app import current_service, service_api_client, template_statistics_client
from app import (
current_service,
service_api_client,
template_statistics_client,
user_api_client,
)
from app.main import main
from app.main.forms import (
ChooseTemplateType,
@@ -202,11 +207,21 @@ def view_template_version_preview(service_id, template_id, version, filetype):
def add_template_by_type(service_id):
form = ChooseTemplateType(
include_letters='letter' in current_service['permissions']
include_letters='letter' in current_service['permissions'],
include_copy=any((
service_api_client.count_service_templates(service_id),
len(user_api_client.get_service_ids_for_user(current_user)) > 1,
)),
)
if form.validate_on_submit():
if form.template_type.data == 'copy-existing':
return redirect(url_for(
'.choose_template_to_copy',
service_id=service_id,
))
if form.template_type.data == 'letter':
blank_letter = service_api_client.create_service_template(
'Untitled',
@@ -240,6 +255,51 @@ def add_template_by_type(service_id):
return render_template('views/templates/add.html', form=form)
@main.route("/services/<service_id>/templates/copy")
@login_required
@user_has_permissions('manage_templates')
def choose_template_to_copy(service_id):
return render_template(
'views/templates/copy.html',
services=[{
'name': service['name'],
'id': service['id'],
'templates': [
template for template in
service_api_client.get_service_templates(service['id'])['data']
if template['template_type'] in current_service['permissions']
],
} for service in user_api_client.get_services_for_user(current_user)],
)
@main.route("/services/<service_id>/templates/copy/<uuid:template_id>", methods=['GET', 'POST'])
@login_required
@user_has_permissions('manage_templates')
def copy_template(service_id, template_id):
if not user_api_client.user_belongs_to_service(
current_user, request.args.get('from_service')
):
abort(403)
template = service_api_client.get_service_template(
request.args.get('from_service'),
str(template_id),
)['data']
template['template_content'] = template['content']
template['name'] = 'Copy of {}'.format(template['name'])
form = form_objects[template['template_type']](**template)
return render_template(
'views/edit-{}-template.html'.format(template['template_type']),
form=form,
template_type=template['template_type'],
heading_action='Add',
services=user_api_client.get_service_ids_for_user(current_user),
)
@main.route("/services/<service_id>/templates/action-blocked/<notification_type>/<return_to>/<template_id>")
@login_required
@user_has_permissions('manage_templates')

View File

@@ -125,6 +125,7 @@ class HeaderNavigation(Navigation):
'choose_account',
'choose_service',
'choose_template',
'choose_template_to_copy',
'confirm_edit_organisation_name',
'confirm_redact_template',
'conversation',
@@ -132,6 +133,7 @@ class HeaderNavigation(Navigation):
'conversation_reply_with_template',
'conversation_updates',
'cookies',
'copy_template',
'create_api_key',
'delete_service_template',
'delivery_and_failure',
@@ -291,8 +293,10 @@ class MainNavigation(Navigation):
'check_messages',
'check_notification',
'choose_template',
'choose_template_to_copy',
'confirm_redact_template',
'conversation_reply',
'copy_template',
'delete_service_template',
'edit_service_template',
'send_messages',
@@ -544,6 +548,7 @@ class CaseworkNavigation(Navigation):
'check_notification',
'choose_account',
'choose_service',
'choose_template_to_copy',
'confirm_edit_organisation_name',
'confirm_redact_template',
'conversation',
@@ -551,6 +556,7 @@ class CaseworkNavigation(Navigation):
'conversation_reply_with_template',
'conversation_updates',
'cookies',
'copy_template',
'create_api_key',
'create_email_branding',
'delete_service_template',
@@ -771,12 +777,14 @@ class OrgNavigation(Navigation):
'choose_account',
'choose_service',
'choose_template',
'choose_template_to_copy',
'confirm_redact_template',
'conversation',
'conversation_reply',
'conversation_reply_with_template',
'conversation_updates',
'cookies',
'copy_template',
'create_api_key',
'create_email_branding',
'delete_service_template',

View File

@@ -1,3 +1,5 @@
from itertools import chain
from notifications_python_client.errors import HTTPError
from app.notify_client import NotifyAdminAPIClient, cache
@@ -209,3 +211,17 @@ class UserApiClient(NotifyAdminAPIClient):
def get_organisations_and_services_for_user(self, user):
endpoint = '/user/{}/organisations-and-services'.format(user.id)
return self.get(endpoint)
def get_services_for_user(self, user):
orgs_and_services_for_user = self.get_organisations_and_services_for_user(user)
return orgs_and_services_for_user['services_without_organisations'] + next(chain(
org['services'] for org in orgs_and_services_for_user['organisations']
), [])
def get_service_ids_for_user(self, user):
return {
service['id'] for service in self.get_services_for_user(user)
}
def user_belongs_to_service(self, user, service_id):
return service_id in self.get_service_ids_for_user(user)

View File

@@ -109,7 +109,7 @@
{% block footer_support_links %}
<nav class="footer-nav">
Built by the <a href="https://www.gov.uk/government/organisations/government-digital-service">Government Digital Service</a>
<a href="{{ url_for("main.privacy") }}">Privacy</a>
<a href="{{ url_for("main.privacy") }}">Privacy</a>
<a href="{{ url_for("main.cookies") }}">Cookies</a>
{% if current_service.research_mode %}
<span id="research-mode" class="research-mode">research mode</span>

View File

@@ -13,7 +13,10 @@
{{ heading_action }} email template
</h1>
<form method="post">
<form
method="post"
action="{{ url_for('.add_service_template', service_id=current_service.id, template_type=template_type) }}"
>
<div class="grid-row">
<div class="column-five-sixths">
{{ textbox(form.name, width='1-1', hint='Your recipients wont see this', rows=10) }}

View File

@@ -12,7 +12,10 @@
{{ heading_action }} letter template
</h1>
<form method="post">
<form
method="post"
action="{{ url_for('.add_service_template', service_id=current_service.id, template_type=template_type) }}"
>
<div class="grid-row">
<div class="column-five-sixths">
{{ textbox(form.name, width='1-1', hint='Your recipients wont see this', rows=10) }}

View File

@@ -13,7 +13,10 @@
{{ heading_action }} text message template
</h1>
<form method="post">
<form
method="post"
action="{{ url_for('.add_service_template', service_id=current_service.id, template_type=template_type) }}"
>
<div class="grid-row">
<div class="column-two-thirds">
{{ textbox(form.name, width='1-1', hint='Your recipients wont see this') }}

View File

@@ -0,0 +1,36 @@
{% from "components/message-count-label.html" import message_count_label %}
{% extends "withnav_template.html" %}
{% block service_page_title %}
Copy an existing template
{% endblock %}
{% block maincolumn_content %}
<div class="bottom-gutter-3-2">
<h1 class="heading-large">Copy an existing template</h1>
</div>
<nav>
{% for service in services %}
{% if service.templates and services|length > 1 %}
<h2 class="">
{{ service.name }}
</h2>
<div class="left-gutter-4-3 bottom-gutter-3-2">
{% endif %}
{% for template in service.templates %}
<h2 class="message-name">
<a href="{{ url_for('.copy_template', service_id=current_service.id, template_id=template.id, from_service=service.id) }}">{{ template.name }}</a>
</h2>
<p class="message-type">
{{ message_count_label(1, template.template_type, suffix='')|capitalize }} template
</p>
{% endfor %}
{% if service.templates %}
</div>
{% endif %}
{% endfor %}
</nav>
{% endblock %}

View File

@@ -56,17 +56,19 @@ def test_letters_lets_in_without_permission(
@pytest.mark.parametrize('permissions, choices', [
(
['email', 'sms', 'letter'],
['Email', 'Text message', 'Letter']
['Email', 'Text message', 'Letter', 'Copy of an existing template']
),
(
['email', 'sms'],
['Email', 'Text message']
['Email', 'Text message', 'Copy of an existing template']
),
])
def test_given_option_to_add_letters_if_allowed(
logged_in_client,
service_one,
mocker,
mock_get_service_templates,
mock_get_organisations_and_services_for_user,
permissions,
choices,
):

View File

@@ -18,6 +18,8 @@ from tests import (
)
from tests.conftest import (
SERVICE_ONE_ID,
SERVICE_TWO_ID,
TEMPLATE_ONE_ID,
active_caseworking_user,
active_user_view_permissions,
mock_get_service_email_template,
@@ -394,11 +396,127 @@ def test_dont_show_preview_letter_templates_for_bad_filetype(
assert mock_get_service_template.called is False
def test_choosing_to_copy_redirects(
client_request,
mock_get_service_templates,
mock_get_organisations_and_services_for_user,
):
client_request.post(
'main.add_template_by_type',
service_id=SERVICE_ONE_ID,
_data={'template_type': 'copy-existing'}
)
def test_choose_a_template_to_copy(
client_request,
mock_get_service_templates,
mock_get_non_empty_organisations_and_services_for_user,
):
page = client_request.get(
'main.choose_template_to_copy',
service_id=SERVICE_ONE_ID,
)
assert normalize_spaces(
page.select_one('main nav').text
) == normalize_spaces(
'Service 1 '
' sms_template_one '
' Text message template'
' sms_template_two Text message template'
' email_template_one Email template'
' email_template_two Email template '
'Service 2 '
' sms_template_one'
' Text message template'
' sms_template_two'
' Text message template'
' email_template_one'
' Email template'
' email_template_two'
' Email template '
'Org 1 service 1 '
' sms_template_one'
' Text message template'
' sms_template_two'
' Text message template'
' email_template_one'
' Email template'
' email_template_two'
' Email template '
'Org 1 service 2 '
' sms_template_one'
' Text message template'
' sms_template_two'
' Text message template'
' email_template_one'
' Email template'
' email_template_two'
' Email template'
)
assert page.select_one('main nav a')['href'] == url_for(
'main.copy_template',
service_id=SERVICE_ONE_ID,
template_id=TEMPLATE_ONE_ID,
from_service=SERVICE_TWO_ID,
)
def test_load_edit_template_with_copy_of_template(
client_request,
mock_get_service_email_template,
mock_get_non_empty_organisations_and_services_for_user,
):
page = client_request.get(
'main.copy_template',
service_id=SERVICE_ONE_ID,
template_id=TEMPLATE_ONE_ID,
from_service=SERVICE_TWO_ID,
)
assert page.select_one('form')['method'] == 'post'
assert page.select_one('form')['action'] == url_for(
'main.add_service_template',
service_id=SERVICE_ONE_ID,
template_type='email',
)
assert page.select_one('input')['value'] == (
'Copy of Two week reminder'
)
assert page.select_one('textarea').text == (
'Your ((thing)) is due soon'
)
mock_get_service_email_template.assert_called_once_with(
SERVICE_TWO_ID,
TEMPLATE_ONE_ID,
)
def test_cant_copy_template_from_non_member_service(
client_request,
mock_get_service_email_template,
mock_get_organisations_and_services_for_user,
):
client_request.get(
'main.copy_template',
service_id=SERVICE_ONE_ID,
template_id=TEMPLATE_ONE_ID,
from_service=SERVICE_TWO_ID,
_expected_status=403,
)
assert mock_get_service_email_template.call_args_list == []
@pytest.mark.parametrize('type_of_template', ['email', 'sms'])
def test_should_not_allow_creation_of_template_through_form_without_correct_permission(
logged_in_client,
service_one,
mocker,
mock_get_service_templates,
mock_get_organisations_and_services_for_user,
type_of_template,
):
service_one['permissions'] = []

View File

@@ -680,6 +680,7 @@ def mock_update_service_raise_httperror_duplicate_name(mocker):
SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb"
SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
ORGANISATION_ID = "c011fa40-4cbe-4524-b415-dde2f421bd9c"
TEMPLATE_ONE_ID = "b22d7d94-2197-4a7d-a8e7-fd5f9770bf48"
@pytest.fixture(scope='function')
@@ -949,7 +950,7 @@ def mock_update_service_template_400_content_too_big(mocker):
@pytest.fixture(scope='function')
def mock_get_service_templates(mocker):
uuid1 = str(generate_uuid())
uuid1 = TEMPLATE_ONE_ID
uuid2 = str(generate_uuid())
uuid3 = str(generate_uuid())
uuid4 = str(generate_uuid())
@@ -3013,6 +3014,30 @@ def mock_get_organisations_and_services_for_user(mocker, organisation_one, api_u
)
@pytest.fixture
def mock_get_non_empty_organisations_and_services_for_user(mocker, organisation_one, api_user_active):
def _make_services(name):
return [{
'name': '{} {}'.format(name, i),
'id': SERVICE_TWO_ID,
} for i in range(1, 3)]
def _get_orgs_and_services(user_id):
return {
'organisations': [
{'name': 'Org 1', 'services': _make_services('Org 1 service')},
{'name': 'Org 2', 'services': _make_services('Org 2 service')},
],
'services_without_organisations': _make_services('Service')
}
return mocker.patch(
'app.user_api_client.get_organisations_and_services_for_user',
side_effect=_get_orgs_and_services
)
@pytest.fixture
def mock_create_event(mocker):
"""