From b11653fe082eff4a022dc2ed1ecda4f1a2e28388 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 31 Jan 2018 10:26:37 +0000
Subject: [PATCH] Refactor to use validator class
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Using a separate validator class to check for appropriate characters in
a text message sender means that we’re not doing this validation in a
different way from the other checks (length and required). So the code
is cleaner.
---
 app/main/forms.py                             | 17 ++++++++++-------
 app/main/validators.py                        | 13 +++++++++++++
 tests/app/main/views/test_service_settings.py |  3 ++-
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index c7dd469c4..ba7dd619a 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -1,4 +1,3 @@
-import re
 import pytz
 import weakref
 
@@ -30,7 +29,14 @@ from wtforms.fields.html5 import EmailField, TelField, SearchField
 from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional)
 from flask_wtf.file import FileField as FileField_wtf, FileAllowed
 
-from app.main.validators import (Blacklist, CsvFileValidator, ValidGovEmail, NoCommasInPlaceHolders, OnlyGSMCharacters)
+from app.main.validators import (
+    Blacklist,
+    CsvFileValidator,
+    ValidGovEmail,
+    NoCommasInPlaceHolders,
+    OnlyGSMCharacters,
+    LettersAndNumbersOnly,
+)
 
 
 def get_time_value_and_label(future_time):
@@ -553,15 +559,12 @@ class ServiceSmsSenderForm(StripWhitespaceForm):
         'Text message sender',
         validators=[
             DataRequired(message="Can’t be empty"),
-            Length(max=11, message="Enter 11 characters or fewer")
+            Length(max=11, message="Enter 11 characters or fewer"),
+            LettersAndNumbersOnly(),
         ]
     )
     is_default = BooleanField("Make this text message sender the default")
 
-    def validate_sms_sender(self, field):
-        if field.data and not re.match(r'^[a-zA-Z0-9\s]+$', field.data):
-            raise ValidationError('Use letters and numbers only')
-
 
 class ServiceEditInboundNumberForm(StripWhitespaceForm):
     is_default = BooleanField("Make this text message sender the default")
diff --git a/app/main/validators.py b/app/main/validators.py
index 709274dda..d7bee1af4 100644
--- a/app/main/validators.py
+++ b/app/main/validators.py
@@ -1,3 +1,4 @@
+import re
 from wtforms import ValidationError
 from notifications_utils.field import Field
 from notifications_utils.gsm import get_non_gsm_compatible_characters
@@ -63,3 +64,15 @@ class OnlyGSMCharacters:
                     ('It' if len(non_gsm_characters) == 1 else 'They')
                 )
             )
+
+
+class LettersAndNumbersOnly:
+
+    regex = re.compile(r'^[a-zA-Z0-9\s]+$')
+
+    def __init__(self, message='Use letters and numbers only'):
+        self.message = message
+
+    def __call__(self, form, field):
+        if field.data and not re.match(self.regex, field.data):
+            raise ValidationError(self.message)
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index b87f3a572..cfe3b47e7 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -810,7 +810,8 @@ def test_incorrect_letter_contact_block_input(
 
 @pytest.mark.parametrize('sms_sender_input, expected_error', [
     ('', 'Can’t be empty'),
-    ('abcdefghijkhgkg', 'Enter 11 characters or fewer')
+    ('abcdefghijkhgkg', 'Enter 11 characters or fewer'),
+    (' ¯\_(ツ)_/¯ ', 'Use letters and numbers only'),
 ])
 def test_incorrect_sms_sender_input(
     sms_sender_input,