darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-06 02:08:26 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1960 from alphagov/dont-validate-across-channels

Browse Source 
Don’t validate phone numbers when sending emails
This commit is contained in:
Chris Hill-Scott
2018-03-16 14:59:42 +00:00
committed by GitHub
parent ee3ef18ebf 5a2fafb66b
commit cd3910556c
3 changed files with 34 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
app/main/forms.py
View File

@@ -873,13 +873,20 @@ class SMSPrefixForm(StripWhitespaceForm):
def get_placeholder_form_instance( def get_placeholder_form_instance(
placeholder_name, placeholder_name,
dict_to_populate_from, dict_to_populate_from,
template_type,
optional_placeholder=False, optional_placeholder=False,
allow_international_phone_numbers=False, allow_international_phone_numbers=False,
): ):
if Columns.make_key(placeholder_name) == 'emailaddress': if (
Columns.make_key(placeholder_name) == 'emailaddress' and
template_type == 'email'
):
field = email_address(label=placeholder_name, gov_user=False) field = email_address(label=placeholder_name, gov_user=False)
elif Columns.make_key(placeholder_name) == 'phonenumber': elif (
Columns.make_key(placeholder_name) == 'phonenumber' and
template_type == 'sms'
):
if allow_international_phone_numbers: if allow_international_phone_numbers:
field = international_phone_number(label=placeholder_name) field = international_phone_number(label=placeholder_name)
else: else:

1
app/main/views/send.py
View File

@@ -379,6 +379,7 @@ def send_test_step(service_id, template_id, step_index):
form = get_placeholder_form_instance( form = get_placeholder_form_instance(
current_placeholder, current_placeholder,
dict_to_populate_from=get_normalised_placeholders_from_session(), dict_to_populate_from=get_normalised_placeholders_from_session(),
template_type=template.template_type,
optional_placeholder=optional_placeholder, optional_placeholder=optional_placeholder,
allow_international_phone_numbers='international_sms' in current_service['permissions'], allow_international_phone_numbers='international_sms' in current_service['permissions'],
) )

40
tests/app/main/test_placeholder_form.py
View File

@@ -9,8 +9,8 @@ def test_form_class_not_mutated(app_):
method='POST', method='POST',
data={'placeholder_value': ''} data={'placeholder_value': ''}
): ):
form1 = get_placeholder_form_instance('name', {}, optional_placeholder=False) form1 = get_placeholder_form_instance('name', {}, 'sms', optional_placeholder=False)
form2 = get_placeholder_form_instance('city', {}, optional_placeholder=True) form2 = get_placeholder_form_instance('city', {}, 'sms', optional_placeholder=True)
assert not form1.validate_on_submit() assert not form1.validate_on_submit()
assert form2.validate_on_submit() assert form2.validate_on_submit()
@@ -19,29 +19,36 @@ def test_form_class_not_mutated(app_):
assert str(form2.placeholder_value.label) == '<label for="placeholder_value">city</label>' assert str(form2.placeholder_value.label) == '<label for="placeholder_value">city</label>'
@pytest.mark.parametrize('service_can_send_international_sms, placeholder_name, value, expected_error', [ @pytest.mark.parametrize('service_can_send_international_sms, placeholder_name, template_type, value, expected_error', [
(False, 'email address', '', 'Can’t be empty'), (False, 'email address', 'email', '', 'Can’t be empty'),
(False, 'email address', '12345', 'Enter a valid email address'), (False, 'email address', 'email', '12345', 'Enter a valid email address'),
(False, 'email address', '“bad”@email-address.com', 'Enter a valid email address'), (False, 'email address', 'email', '“bad”@email-address.com', 'Enter a valid email address'),
(False, 'email address', 'test@example.com', None), (False, 'email address', 'email', 'test@example.com', None),
(False, 'email address', 'test@example.gov.uk', None), (False, 'email address', 'email', 'test@example.gov.uk', None),
(False, 'phone number', '', 'Can’t be empty'), (False, 'phone number', 'sms', '', 'Can’t be empty'),
(False, 'phone number', '+1-2345-678890', 'Not a UK mobile number'), (False, 'phone number', 'sms', '+1-2345-678890', 'Not a UK mobile number'),
(False, 'phone number', '07900900123', None), (False, 'phone number', 'sms', '07900900123', None),
(False, 'phone number', '+44(0)7900 900-123', None), (False, 'phone number', 'sms', '+44(0)7900 900-123', None),
(True, 'phone number', '+123', 'Not enough digits'), (True, 'phone number', 'sms', '+123', 'Not enough digits'),
(True, 'phone number', '+44(0)7900 900-123', None), (True, 'phone number', 'sms', '+44(0)7900 900-123', None),
(True, 'phone number', '+1-2345-678890', None), (True, 'phone number', 'sms', '+1-2345-678890', None),
(False, 'anything else', '', 'Can’t be empty'), (False, 'anything else', 'sms', '', 'Can’t be empty'),
(False, 'anything else', 'email', '', 'Can’t be empty'),
(True, 'phone number', 'sms', 'invalid', 'Must not contain letters or symbols'),
(True, 'phone number', 'email', 'invalid', None),
(True, 'phone number', 'letter', 'invalid', None),
(True, 'email address', 'sms', 'invalid', None),
]) ])
def test_validates_recipients( def test_validates_recipients(
app_, app_,
placeholder_name, placeholder_name,
template_type,
value, value,
service_can_send_international_sms, service_can_send_international_sms,
expected_error, expected_error,
@@ -53,6 +60,7 @@ def test_validates_recipients(
form = get_placeholder_form_instance( form = get_placeholder_form_instance(
placeholder_name, placeholder_name,
{}, {},
template_type,
allow_international_phone_numbers=service_can_send_international_sms, allow_international_phone_numbers=service_can_send_international_sms,
) )