Merge pull request #2993 from alphagov/fix_test_send_placeholders

Fix error where sending test message flow skipped first placeholder

app/main/views/send.py

@@ -386,7 +386,10 @@ def send_test_step(service_id, template_id, step_index):
     )
 
     try:
-        current_placeholder = placeholders[step_index]
+        if request.endpoint == 'main.send_test_step':
+            current_placeholder = placeholders[step_index - 1]
+        else:
+            current_placeholder = placeholders[step_index]
     except IndexError:
         if all_placeholders_in_session(placeholders):
             return get_notification_check_endpoint(service_id, template)
@@ -413,9 +416,9 @@ def send_test_step(service_id, template_id, step_index):
         # Only if it's not a letter.
         # And only if we're not on the test route, since that will already have the user's own number set
         if (
-            step_index == 0 and
+            step_index == 0
-            template.template_type != 'letter' and
+            and template.template_type != 'letter'
-            request.endpoint != 'main.send_test_step'
+            and request.endpoint != 'main.send_test_step'
         ):
             session['recipient'] = form.placeholder_value.data
 
@@ -438,11 +441,11 @@ def send_test_step(service_id, template_id, step_index):
     template.values[current_placeholder] = None
 
     if (
-        request.endpoint == 'main.send_one_off_step' and
+        request.endpoint == 'main.send_one_off_step'
-        step_index == 0 and
+        and step_index == 0
-        template.template_type != 'letter' and
+        and template.template_type != 'letter'
-        not (template.template_type == 'sms' and current_user.mobile_number is None) and
+        and not (template.template_type == 'sms' and current_user.mobile_number is None)
-        current_user.has_permissions('manage_templates', 'manage_service')
+        and current_user.has_permissions('manage_templates', 'manage_service')
     ):
         skip_link = (
             'Use my {}'.format(first_column_headings[template.template_type][0]),
@@ -465,8 +468,8 @@ def send_test_step(service_id, template_id, step_index):
         back_link=back_link,
         help=get_help_argument(),
         link_to_upload=(
-            request.endpoint == 'main.send_one_off_step' and
+            request.endpoint == 'main.send_one_off_step'
-            step_index == 0
+            and step_index == 0
         ),
     )
 
@@ -808,13 +811,20 @@ def get_back_link(service_id, template, step_index):
                 service_id=service_id,
                 template_id=template.id,
             )
-    elif is_current_user_the_recipient() and step_index >= 1:
+    elif is_current_user_the_recipient() and step_index > 1:
         return url_for(
             'main.send_test_step',
             service_id=service_id,
             template_id=template.id,
             step_index=step_index - 1,
         )
+    elif is_current_user_the_recipient() and step_index == 1:
+        return url_for(
+            'main.send_one_off_step',
+            service_id=service_id,
+            template_id=template.id,
+            step_index=0,
+        )
 
     else:
         return url_for(

app/templates/views/service-settings/email-reply-to/_verify-updates.html

@@ -53,7 +53,11 @@
         width='2-3',
         safe_error_message=True
       ) }}
-      {% if not first_email_address %}
+      {% if form.is_default.data %}
+        <p class="form-group">
+          This is the default reply-to address for {{ current_service.name }} emails
+        </p>
+      {% else %}
         <div class="form-group">
           {{ checkbox(form.is_default) }}
         </div>

tests/app/main/views/test_send.py

@@ -380,7 +380,7 @@ def test_shows_error_if_parsing_exception(
     )
 
 
-def test_upload_csvfile_with_errors_shows_check_page_with_errors(
+def test_upload_csv_file_with_errors_shows_check_page_with_errors(
     logged_in_client,
     service_one,
     mocker,
@@ -507,7 +507,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
         )
     ),
 ])
-def test_upload_csvfile_with_missing_columns_shows_error(
+def test_upload_csv_file_with_missing_columns_shows_error(
     client_request,
     mocker,
     mock_get_service_template_with_placeholders,
@@ -1126,7 +1126,7 @@ def test_send_one_off_or_test_has_correct_page_titles(
 @pytest.mark.parametrize('endpoint, step_index, prefilled, expected_field_label', [
     (
         'main.send_test_step',
-        0,
+        1,
         {'phone number': '07900900123'},
         'one',
     ),
@@ -1138,7 +1138,7 @@ def test_send_one_off_or_test_has_correct_page_titles(
     ),
     (
         'main.send_test_step',
-        1,
+        2,
         {'phone number': '07900900123', 'one': 'foo'},
         'two',
     ),
@@ -1150,7 +1150,7 @@ def test_send_one_off_or_test_has_correct_page_titles(
     ),
     (
         'main.send_test_step',
-        2,
+        3,
         {'phone number': '07900900123', 'one': 'foo', 'two': 'foo'},
         'three',
     ),
@@ -1809,7 +1809,7 @@ def test_send_test_indicates_optional_address_columns(
     )
 
     assert normalize_spaces(page.select('label')[0].text) == (
-        'address line 4 '  # step_index is 0-indexed so step 3 is address line 4
+        'address line 3 '
         'Optional'
     )
     assert page.select('.govuk-back-link')[0]['href'] == url_for(