From 42ebc44b8352a7c7324629a4b45fb11adc7dd7de Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 17 May 2017 13:05:18 +0100
Subject: [PATCH] Make removing placeholders a non-breaking change
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The CSV upload route has always quietly ignored excess personalisation.
We changed the API to do the same here:
https://github.com/alphagov/notifications-api/pull/853

This means that removing a placeholder from a template is never a
breaking change, because the data that you were providing to populate it
is now just ignored.

So we don’t need to show the interstitial page in this case.
---
 app/main/views/templates.py            |  2 +-
 tests/app/main/views/test_templates.py | 32 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index df395924d..c417576f4 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -259,7 +259,7 @@ def edit_service_template(service_id, template_id):
             'process_type': form.process_type.data
         }, current_service)
         template_change = get_template(template, current_service).compare_to(new_template)
-        if template_change.has_different_placeholders and not request.form.get('confirm'):
+        if template_change.placeholders_added and not request.form.get('confirm'):
             example_column_headings = (
                 first_column_headings[new_template.template_type] +
                 list(new_template.placeholders)
diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py
index 0531fa245..cf0bc3fdd 100644
--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -405,6 +405,38 @@ def test_should_show_interstitial_when_making_breaking_change(
     )
 
 
+def test_removing_placeholders_is_not_a_breaking_change(
+    logged_in_client,
+    mock_get_service_email_template,
+    mock_update_service_template,
+    mock_has_permissions,
+    fake_uuid,
+):
+    service_id = fake_uuid
+    template_id = fake_uuid
+    existing_template = mock_get_service_email_template(0, 0)['data']
+    response = logged_in_client.post(
+        url_for(
+            '.edit_service_template',
+            service_id=service_id,
+            template_id=template_id
+        ),
+        data={
+            'name': existing_template['name'],
+            'template_content': "no placeholders",
+            'subject': existing_template['subject'],
+        }
+    )
+
+    assert response.status_code == 302
+    assert response.location == url_for(
+        'main.view_template',
+        service_id=service_id,
+        template_id=template_id,
+        _external=True,
+    )
+
+
 def test_should_not_create_too_big_template(
     logged_in_client,
     service_one,