From 57a8f8d7faa1d0afb6e27a979f1c3ba89ce37cf3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 28 Nov 2016 15:49:29 +0000 Subject: [PATCH] The body of the content in the response to a POST v2/notifications was not replacing the placeholders. This PR fixes that and adds a test for it. I am confused as to why I had to change the test_validators test that is checking if the mock is called. Why did this code pass on preview? --- app/v2/notifications/post_notifications.py | 4 +- tests/app/notifications/test_validators.py | 4 +- .../notifications/test_post_notifications.py | 39 ++++++++++--------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index df7f8edf1..4f63c299d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -42,7 +42,7 @@ def post_sms_notification(): send_notification_to_queue(notification, service.research_mode) resp = create_post_sms_response_from_notification(notification, - template_with_content.content, + template_with_content.replaced, service.sms_sender, request.url_root) return jsonify(resp), 201 @@ -70,7 +70,7 @@ def post_email_notification(): send_notification_to_queue(notification, service.research_mode) resp = create_post_email_response_from_notification(notification=notification, - content=template_with_content.content, + content=template_with_content.replaced, subject=template_with_content.subject, email_from=service.email_from, url_root=request.url_root) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 2e1974359..9d0701360 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -39,7 +39,7 @@ def test_exception_thown_by_redis_store_get_should_not_be_fatal( assert e.value.status_code == 429 assert e.value.message == 'Exceeded send limits (4) for today' assert e.value.fields == [] - app.notifications.validators.redis_store.set.assert_not_called() + assert app.notifications.validators.redis_store.set.called @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) @@ -61,7 +61,7 @@ def test_check_service_message_limit_in_cache_with_unrestricted_service_passes( mocker.patch('app.notifications.validators.redis_store.set') mocker.patch('app.notifications.validators.services_dao') assert not check_service_message_limit(key_type, sample_service) - app.notifications.validators.redis_store.set.assert_not_called() + assert not app.notifications.validators.redis_store.set.called assert not app.notifications.validators.services_dao.mock_calls diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index a2d99fb64..81bf3ff44 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -7,23 +7,23 @@ from tests import create_authorization_header @pytest.mark.parametrize("reference", [None, "reference_from_client"]) -def test_post_sms_notification_returns_201(notify_api, sample_template, mocker, reference): +def test_post_sms_notification_returns_201(notify_api, sample_template_with_placeholders, mocker, reference): with notify_api.test_request_context(): with notify_api.test_client() as client: mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'phone_number': '+447700900855', - 'template_id': str(sample_template.id) + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'} } if reference: data.update({"reference": reference}) - auth_header = create_authorization_header(service_id=sample_template.service_id) + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) response = client.post( path='/v2/notifications/sms', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 201 resp_json = json.loads(response.get_data(as_text=True)) notifications = Notification.query.all() @@ -31,12 +31,12 @@ def test_post_sms_notification_returns_201(notify_api, sample_template, mocker, notification_id = notifications[0].id assert resp_json['id'] == str(notification_id) assert resp_json['reference'] == reference - assert resp_json['content']['body'] == sample_template.content - assert resp_json['content']['from_number'] == sample_template.service.sms_sender + assert resp_json['content']['body'] == sample_template_with_placeholders.content.replace("(( Name))", "Jo") + assert resp_json['content']['from_number'] == sample_template_with_placeholders.service.sms_sender assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] - assert resp_json['template']['id'] == str(sample_template.id) - assert resp_json['template']['version'] == sample_template.version - assert 'v2/templates/{}'.format(sample_template.id) in resp_json['template']['uri'] + assert resp_json['template']['id'] == str(sample_template_with_placeholders.id) + assert resp_json['template']['version'] == sample_template_with_placeholders.version + assert 'v2/templates/{}'.format(sample_template_with_placeholders.id) in resp_json['template']['uri'] assert mocked.called @@ -108,15 +108,16 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s @pytest.mark.parametrize("reference", [None, "reference_from_client"]) -def test_post_email_notification_returns_201(client, sample_email_template, mocker, reference): +def test_post_email_notification_returns_201(client, sample_email_template_with_placeholders, mocker, reference): mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { - "email_address": sample_email_template.service.users[0].email_address, - "template_id": sample_email_template.id, + "email_address": sample_email_template_with_placeholders.service.users[0].email_address, + "template_id": sample_email_template_with_placeholders.id, + "personalisation": {"name": "Bob"} } if reference: data.update({"reference": reference}) - auth_header = create_authorization_header(service_id=sample_email_template.service_id) + auth_header = create_authorization_header(service_id=sample_email_template_with_placeholders.service_id) response = client.post( path="v2/notifications/email", data=json.dumps(data), @@ -127,13 +128,13 @@ def test_post_email_notification_returns_201(client, sample_email_template, mock assert resp_json['id'] == str(notification.id) assert resp_json['reference'] == reference assert notification.reference is None - assert resp_json['content']['body'] == sample_email_template.content - assert resp_json['content']['subject'] == sample_email_template.subject - assert resp_json['content']['from_email'] == sample_email_template.service.email_from + assert resp_json['content']['body'] == sample_email_template_with_placeholders.content.replace('((name))', 'Bob') + assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject + assert resp_json['content']['from_email'] == sample_email_template_with_placeholders.service.email_from assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri'] - assert resp_json['template']['id'] == str(sample_email_template.id) - assert resp_json['template']['version'] == sample_email_template.version - assert 'v2/templates/{}'.format(sample_email_template.id) in resp_json['template']['uri'] + assert resp_json['template']['id'] == str(sample_email_template_with_placeholders.id) + assert resp_json['template']['version'] == sample_email_template_with_placeholders.version + assert 'v2/templates/{}'.format(sample_email_template_with_placeholders.id) in resp_json['template']['uri'] assert mocked.called