From 46beece1583ccf6da6741f9526212d401d75f989 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 21 Nov 2016 17:32:36 +0000 Subject: [PATCH 1/2] For the post_sms_response and post_email_response the reference property is always present but the value can be null. Added a test for an empty reference. Remove datetime format on the created_at attribute of a notification, it is not needed. --- app/notifications/process_notifications.py | 2 +- app/v2/notifications/notification_schemas.py | 4 ++-- app/v2/notifications/post_notifications.py | 4 ++-- .../notifications/test_post_notifications.py | 20 +++++++++++-------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index b5ab8eb97..7e70c5abd 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -57,7 +57,7 @@ def persist_notification(template_id, notification_type=notification_type, api_key_id=api_key_id, key_type=key_type, - created_at=created_at or datetime.utcnow().strftime(DATETIME_FORMAT), + created_at=created_at or datetime.utcnow(), job_id=job_id, job_row_number=job_row_number, client_reference=reference diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index d77a4d21a..9f0155c02 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -47,7 +47,7 @@ post_sms_response = { "title": "response v2/notifications/sms", "properties": { "id": uuid, - "reference": {"type": "string"}, + "reference": {"type": ["string", "null"]}, "content": sms_content, "uri": {"type": "string"}, "template": template @@ -90,7 +90,7 @@ post_email_response = { "title": "response v2/notifications/email", "properties": { "id": uuid, - "reference": {"type": "string"}, + "reference": {"type": ["string", "null"]}, "content": email_content, "uri": {"type": "string"}, "template": template diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 07127b730..df7f8edf1 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -38,7 +38,7 @@ def post_sms_notification(): notification_type=SMS_TYPE, api_key_id=api_user.id, key_type=api_user.key_type, - reference=form['reference']) + reference=form.get('reference')) send_notification_to_queue(notification, service.research_mode) resp = create_post_sms_response_from_notification(notification, @@ -65,7 +65,7 @@ def post_email_notification(): notification_type=EMAIL_TYPE, api_key_id=api_user.id, key_type=api_user.key_type, - reference=form['reference']) + reference=form.get('reference')) send_notification_to_queue(notification, service.research_mode) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index a509d2a2f..e58e93dbf 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -6,15 +6,17 @@ from app.models import Notification from tests import create_authorization_header -def test_post_sms_notification_returns_201(notify_api, sample_template, mocker): +@pytest.mark.parametrize("reference", [None, "reference_from_client"]) +def test_post_sms_notification_returns_201(notify_api, sample_template, 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), - 'reference': 'reference_from_client' + 'template_id': str(sample_template.id) } + if reference: + data.update({"reference": reference}) auth_header = create_authorization_header(service_id=sample_template.service_id) response = client.post( @@ -27,8 +29,8 @@ def test_post_sms_notification_returns_201(notify_api, sample_template, mocker): notifications = Notification.query.all() assert len(notifications) == 1 notification_id = notifications[0].id - assert resp_json['id'] is not None - assert resp_json['reference'] == 'reference_from_client' + assert resp_json['id'] == 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 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] @@ -105,13 +107,15 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s }] -def test_post_email_notification_returns_201(client, sample_email_template, mocker): +@pytest.mark.parametrize("reference", [None, "reference_from_client"]) +def test_post_email_notification_returns_201(client, sample_email_template, mocker, reference): mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { - "reference": "reference from caller", "email_address": sample_email_template.service.users[0].email_address, "template_id": sample_email_template.id, } + if reference: + data.update({"reference": reference}) auth_header = create_authorization_header(service_id=sample_email_template.service_id) response = client.post( path="v2/notifications/email", @@ -121,7 +125,7 @@ def test_post_email_notification_returns_201(client, sample_email_template, mock resp_json = json.loads(response.get_data(as_text=True)) notification = Notification.query.first() assert resp_json['id'] == str(notification.id) - assert resp_json['reference'] == "reference from caller" + 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 From fa422a85ef0e5e2fbdb3a994edc1db435bc6ef08 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 22 Nov 2016 13:25:10 +0000 Subject: [PATCH 2/2] Fix unit test failure --- tests/app/v2/notifications/test_post_notifications.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index e58e93dbf..a2d99fb64 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -29,7 +29,7 @@ def test_post_sms_notification_returns_201(notify_api, sample_template, mocker, notifications = Notification.query.all() assert len(notifications) == 1 notification_id = notifications[0].id - assert resp_json['id'] == notification_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