Don’t send users back to a page that will send them forward again

If you’re on a page with a normal placeholder and the previous
placeholder is one that’s also in the address block then going back to
the previous page will send you immediately forward to the current page
again.

This commit makes the back link a bit smarter by skipping over pages
where it can see that they relate to a placeholder from the address
block.

If it gets all the way to the start of the list of placeholders without
finding any non-address ones then it will default to generating a link
that will redirect the user to filling in the address block again.

app/main/views/send.py

@@ -519,7 +519,7 @@ def send_test_step(service_id, template_id, step_index):
             help=get_help_argument(),
         ))
 
-    back_link = get_back_link(service_id, template, step_index)
+    back_link = get_back_link(service_id, template, step_index, placeholders)
 
     template.values = get_recipient_and_placeholders_from_session(template.template_type)
     template.values[current_placeholder] = None
@@ -915,7 +915,7 @@ def is_current_user_the_recipient():
     return session['recipient'] == current_user.email_address
 
 
-def get_back_link(service_id, template, step_index):
+def get_back_link(service_id, template, step_index, placeholders=None):
     if get_help_argument():
         # if we're on the check page, redirect back to the beginning. anywhere else, don't return the back link
         if request.endpoint == 'main.check_notification':
@@ -967,13 +967,26 @@ def get_back_link(service_id, template, step_index):
             step_index=0,
         )
 
-    else:
-        return url_for(
-            'main.send_one_off_step',
-            service_id=service_id,
-            template_id=template.id,
-            step_index=step_index - 1,
-        )
+    if template.template_type == 'letter' and placeholders:
+        # Make sure we’re not redirecting users to a page which will
+        # just redirect them forwards again
+        back_link_destination_step_index = next((
+            index
+            for index, placeholder in reversed(
+                list(enumerate(placeholders[:step_index]))
+            )
+            if placeholder not in Columns(
+                PostalAddress('').as_personalisation
+            )
+        ), 1)
+        return get_back_link(service_id, template, back_link_destination_step_index + 1)
+
+    return url_for(
+        'main.send_one_off_step',
+        service_id=service_id,
+        template_id=template.id,
+        step_index=step_index - 1,
+    )
 
 
 @main.route("/services/<uuid:service_id>/template/<uuid:template_id>/notification/check", methods=['GET'])

tests/app/main/views/test_send.py

@@ -2061,6 +2061,7 @@ def test_send_one_off_letter_copes_with_placeholder_from_address_block(
     mocker,
     fake_uuid,
     mock_template_preview,
+    no_letter_contact_blocks,
     placeholder,
 ):
     mocker.patch(
@@ -2107,6 +2108,19 @@ def test_send_one_off_letter_copes_with_placeholder_from_address_block(
             'postcode': 'SW1A 1AA',
         }
 
+    back_link = page.select_one('.govuk-back-link')['href']
+    assert back_link == url_for(
+        'main.send_one_off_step',
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+        step_index=1,
+    )
+    previous_page = client_request.get_url(back_link, _follow_redirects=True)
+
+    # We’ve skipped past the address placeholder and gone back to the
+    # address block
+    assert normalize_spaces(previous_page.select_one('form label').text) == 'Address'
+
 
 def test_send_test_sms_message_puts_submitted_data_in_session(
     client_request,