From 236339435ca837e9a279a3a22cbf83f80b317db5 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 3 May 2018 15:40:24 +0100
Subject: [PATCH] conversations only looks for 404 errors from inbound sms

stops masking some 503s in tests
---
 app/main/views/conversation.py            |  4 +++-
 tests/app/main/views/test_conversation.py | 19 +++++++++++++++----
 tests/conftest.py                         | 14 ++++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py
index 1aa958ca3..2dac0478a 100644
--- a/app/main/views/conversation.py
+++ b/app/main/views/conversation.py
@@ -94,7 +94,9 @@ def get_conversation_partials(service_id, user_number):
 def get_user_number(service_id, notification_id):
     try:
         user_number = service_api_client.get_inbound_sms_by_id(service_id, notification_id)['user_number']
-    except HTTPError:
+    except HTTPError as e:
+        if e.status_code != 404:
+            raise
         user_number = notification_api_client.get_notification(service_id, notification_id)['to']
     return format_phone_number_human_readable(user_number)
 
diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py
index f5157bb60..ce55ad28e 100644
--- a/tests/app/main/views/test_conversation.py
+++ b/tests/app/main/views/test_conversation.py
@@ -1,5 +1,6 @@
 import json
 from datetime import datetime
+from unittest.mock import Mock
 
 import pytest
 from flask import url_for
@@ -25,7 +26,7 @@ def test_get_user_phone_number_when_only_inbound_exists(mocker):
     )
     mock_get_notification = mocker.patch(
         'app.main.views.conversation.notification_api_client.get_notification',
-        side_effect=HTTPError,
+        side_effect=HTTPError(response=Mock(status_code=404)),
     )
     assert get_user_number('service', 'notification') == '07900 900123'
     mock_get_inbound_sms.assert_called_once_with('service', 'notification')
@@ -35,7 +36,7 @@ def test_get_user_phone_number_when_only_inbound_exists(mocker):
 def test_get_user_phone_number_when_only_outbound_exists(mocker):
     mock_get_inbound_sms = mocker.patch(
         'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
-        side_effect=HTTPError,
+        side_effect=HTTPError(response=Mock(status_code=404)),
     )
     mock_get_notification = mocker.patch(
         'app.main.views.conversation.notification_api_client.get_notification',
@@ -51,11 +52,11 @@ def test_get_user_phone_number_when_only_outbound_exists(mocker):
 def test_get_user_phone_number_raises_if_both_api_requests_fail(mocker):
     mock_get_inbound_sms = mocker.patch(
         'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
-        side_effect=HTTPError,
+        side_effect=HTTPError(response=Mock(status_code=404)),
     )
     mock_get_notification = mocker.patch(
         'app.main.views.conversation.notification_api_client.get_notification',
-        side_effect=HTTPError,
+        side_effect=HTTPError(response=Mock(status_code=404)),
     )
     with pytest.raises(HTTPError):
         get_user_number('service', 'notification')
@@ -72,6 +73,7 @@ def test_view_conversation(
     client_request,
     mocker,
     api_user_active,
+    mock_get_inbound_sms_by_id_with_no_messages,
     mock_get_notification,
     fake_uuid,
     outbound_redacted,
@@ -158,6 +160,7 @@ def test_view_conversation(
             normalize_spaces(statuses[index].text),
         ) == expected
 
+    mock_get_inbound_sms.assert_called_once_with(SERVICE_ONE_ID, user_number='07123 456789')
     mock.assert_called_once_with(SERVICE_ONE_ID, to='07123 456789', template_type='sms')
 
 
@@ -165,9 +168,14 @@ def test_view_conversation_updates(
     logged_in_client,
     mocker,
     fake_uuid,
+    mock_get_inbound_sms_by_id_with_no_messages,
     mock_get_notification,
 ):
 
+    mocker.patch(
+        'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+        side_effect=HTTPError(response=Mock(status_code=404)),
+    )
     mock_get_partials = mocker.patch(
         'app.main.views.conversation.get_conversation_partials',
         return_value={'messages': 'foo'}
@@ -190,6 +198,7 @@ def test_view_conversation_with_empty_inbound(
     client_request,
     mocker,
     api_user_active,
+    mock_get_inbound_sms_by_id_with_no_messages,
     mock_get_notification,
     mock_get_notifications_with_no_notifications,
     fake_uuid
@@ -222,6 +231,7 @@ def test_view_conversation_with_empty_inbound(
 def test_conversation_links_to_reply(
     client_request,
     fake_uuid,
+    mock_get_inbound_sms_by_id_with_no_messages,
     mock_get_notification,
     mock_get_notifications,
     mock_get_inbound_sms,
@@ -271,6 +281,7 @@ def test_conversation_reply_shows_templates(
 def test_conversation_reply_redirects_with_phone_number_from_notification(
     client_request,
     fake_uuid,
+    mock_get_inbound_sms_by_id_with_no_messages,
     mock_get_notification,
     mock_get_service_template,
 ):
diff --git a/tests/conftest.py b/tests/conftest.py
index 93f2dbf44..29515db74 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1886,6 +1886,20 @@ def mock_get_inbound_sms(mocker):
     )
 
 
+@pytest.fixture
+def mock_get_inbound_sms_by_id_with_no_messages(mocker):
+    def _get_inbound_sms_by_id(
+        service_id,
+        notification_id
+    ):
+        raise HTTPError(response=Mock(status_code=404))
+
+    return mocker.patch(
+        'app.service_api_client.get_inbound_sms_by_id',
+        side_effect=_get_inbound_sms_by_id,
+    )
+
+
 @pytest.fixture(scope='function')
 def mock_get_most_recent_inbound_sms(mocker):
     def _get_most_recent_inbound_sms(