Add tests for getting a user’s phone number

Also makes the code a bit more specific by not squashing any exception,
only `HTTPError`s.

app/main/views/conversation.py

@@ -8,6 +8,7 @@ from notifications_utils.template import SMSPreviewTemplate
 from app.main import main
 from app.utils import user_has_permissions
 from app import notification_api_client, service_api_client
+from notifications_python_client.errors import HTTPError
 
 
 @main.route("/services/<service_id>/conversation/<notification_id>")
@@ -27,7 +28,7 @@ def conversation(service_id, notification_id):
 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 Exception:
+    except HTTPError:
         user_number = notification_api_client.get_notification(service_id, notification_id)['to']
     return format_phone_number_human_readable(user_number)
 

tests/app/main/views/test_conversation.py

@@ -4,11 +4,70 @@ from bs4 import BeautifulSoup
 from flask import (
     url_for,
 )
+from notifications_python_client.errors import HTTPError
 from tests.conftest import (
     SERVICE_ONE_ID,
 )
 from tests.app.test_utils import normalize_spaces
 from freezegun import freeze_time
+from unittest import mock
+from app.main.views.conversation import get_user_number
+
+
+@mock.patch(
+    'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+    return_value={
+        'user_number': '4407900900123'
+    }
+)
+@mock.patch(
+    'app.main.views.conversation.notification_api_client.get_notification',
+    side_effect=HTTPError,
+)
+def test_get_user_phone_number_when_only_inbound_exists(
+    mock_get_notification,
+    mock_get_inbound_sms,
+):
+    assert get_user_number('service', 'notification') == '07900 900123'
+    mock_get_inbound_sms.assert_called_once_with('service', 'notification')
+    assert mock_get_notification.called is False
+
+
+@mock.patch(
+    'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+    side_effect=HTTPError,
+)
+@mock.patch(
+    'app.main.views.conversation.notification_api_client.get_notification',
+    return_value={
+        'to': '15550000000'
+    }
+)
+def test_get_user_phone_number_when_only_outbound_exists(
+    mock_get_notification,
+    mock_get_inbound_sms,
+):
+    assert get_user_number('service', 'notification') == '+1 555-000-0000'
+    mock_get_inbound_sms.assert_called_once_with('service', 'notification')
+    mock_get_notification.assert_called_once_with('service', 'notification')
+
+
+@mock.patch(
+    'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+    side_effect=HTTPError,
+)
+@mock.patch(
+    'app.main.views.conversation.notification_api_client.get_notification',
+    side_effect=HTTPError,
+)
+def test_get_user_phone_number_raises_if_both_API_requests_fail(
+    mock_get_notification,
+    mock_get_inbound_sms,
+):
+    with pytest.raises(HTTPError):
+        get_user_number('service', 'notification')
+    mock_get_inbound_sms.assert_called_once_with('service', 'notification')
+    mock_get_notification.assert_called_once_with('service', 'notification')
 
 
 @pytest.mark.parametrize('index, expected', enumerate([