From ab150d876d2ca06cd332dbdfaafbb181dac3cf56 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Fri, 19 May 2017 17:12:44 +0100
Subject: [PATCH] Stop letting people enter empty SMS senders
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This shouldn’t be deployed until we’ve migrated the database to default
to GOVUK.
---
 app/main/forms.py                             |  1 +
 .../service-settings/set-sms-sender.html      | 12 ++-------
 tests/app/main/test_validators.py             |  2 +-
 tests/app/main/views/test_service_settings.py | 26 +++++++++++++++++++
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index a688c8bdb..430166fc7 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -492,6 +492,7 @@ class ServiceSmsSender(Form):
     sms_sender = StringField(
         'Text message sender',
         validators=[
+            DataRequired(message="Can’t be empty"),
             Length(max=11, message="Enter 11 characters or fewer")
         ]
     )
diff --git a/app/templates/views/service-settings/set-sms-sender.html b/app/templates/views/service-settings/set-sms-sender.html
index 19316b83e..b80d6d7cd 100644
--- a/app/templates/views/service-settings/set-sms-sender.html
+++ b/app/templates/views/service-settings/set-sms-sender.html
@@ -13,17 +13,9 @@
     This appears instead of a phone number when a user receives a
     text message from your service.
   <p>
-    If you leave this blank:
+    If you set this to ‘GOVUK’ each message will begin with
+    ‘{{ current_service.name }}:’.
   </p>
-  <ul class="list list-bullet">
-    <li>
-      your messages will be sent from 40604 (a shortcode that’s
-      reserved for government use)
-    </li>
-    <li>
-      each message will begin with ‘{{ current_service.name }}:’
-    </li>
-  </ul>
   <form method="post">
     {{ textbox(
       form.sms_sender,
diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py
index dd90b77eb..4818a1ee8 100644
--- a/tests/app/main/test_validators.py
+++ b/tests/app/main/test_validators.py
@@ -183,7 +183,7 @@ def test_sms_sender_form_validation(
 
     form.sms_sender.data = ''
     form.validate()
-    assert not form.errors
+    assert "Can’t be empty" == form.errors['sms_sender'][0]
 
     form.sms_sender.data = 'morethanelevenchars'
     form.validate()
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 9ac91dddd..d9ccb561f 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -615,6 +615,32 @@ def test_set_text_message_sender(
     )
 
 
+@pytest.mark.parametrize('content, expected_error', [
+    ("", "Can’t be empty"),
+    ("twelvecharss", "Enter 11 characters or fewer"),
+    (".", "Use letters and numbers only")
+])
+def test_set_text_message_sender_validation(
+    logged_in_client,
+    mock_update_service,
+    service_one,
+    mock_get_letter_organisations,
+    content,
+    expected_error,
+):
+    response = logged_in_client.post(url_for(
+        'main.service_set_sms_sender',
+        service_id=service_one['id']),
+        data={"sms_sender": content},
+        follow_redirects=True
+    )
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    assert response.status_code == 200
+    assert page.select(".error-message")[0].text.strip() == expected_error
+    assert not mock_update_service.called
+
+
 def test_if_sms_sender_set_then_form_populated(
     logged_in_client,
     service_one,