diff --git a/app/main/views/send.py b/app/main/views/send.py index f4aa8e4ee..494a3a1b4 100644 --- a/app/main/views/send.py +++ b/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//template//notification/check", methods=['GET']) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 1e2af293f..ddafa5cc4 100644 --- a/tests/app/main/views/test_send.py +++ b/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,