From 65bdc7f5a41a42d4a18887a117517b721206b733 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 26 Jul 2021 14:33:54 +0100 Subject: [PATCH 1/2] Refactor of test cases into parametrized tests No reason this shouldn't be parametrized. It will be neater, more extendable and give better error messages No functionality change, however I did slightly adapt one or two of the test cases (for example to change the 11 characters or fewer test to test on the boundary with 12 characters rather than many more). --- tests/app/main/test_validators.py | 58 ++++++++++++------------------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index 5798dd2d4..21fd64692 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -187,46 +187,34 @@ def test_if_string_contains_alphanumeric_characters_does_not_raise(string): MustContainAlphanumericCharacters()(None, _gen_mock_field(string)) +@pytest.mark.parametrize( + "sms_sender,error_expected,error_message", + [ + ('', True, 'Cannot be empty'), + ('22', True, 'Enter 3 characters or more'), + ('333', False, None), + ('elevenchars', False, None), # 11 chars + ('twelvecharas', True, 'Enter 11 characters or fewer'), # 12 chars + ('###', True, 'Use letters and numbers only'), + ('00111222333', True, 'Cannot start with 00'), + ('UK_GOV', False, None), # Underscores are allowed + ('UK.GOV', False, None), # Full stops are allowed + ] +) def test_sms_sender_form_validation( client, mock_get_user_by_email, + sms_sender, + error_expected, + error_message ): form = ServiceSmsSenderForm() + form.sms_sender.data = sms_sender - form.sms_sender.data = 'elevenchars' form.validate() - assert not form.errors - form.sms_sender.data = '' - form.validate() - assert "Cannot be empty" == form.errors['sms_sender'][0] - - form.sms_sender.data = 'morethanelevenchars' - form.validate() - assert "Enter 11 characters or fewer" == form.errors['sms_sender'][0] - - form.sms_sender.data = '###########' - form.validate() - assert 'Use letters and numbers only' == form.errors['sms_sender'][0] - - # Underscores are allowed - form.sms_sender.data = 'UK_GOV' - form.validate() - assert not form.errors - - # Full stops are allowed - form.sms_sender.data = 'UK.GOV' - form.validate() - assert not form.errors - - form.sms_sender.data = '22' - form.validate() - assert 'Enter 3 characters or more' == form.errors['sms_sender'][0] - - form.sms_sender.data = '333' - form.validate() - assert not form.errors - - form.sms_sender.data = '00111222333' - form.validate() - assert "Cannot start with 00" == form.errors['sms_sender'][0] + if error_expected: + assert form.errors + assert error_message == form.errors['sms_sender'][0] + else: + assert not form.errors From a6cac279579a7f6b873779ff841c96634c815421 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 26 Jul 2021 15:10:57 +0100 Subject: [PATCH 2/2] Allow straight single quote in sms sender names This is so we can allow the sender name 'UC' for DWP. Note, this is specifically only straight single quotes and not curly quotes or double quotes. Curly quotes are not supported in the GSM character set (https://en.wikipedia.org/wiki/GSM_03.38). There is currently no defined user ask to support double quotes in sms sender names. I have tested this by sending a message through both Firetext and MMG to make sure they both support the single quote character in SMS sender names. DWP also have had no particular issues using the SMS sender name with their existing system in the past either. --- app/main/forms.py | 4 ++-- app/main/validators.py | 4 ++-- tests/app/main/test_validators.py | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 90dd618be..b8af1c9dc 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -45,7 +45,7 @@ from app.main.validators import ( CommonlyUsedPassword, CsvFileValidator, DoesNotStartWithDoubleZero, - LettersNumbersFullStopsAndUnderscoresOnly, + LettersNumbersSingleQuotesFullStopsAndUnderscoresOnly, MustContainAlphanumericCharacters, NoCommasInPlaceHolders, NoEmbeddedImagesInSVG, @@ -1734,7 +1734,7 @@ class ServiceSmsSenderForm(StripWhitespaceForm): DataRequired(message="Cannot be empty"), Length(max=11, message="Enter 11 characters or fewer"), Length(min=3, message="Enter 3 characters or more"), - LettersNumbersFullStopsAndUnderscoresOnly(), + LettersNumbersSingleQuotesFullStopsAndUnderscoresOnly(), DoesNotStartWithDoubleZero(), ] ) diff --git a/app/main/validators.py b/app/main/validators.py index 9d09fbe0c..d3b267332 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -144,9 +144,9 @@ class BroadcastLength: ) -class LettersNumbersFullStopsAndUnderscoresOnly: +class LettersNumbersSingleQuotesFullStopsAndUnderscoresOnly: - regex = re.compile(r'^[a-zA-Z0-9\s\._]+$') + regex = re.compile(r"^[a-zA-Z0-9\s\._']+$") def __init__(self, message='Use letters and numbers only'): self.message = message diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index 21fd64692..9458dbd62 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -199,6 +199,7 @@ def test_if_string_contains_alphanumeric_characters_does_not_raise(string): ('00111222333', True, 'Cannot start with 00'), ('UK_GOV', False, None), # Underscores are allowed ('UK.GOV', False, None), # Full stops are allowed + ("'UC'", False, None), # Straight single quotes are allowed ] ) def test_sms_sender_form_validation(