From eabd3f551a4ae66a50f33e51a8d2d99574f12028 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 14 Jun 2017 15:37:57 +0100 Subject: [PATCH] =?UTF-8?q?Add=20tests=20for=20getting=20a=20user=E2=80=99?= =?UTF-8?q?s=20phone=20number?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also makes the code a bit more specific by not squashing any exception, only `HTTPError`s. --- app/main/views/conversation.py | 3 +- tests/app/main/views/test_conversation.py | 59 +++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index ffbda324f..78e3ee632 100644 --- a/app/main/views/conversation.py +++ b/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//conversation/") @@ -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) diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index 26f845b19..3a30d6a3c 100644 --- a/tests/app/main/views/test_conversation.py +++ b/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([