From 21ab8638b9ea31b491de3b6db61d2a8d710c2c81 Mon Sep 17 00:00:00 2001
From: Pea Tyczynska <pea.tyczynska@digital.cabinet-office.gov.uk>
Date: Tue, 28 May 2019 17:44:27 +0100
Subject: [PATCH] Fix error where sending test message flow skipped first
 placeholder

first way round and then collected placeholders again. Now the flow
collects all placeholders in one round.

Also fix the back link for step-1 for test flow so it goes back
to choosing recipient number

Also move operators around following flake8's advice :)
---
 app/main/views/send.py            | 34 ++++++++++++++++++++-----------
 tests/app/main/views/test_send.py |  4 ++--
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/app/main/views/send.py b/app/main/views/send.py
index 1c2eb352f..ee7d733c8 100644
--- a/app/main/views/send.py
+++ b/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 "test" in request.endpoint:
+            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
-            template.template_type != 'letter' and
-            request.endpoint != 'main.send_test_step'
+            step_index == 0
+            and template.template_type != 'letter'
+            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
-        step_index == 0 and
-        template.template_type != 'letter' and
-        not (template.template_type == 'sms' and current_user.mobile_number is None) and
-        current_user.has_permissions('manage_templates', 'manage_service')
+        request.endpoint == 'main.send_one_off_step'
+        and step_index == 0
+        and template.template_type != 'letter'
+        and not (template.template_type == 'sms' and current_user.mobile_number is None)
+        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
-            step_index == 0
+            request.endpoint == 'main.send_one_off_step'
+            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(
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 5aa7a764d..e9aeeea04 100644
--- a/tests/app/main/views/test_send.py
+++ b/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,