Fix ‘help’ appearing when it shouldn’t

Steps to reproduce:
- make a template with a placeholder
- click ‘send yourself a test’
- leave fields blank
- click ‘check’
- see error, click ‘back’

Expected: previous page

Actual: previous page with blue help sidebar

When the URL contains `help=0`, `request.args.get('help')` returns '0'.
Doing `if '0':` is the same as doing any `if <non empty string>:` which
returns `True`.

So we should only display the help when the help query parameter is:

- not missing
- AND a string that isn’t `'0'`
This commit is contained in:
Chris Hill-Scott
2016-06-24 10:15:20 +01:00
parent 68cb76a12e
commit ceef77b2af
2 changed files with 29 additions and 6 deletions

View File

@@ -243,7 +243,7 @@ def check_messages(service_id, template_type, upload_id):
) )
if request.args.get('from_test') and len(template.placeholders): if request.args.get('from_test') and len(template.placeholders):
extra_args = {'help': 1} if request.args.get('help') else {} extra_args = {'help': 1} if request.args.get('help', '0') != '0' else {}
back_link = url_for( back_link = url_for(
'.send_test', service_id=service_id, template_id=template.id, **extra_args '.send_test', service_id=service_id, template_id=template.id, **extra_args
) )

View File

@@ -5,6 +5,7 @@ from io import BytesIO
from os import path from os import path
from glob import glob from glob import glob
from bs4 import BeautifulSoup from bs4 import BeautifulSoup
from functools import partial
from flask import url_for from flask import url_for
from tests import validate_route_permission from tests import validate_route_permission
from datetime import datetime from datetime import datetime
@@ -630,6 +631,23 @@ def test_check_messages_back_link_with_help(app_,
assert url_for('.send_test', service_id=fake_uuid, template_id=fake_uuid, help=1) in content assert url_for('.send_test', service_id=fake_uuid, template_id=fake_uuid, help=1) in content
@pytest.mark.parametrize(
'extra_args,expected_url',
[
(
dict(),
partial(url_for, '.send_test')
),
(
dict(help='0'),
partial(url_for, '.send_test')
),
(
dict(help='2'),
partial(url_for, '.send_test', help='1')
)
]
)
def test_check_messages_back_link_without_help(app_, def test_check_messages_back_link_without_help(app_,
api_user_active, api_user_active,
mock_login, mock_login,
@@ -640,7 +658,9 @@ def test_check_messages_back_link_without_help(app_,
mock_has_permissions, mock_has_permissions,
mock_get_service_statistics_for_day, mock_get_service_statistics_for_day,
mock_s3_download, mock_s3_download,
fake_uuid): fake_uuid,
extra_args,
expected_url):
with app_.test_request_context(): with app_.test_request_context():
with app_.test_client() as client: with app_.test_client() as client:
client.login(api_user_active) client.login(api_user_active)
@@ -654,11 +674,14 @@ def test_check_messages_back_link_without_help(app_,
service_id=fake_uuid, service_id=fake_uuid,
upload_id=fake_uuid, upload_id=fake_uuid,
template_type='sms', template_type='sms',
from_test=True) from_test=True,
) **extra_args
))
assert response.status_code == 200 assert response.status_code == 200
content = response.get_data(as_text=True) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert url_for('.send_test', service_id=fake_uuid, template_id=fake_uuid) in content assert (
page.findAll('a', {'class': 'page-footer-back-link'})[0]['href']
) == expected_url(service_id=fake_uuid, template_id=fake_uuid)
def test_send_and_check_page_renders_if_no_statistics( def test_send_and_check_page_renders_if_no_statistics(