diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index 5542e25de..84b6528be 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -41,14 +41,20 @@ 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) + + if redact_personalisation: + notification['personalisation'] = {} yield { 'inbound': is_inbound, 'content': SMSPreviewTemplate( { - 'content': notification.get('content') or notification['body'] + 'content': notification.get('content') or notification['template']['content'] }, - downgrade_non_gsm_characters=(not is_inbound) + notification.get('personalisation'), + downgrade_non_gsm_characters=(not is_inbound), + redact_missing_personalisation=redact_personalisation, ), 'created_at': notification['created_at'], 'status': notification.get('status'), diff --git a/tests/__init__.py b/tests/__init__.py index 33a46087d..ab20273e6 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -254,7 +254,6 @@ def notification_json( 'notifications': [{ 'id': uuid.uuid4(), 'to': to, - 'body': template['content'], 'template': template, 'job': job_payload, 'sent_at': sent_at, diff --git a/tests/app/main/test_asset_fingerprinter.py b/tests/app/main/test_asset_fingerprinter.py index acdaa6fef..c2cbc5df7 100644 --- a/tests/app/main/test_asset_fingerprinter.py +++ b/tests/app/main/test_asset_fingerprinter.py @@ -6,9 +6,9 @@ from unittest import mock from app.asset_fingerprinter import AssetFingerprinter -@mock.patch.object(AssetFingerprinter, 'get_asset_file_contents') class TestAssetFingerprint(object): - def test_url_format(self, get_file_content_mock): + def test_url_format(self, mocker): + get_file_content_mock = mocker.patch.object(AssetFingerprinter, 'get_asset_file_contents') get_file_content_mock.return_value = """ body { font-family: nta; @@ -26,7 +26,8 @@ class TestAssetFingerprint(object): '/suppliers/static/application-ie6.css?418e6f4a6cdf1142e45c072ed3e1c90a' # noqa ) - def test_building_file_path(self, get_file_content_mock): + def test_building_file_path(self, mocker): + get_file_content_mock = mocker.patch.object(AssetFingerprinter, 'get_asset_file_contents') get_file_content_mock.return_value = """ document.write('Hello world!'); """ @@ -36,7 +37,8 @@ class TestAssetFingerprint(object): 'app/static/javascripts/application.js' ) - def test_hashes_are_consistent(self, get_file_content_mock): + def test_hashes_are_consistent(self, mocker): + get_file_content_mock = mocker.patch.object(AssetFingerprinter, 'get_asset_file_contents') get_file_content_mock.return_value = """ body { font-family: nta; @@ -49,8 +51,9 @@ class TestAssetFingerprint(object): ) def test_hashes_are_different_for_different_files( - self, get_file_content_mock + self, mocker ): + get_file_content_mock = mocker.patch.object(AssetFingerprinter, 'get_asset_file_contents') asset_fingerprinter = AssetFingerprinter() get_file_content_mock.return_value = """ body { @@ -66,7 +69,8 @@ class TestAssetFingerprint(object): js_hash != css_hash ) - def test_hash_gets_cached(self, get_file_content_mock): + def test_hash_gets_cached(self, mocker): + get_file_content_mock = mocker.patch.object(AssetFingerprinter, 'get_asset_file_contents') get_file_content_mock.return_value = """ body { font-family: nta; diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index 0408690d5..605054c32 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -7,85 +7,96 @@ from flask import ( from notifications_python_client.errors import HTTPError from tests.conftest import ( SERVICE_ONE_ID, + normalize_spaces, + mock_get_notifications, + mock_get_inbound_sms, ) -from tests.conftest 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, -): +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' + } + ) + mock_get_notification = mocker.patch( + 'app.main.views.conversation.notification_api_client.get_notification', + side_effect=HTTPError, + ) 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, -): +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, + ) + mock_get_notification = mocker.patch( + 'app.main.views.conversation.notification_api_client.get_notification', + return_value={ + 'to': '15550000000' + } + ) 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, -): +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, + ) + mock_get_notification = mocker.patch( + 'app.main.views.conversation.notification_api_client.get_notification', + side_effect=HTTPError, + ) 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('outbound_redacted, expected_outbound_content', [ + (True, 'Hello hidden'), + (False, 'Hello Jo'), +]) @freeze_time("2012-01-01 00:00:00") def test_view_conversation( - logged_in_client, - fake_uuid, + client_request, + mocker, + api_user_active, mock_get_notification, - mock_get_inbound_sms, - mock_get_notifications, + fake_uuid, + outbound_redacted, + expected_outbound_content, ): - response = logged_in_client.get(url_for( + mock_get_notifications( + mocker, + api_user_active, + template_content='Hello ((name))', + personalisation={'name': 'Jo'}, + redact_personalisation=outbound_redacted, + ) + + mock_get_inbound_sms( + mocker + ) + + page = client_request.get( 'main.conversation', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + _test_page_title=False, + ) messages = page.select('.sms-message-wrapper') statuses = page.select('.sms-message-status') @@ -127,23 +138,23 @@ def test_view_conversation( 'Failed (sent yesterday at 11:00pm)', ), ( - 'template content', + expected_outbound_content, 'yesterday at midnight', ), ( - 'template content', + expected_outbound_content, 'yesterday at midnight', ), ( - 'template content', + expected_outbound_content, 'yesterday at midnight', ), ( - 'template content', + expected_outbound_content, 'yesterday at midnight', ), ( - 'template content', + expected_outbound_content, 'yesterday at midnight', ), ]):