From 0888bdf5d4c3f48d62530992684c99a73292616e Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 13 Nov 2018 15:06:39 +0000
Subject: [PATCH] =?UTF-8?q?Put=20=E2=80=98copy=E2=80=99=20at=20end=20of=20?=
 =?UTF-8?q?new=20template=20name?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In multiple user research sessions we’ve noticed people edit the
auto-generated template name to put something at the end of it.

This is fiddly because of the quotes we put around the name:
> Copy of ‘Exiting template’

It also means that if they keep our prefix then the template doesn’t
sort alongside the one it’s replacing.

This commit changes the name of copied templates to better match the
behaviour our users are showing.

Also adds a bit of auto numbering, just as a nice detail.
---
 app/main/views/templates.py            | 16 +++++++++++-
 tests/app/main/views/test_templates.py | 35 +++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index 8c91f609b..f69a830a4 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -317,7 +317,7 @@ def copy_template(service_id, template_id):
         return add_service_template(service_id, template['template_type'])
 
     template['template_content'] = template['content']
-    template['name'] = 'Copy of ‘{}’'.format(template['name'])
+    template['name'] = _get_template_copy_name(template, current_service.all_templates)
     form = form_objects[template['template_type']](**template)
 
     return render_template(
@@ -329,6 +329,20 @@ def copy_template(service_id, template_id):
     )
 
 
+def _get_template_copy_name(template, existing_templates):
+
+    template_names = [existing['name'] for existing in existing_templates]
+
+    for index in reversed(range(1, 10)):
+        if '{} (copy {})'.format(template['name'], index) in template_names:
+            return '{} (copy {})'.format(template['name'], index + 1)
+
+    if '{} (copy)'.format(template['name']) in template_names:
+        return '{} (copy 2)'.format(template['name'])
+
+    return '{} (copy)'.format(template['name'])
+
+
 @main.route("/services/<service_id>/templates/action-blocked/<notification_type>/<return_to>/<template_id>")
 @login_required
 @user_has_permissions('manage_templates')
diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py
index 1a57db7e5..6cd8a6160 100644
--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -785,11 +785,44 @@ def test_choose_a_template_to_copy(
     )
 
 
+@pytest.mark.parametrize('existing_template_names, expected_name', (
+    (
+        ['Two week reminder'],
+        'Two week reminder (copy)'
+    ),
+    (
+        ['Two week reminder (copy)'],
+        'Two week reminder (copy 2)'
+    ),
+    (
+        ['Two week reminder', 'Two week reminder (copy)'],
+        'Two week reminder (copy 2)'
+    ),
+    (
+        ['Two week reminder (copy 8)', 'Two week reminder (copy 9)'],
+        'Two week reminder (copy 10)'
+    ),
+    (
+        ['Two week reminder (copy)', 'Two week reminder (copy 9)'],
+        'Two week reminder (copy 10)'
+    ),
+    (
+        ['Two week reminder (copy)', 'Two week reminder (copy 10)'],
+        'Two week reminder (copy 2)'
+    ),
+))
 def test_load_edit_template_with_copy_of_template(
     client_request,
+    mock_get_service_templates,
     mock_get_service_email_template,
     mock_get_non_empty_organisations_and_services_for_user,
+    existing_template_names,
+    expected_name,
 ):
+    mock_get_service_templates.side_effect = lambda service_id: {'data': [
+        {'name': existing_template_name, 'template_type': 'sms'}
+        for existing_template_name in existing_template_names
+    ]}
     page = client_request.get(
         'main.copy_template',
         service_id=SERVICE_ONE_ID,
@@ -800,7 +833,7 @@ def test_load_edit_template_with_copy_of_template(
     assert page.select_one('form')['method'] == 'post'
 
     assert page.select_one('input')['value'] == (
-        'Copy of ‘Two week reminder’'
+        expected_name
     )
     assert page.select_one('textarea').text == (
         'Your ((thing)) is due soon'