From 53918f8d9fa1ce3446d3d540c6022f202b28f1f2 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 9 Apr 2020 14:33:47 +0100
Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20go=20back=20to=20address=20if?=
 =?UTF-8?q?=20address=20placeholder=20in=20letter?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If you have an placeholder from the address block elsewhere in your
letter then you currently get redirected to the address block page
instead of being offered to fill that placeholder in. This commit
tightens up the check to only do this when the placeholder is in the
first 7 placeholders, which is where we store the address placeholders.
---
 app/main/views/send.py            |  9 ++---
 tests/app/main/views/test_send.py | 63 +++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/app/main/views/send.py b/app/main/views/send.py
index 249be7bd7..946900f8f 100644
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -18,10 +18,7 @@ from notifications_python_client.errors import HTTPError
 from notifications_utils import LETTER_MAX_PAGE_COUNT, SMS_CHAR_COUNT_LIMIT
 from notifications_utils.columns import Columns
 from notifications_utils.pdf import is_letter_too_long
-from notifications_utils.postal_address import (
-    PostalAddress,
-    address_lines_1_to_6_and_postcode_keys,
-)
+from notifications_utils.postal_address import PostalAddress
 from notifications_utils.recipients import (
     RecipientCSV,
     first_column_headings,
@@ -476,8 +473,8 @@ def send_test_step(service_id, template_id, step_index):
         ))
 
     # if we're in a letter, we should show address block rather than "address line #" or "postcode"
-    if template.template_type == 'letter' and current_placeholder in (
-        Columns.from_keys(address_lines_1_to_6_and_postcode_keys)
+    if template.template_type == 'letter' and (
+        step_index < len(first_column_headings['letter'])
     ):
         return redirect(url_for('.send_one_off_letter_address', service_id=service_id, template_id=template_id))
 
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 2e10a971d..09f54b723 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -29,6 +29,7 @@ from xlrd.xldate import (
 )
 
 from tests import (
+    template_json,
     validate_route_permission,
     validate_route_permission_with_client,
 )
@@ -2045,6 +2046,68 @@ def test_send_one_off_back_link_populates_address_textarea(
     assert textarea.text == 'foo\nbar'
 
 
+@pytest.mark.parametrize('placeholder', (
+    'address_line_1',
+    'address_line_2',
+    'address_line_3',
+    'address_line_4',
+    'address_line_5',
+    'address_line_6',
+    'address_line_7',
+    'postcode',
+))
+def test_send_one_off_letter_copes_with_placeholder_from_address_block(
+    client_request,
+    mocker,
+    fake_uuid,
+    mock_template_preview,
+    placeholder,
+):
+    mocker.patch(
+        'app.service_api_client.get_service_template',
+        return_value={'data': template_json(
+            SERVICE_ONE_ID,
+            fake_uuid,
+            name="Awkward letter",
+            type_="letter",
+            subject=f"Hello (({placeholder}))",
+            content="We need to talk about ((thing))",
+        )},
+    )
+    with client_request.session_transaction() as session:
+        session['recipient'] = None
+        session['placeholders'] = {}
+        session['send_test_letter_page_count'] = None
+
+    page = client_request.post(
+        'main.send_one_off_letter_address',
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+        _data={'address': '''
+            foo
+            bar
+            SW1A 1AA
+        '''},
+        _follow_redirects=True,
+    )
+
+    with client_request.session_transaction() as session:
+        assert normalize_spaces(page.select_one('form label').text) == placeholder
+        assert page.select_one('form input[type=text]')['value'] == (
+            session['placeholders'].get(placeholder, '')
+        )
+        assert session['placeholders'] == {
+            'address_line_1': 'foo',
+            'address_line_2': 'bar',
+            'address_line_3': '',
+            'address_line_4': '',
+            'address_line_5': '',
+            'address_line_6': '',
+            'address_line_7': 'SW1A 1AA',
+            'postcode': 'SW1A 1AA',
+        }
+
+
 def test_send_test_sms_message_puts_submitted_data_in_session(
     client_request,
     service_one,