From 5cd3043fc524b2337f074cf301edac712f6c7f1c Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 21 Jun 2016 15:03:33 +0100 Subject: [PATCH 1/5] return replaced subject back from send_notification API --- app/notifications/rest.py | 2 +- .../rest/test_send_notification.py | 23 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 934f6e093..fc82668bc 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -365,7 +365,7 @@ def get_notification_return_data(notification_id, notification, template): } if template.template_type == 'email': - output.update({'subject': template.subject}) + output.update({'subject': template.replaced_subject}) return output diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 3413820e8..92078ded4 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -90,39 +90,40 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock @freeze_time("2016-01-01 11:09:00.061258") -def test_send_notification_with_placeholders_replaced(notify_api, sample_template_with_placeholders, mocker): +def test_send_notification_with_placeholders_replaced(notify_api, sample_email_template_with_placeholders, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.celery.tasks.send_email.apply_async') data = { - 'to': '+447700900855', - 'template': str(sample_template_with_placeholders.id), + 'to': 'ok@ok.com', + 'template': str(sample_email_template_with_placeholders.id), 'personalisation': { 'name': 'Jo' } } - auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service.id) + auth_header = create_authorization_header(service_id=sample_email_template_with_placeholders.service.id) response = client.post( - path='/notifications/sms', + path='/notifications/email', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) response_data = json.loads(response.data)['data'] notification_id = response_data['notification']['id'] - data.update({"template_version": sample_template_with_placeholders.version}) + data.update({"template_version": sample_email_template_with_placeholders.version}) - app.celery.tasks.send_sms.apply_async.assert_called_once_with( - (str(sample_template_with_placeholders.service.id), + app.celery.tasks.send_email.apply_async.assert_called_once_with( + (str(sample_email_template_with_placeholders.service.id), notification_id, ANY, "2016-01-01T11:09:00.061258"), - queue="sms" + queue="email" ) assert response.status_code == 201 - assert encryption.decrypt(app.celery.tasks.send_sms.apply_async.call_args[0][0][2]) == data + assert encryption.decrypt(app.celery.tasks.send_email.apply_async.call_args[0][0][2]) == data assert response_data['body'] == 'Hello Jo' + assert response_data['subject'] == 'Jo' def test_should_not_send_notification_for_archived_template(notify_api, sample_template): From c92138d5ab4790ba76bed6d7385873b0492532b2 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 21 Jun 2016 16:45:13 +0100 Subject: [PATCH 2/5] return replaced subject back from get notifications API --- app/schemas.py | 9 ++++++--- tests/app/notifications/test_rest.py | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 9ee71cf49..23e99fb5a 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -248,7 +248,7 @@ class SmsAdminNotificationSchema(SmsNotificationSchema): class NotificationStatusSchema(BaseSchema): - template = fields.Nested(TemplateSchema, only=["id", "name", "template_type", "content"], dump_only=True) + template = fields.Nested(TemplateSchema, only=["id", "name", "template_type", "content", "subject"], dump_only=True) job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True) personalisation = fields.Dict(required=False) @@ -267,10 +267,13 @@ class NotificationStatusSchema(BaseSchema): def handle_template_merge(self, in_data): if in_data.get('personalisation'): from notifications_utils.template import Template - merged = Template(in_data['template'], in_data['personalisation']).replaced - in_data['body'] = merged + template = Template(in_data['template'], in_data['personalisation']) + in_data['body'] = template.replaced + if in_data['template']['template_type'] == 'email': + in_data['subject'] = template.replaced_subject in_data.pop('personalisation', None) in_data['template'].pop('content', None) + in_data['template'].pop('subject', None) return in_data diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 277762f27..76157ccad 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1248,6 +1248,30 @@ def test_get_notification_by_id_returns_merged_template_content(notify_db, notification = json.loads(response.get_data(as_text=True))['data']['notification'] assert response.status_code == 200 assert notification['body'] == 'Hello world' + assert 'subject' not in notification + + +def test_get_notification_by_id_returns_merged_template_content_for_email( + notify_db, + notify_db_session, + notify_api, + sample_email_template_with_placeholders +): + sample_notification = create_sample_notification(notify_db, + notify_db_session, + template=sample_email_template_with_placeholders, + personalisation={"name": "world"}) + with notify_api.test_request_context(), notify_api.test_client() as client: + auth_header = create_authorization_header(service_id=sample_notification.service_id) + + response = client.get( + '/notifications/{}'.format(sample_notification.id), + headers=[auth_header]) + + notification = json.loads(response.get_data(as_text=True))['data']['notification'] + assert response.status_code == 200 + assert notification['body'] == 'Hello world' + assert notification['subject'] == 'world' def test_get_notifications_for_service_returns_merged_template_content(notify_api, From f65b86cfc9957c4f8b18193b7b6771dcf2b74378 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 23 Jun 2016 10:20:49 +0100 Subject: [PATCH 3/5] Body of notification without placeholder should have been present. --- app/schemas.py | 2 ++ tests/app/notifications/test_rest.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/app/schemas.py b/app/schemas.py index 23e99fb5a..ca1e2fb13 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -272,6 +272,8 @@ class NotificationStatusSchema(BaseSchema): if in_data['template']['template_type'] == 'email': in_data['subject'] = template.replaced_subject in_data.pop('personalisation', None) + else: + in_data['body'] = in_data['template']['content'] in_data['template'].pop('content', None) in_data['template'].pop('subject', None) return in_data diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index a7fc33593..41694789d 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -35,6 +35,7 @@ def test_get_notification_by_id(notify_api, sample_notification): assert notification['to'] == '+447700900855' assert notification['service'] == str(sample_notification.service_id) assert response.status_code == 200 + assert notification['body'] == "This is a template" # sample_template.content def test_get_notifications_empty_result(notify_api, sample_api_key): @@ -75,6 +76,7 @@ def test_get_all_notifications(notify_api, sample_notification): assert notifications['notifications'][0]['to'] == '+447700900855' assert notifications['notifications'][0]['service'] == str(sample_notification.service_id) + assert notifications['notifications'][0]['body'] == "This is a template" # sample_template.content assert response.status_code == 200 From 3423c0c44df24783b2efc89fcbd53d3de4d92a8e Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 23 Jun 2016 15:21:03 +0100 Subject: [PATCH 4/5] Added subject for email templates --- app/schemas.py | 2 ++ tests/app/notifications/test_rest.py | 37 +++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/app/schemas.py b/app/schemas.py index ca1e2fb13..bd45dca22 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -274,6 +274,8 @@ class NotificationStatusSchema(BaseSchema): in_data.pop('personalisation', None) else: in_data['body'] = in_data['template']['content'] + if in_data['template']['template_type'] == 'email': + in_data['subject'] = in_data['template']['subject'] in_data['template'].pop('content', None) in_data['template'].pop('subject', None) return in_data diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 41694789d..3890da134 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -13,7 +13,7 @@ from tests.app.conftest import sample_job as create_sample_job from tests.app.conftest import sample_service as create_sample_service -def test_get_notification_by_id(notify_api, sample_notification): +def test_get_sms_notification_by_id(notify_api, sample_notification): with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header(service_id=sample_notification.service_id) @@ -36,6 +36,41 @@ def test_get_notification_by_id(notify_api, sample_notification): assert notification['service'] == str(sample_notification.service_id) assert response.status_code == 200 assert notification['body'] == "This is a template" # sample_template.content + assert not notification.get('subject') + + +def test_get_email_notification_by_id(notify_api, notify_db, notify_db_session, sample_email_template): + + email_notification = create_sample_notification(notify_db, + notify_db_session, + service=sample_email_template.service, + template=sample_email_template) + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header(service_id=email_notification.service_id) + + response = client.get( + '/notifications/{}'.format(email_notification.id), + headers=[auth_header]) + + notification = json.loads(response.get_data(as_text=True))['data']['notification'] + + assert notification['status'] == 'sending' + assert notification['template'] == { + 'id': str(email_notification.template.id), + 'name': email_notification.template.name, + 'template_type': email_notification.template.template_type} + assert notification['job'] == { + 'id': str(email_notification.job.id), + 'original_file_name': email_notification.job.original_file_name + } + + assert notification['to'] == '+447700900855' + assert notification['service'] == str(email_notification.service_id) + assert response.status_code == 200 + assert notification['body'] == sample_email_template.content + assert notification['subject'] == sample_email_template.subject def test_get_notifications_empty_result(notify_api, sample_api_key): From 0e2e99f64e8aeb7df5b7ca2e25cf7c640ecb2840 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 23 Jun 2016 15:35:35 +0100 Subject: [PATCH 5/5] notifications_utils Template constructor accepts None for personalisation data therefore None check not needed. If personalisation is None in in db it will get passed through to template which returns content. --- app/schemas.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index bd45dca22..0a7eab8d1 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -259,23 +259,17 @@ class NotificationStatusSchema(BaseSchema): @pre_dump def handle_personalisation_property(self, in_data): - if in_data.personalisation: - self.personalisation = in_data.personalisation + self.personalisation = in_data.personalisation return in_data @post_dump def handle_template_merge(self, in_data): - if in_data.get('personalisation'): - from notifications_utils.template import Template - template = Template(in_data['template'], in_data['personalisation']) - in_data['body'] = template.replaced - if in_data['template']['template_type'] == 'email': - in_data['subject'] = template.replaced_subject - in_data.pop('personalisation', None) - else: - in_data['body'] = in_data['template']['content'] - if in_data['template']['template_type'] == 'email': - in_data['subject'] = in_data['template']['subject'] + from notifications_utils.template import Template + template = Template(in_data['template'], in_data['personalisation']) + in_data['body'] = template.replaced + if in_data['template']['template_type'] == 'email': + in_data['subject'] = template.replaced_subject + in_data.pop('personalisation', None) in_data['template'].pop('content', None) in_data['template'].pop('subject', None) return in_data