Merge pull request #1516 from alphagov/empty-inbound-sms

make sure inbound sms page doesn't break if you receive a blank inbou…

app/config.py

@@ -107,7 +107,7 @@ class Development(Config):
 class Test(Development):
     DEBUG = True
     TESTING = True
-    STATSD_ENABLED = True
+    STATSD_ENABLED = False
     WTF_CSRF_ENABLED = False
     CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test'

app/main/views/conversation.py

@@ -64,7 +64,7 @@ def get_sms_thread(service_id, user_number):
     ), key=lambda notification: notification['created_at']):
 
         is_inbound = ('notify_number' in notification)
-        redact_personalisation = notification.get('template', {}).get('redact_personalisation', False)
+        redact_personalisation = not is_inbound and notification['template']['redact_personalisation']
 
         if redact_personalisation:
             notification['personalisation'] = {}
@@ -73,7 +73,10 @@ def get_sms_thread(service_id, user_number):
             'inbound': is_inbound,
             'content': SMSPreviewTemplate(
                 {
-                    'content': notification.get('content') or notification['template']['content']
+                    'content': (
+                        notification['content'] if is_inbound else
+                        notification['template']['content']
+                    )
                 },
                 notification.get('personalisation'),
                 downgrade_non_gsm_characters=(not is_inbound),

tests/app/main/views/test_conversation.py

@@ -1,19 +1,19 @@
+from datetime import datetime
 import json
 import pytest
-from bs4 import BeautifulSoup
 
+from bs4 import BeautifulSoup
 from flask import (
     url_for,
 )
 from notifications_python_client.errors import HTTPError
+from freezegun import freeze_time
+
 from tests.conftest import (
     SERVICE_ONE_ID,
     normalize_spaces,
     mock_get_notifications,
-    mock_get_inbound_sms,
 )
-from freezegun import freeze_time
-from unittest import mock
 from app.main.views.conversation import get_user_number
 
 
@@ -22,7 +22,8 @@ def test_get_user_phone_number_when_only_inbound_exists(mocker):
     mock_get_inbound_sms = mocker.patch(
         'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
         return_value={
-            'user_number': '4407900900123'
+            'user_number': '4407900900123',
+            'notify_number': '07900000002'
         }
     )
     mock_get_notification = mocker.patch(
@@ -78,6 +79,7 @@ def test_view_conversation(
     fake_uuid,
     outbound_redacted,
     expected_outbound_content,
+    mock_get_inbound_sms
 ):
 
     mock_get_notifications(
@@ -88,10 +90,6 @@ def test_view_conversation(
         redact_personalisation=outbound_redacted,
     )
 
-    mock_get_inbound_sms(
-        mocker
-    )
-
     page = client_request.get(
         'main.conversation',
         service_id=SERVICE_ONE_ID,
@@ -107,35 +105,35 @@ def test_view_conversation(
     for index, expected in enumerate([
         (
             'message-8',
-            'Failed (sent yesterday at 2:59pm)',
+            'yesterday at 2:59pm',
         ),
         (
             'message-7',
-            'Failed (sent yesterday at 2:59pm)',
+            'yesterday at 2:59pm',
         ),
         (
             'message-6',
-            'Failed (sent yesterday at 4:59pm)',
+            'yesterday at 4:59pm',
         ),
         (
             'message-5',
-            'Failed (sent yesterday at 6:59pm)',
+            'yesterday at 6:59pm',
         ),
         (
             'message-4',
-            'Failed (sent yesterday at 8:59pm)',
+            'yesterday at 8:59pm',
         ),
         (
             'message-3',
-            'Failed (sent yesterday at 10:59pm)',
+            'yesterday at 10:59pm',
         ),
         (
             'message-2',
-            'Failed (sent yesterday at 10:59pm)',
+            'yesterday at 10:59pm',
         ),
         (
             'message-1',
-            'Failed (sent yesterday at 11:00pm)',
+            'yesterday at 11:00pm',
         ),
         (
             expected_outbound_content,
@@ -186,3 +184,33 @@ def test_view_conversation_updates(
     assert json.loads(response.get_data(as_text=True)) == {'messages': 'foo'}
 
     mock_get_partials.assert_called_once_with(SERVICE_ONE_ID, '07123 456789')
+
+
+@freeze_time("2012-01-01 00:00:00")
+def test_view_conversation_with_empty_inbound(
+    client_request,
+    mocker,
+    api_user_active,
+    mock_get_notification,
+    mock_get_notifications_with_no_notifications,
+    fake_uuid
+):
+    mock_get_inbound_sms = mocker.patch(
+        'app.main.views.conversation.service_api_client.get_inbound_sms',
+        return_value=[{
+            'user_number': '07900000001',
+            'notify_number': '07900000002',
+            'content': '',
+            'created_at': datetime.utcnow().isoformat(),
+            'id': fake_uuid
+        }]
+    )
+
+    page = client_request.get(
+        'main.conversation',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+    )
+
+    messages = page.select('.sms-message-wrapper')
+    assert len(messages) == 1

tests/conftest.py

@@ -1333,6 +1333,7 @@ def mock_get_inbound_sms(mocker):
     ):
         return [{
             'user_number': '0790090000' + str(i),
+            'notify_number': '07900000002',
             'content': 'message-{}'.format(index + 1),
             'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1), seconds=index)).isoformat(),
             'id': sample_uuid(),