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 :)
This commit is contained in:
Pea Tyczynska
2019-05-28 17:44:27 +01:00
parent 378fec281a
commit 21ab8638b9
2 changed files with 24 additions and 14 deletions

View File

@@ -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(

View File

@@ -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,