From 049daa0cb8960c465471745b30cc314524c38179 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Sat, 25 Nov 2017 11:31:36 +0000 Subject: [PATCH] Save reply_to_text for one off notiications and csv notificaitons. --- app/celery/tasks.py | 6 +- app/service/send_notification.py | 32 +++++++-- tests/app/celery/test_tasks.py | 43 ++++++++++- tests/app/db.py | 3 +- .../test_process_notification.py | 17 +++++ tests/app/service/test_rest.py | 10 +-- .../service/test_send_one_off_notification.py | 72 +++++++++++-------- 7 files changed, 140 insertions(+), 43 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 436def14d..67d553fcd 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -213,7 +213,8 @@ def save_sms(self, created_at=datetime.utcnow(), job_id=notification.get('job', None), job_row_number=notification.get('row_number', None), - notification_id=notification_id + notification_id=notification_id, + reply_to_text=service.get_default_sms_sender() ) provider_tasks.deliver_sms.apply_async( @@ -260,7 +261,8 @@ def save_email(self, created_at=datetime.utcnow(), job_id=notification.get('job', None), job_row_number=notification.get('row_number', None), - notification_id=notification_id + notification_id=notification_id, + reply_to_text=service.get_default_reply_to_email_address() ) provider_tasks.deliver_email.apply_async( diff --git a/app/service/send_notification.py b/app/service/send_notification.py index fa6dd1bb2..068331167 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -1,8 +1,10 @@ from app.config import QueueNames +from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id +from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id from app.notifications.validators import ( check_service_over_daily_message_limit, validate_and_format_recipient, - validate_template) + validate_template, check_service_sms_sender_id) from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -13,7 +15,7 @@ from app.models import ( KEY_TYPE_NORMAL, PRIORITY, SMS_TYPE, - EMAIL_TYPE) + EMAIL_TYPE, LETTER_TYPE) from app.dao.services_dao import dao_fetch_service_by_id from app.dao.templates_dao import dao_get_template_by_id_and_service_id from app.dao.users_dao import get_user_by_id @@ -52,6 +54,8 @@ def send_one_off_notification(service_id, post_data): validate_created_by(service, post_data['created_by']) + sender_id = post_data.get('sender_id', None) + reply_to = get_reply_to_text(notification_type=template.template_type, sender_id=sender_id, service=service) notification = persist_notification( template_id=template.id, template_version=template.version, @@ -61,9 +65,9 @@ def send_one_off_notification(service_id, post_data): notification_type=template.template_type, api_key_id=None, key_type=KEY_TYPE_NORMAL, - created_by_id=post_data['created_by'] + created_by_id=post_data['created_by'], + reply_to_text=reply_to ) - sender_id = post_data.get('sender_id', None) if sender_id: if template.template_type == EMAIL_TYPE: persist_email_reply_to_id_for_notification(notification.id, sender_id) @@ -78,3 +82,23 @@ def send_one_off_notification(service_id, post_data): ) return {'id': str(notification.id)} + + +def get_reply_to_text(notification_type, sender_id, service): + reply_to = None + if notification_type == EMAIL_TYPE: + if sender_id: + reply_to = dao_get_reply_to_by_id(service.id, sender_id).email_address + else: + service.get_default_reply_to_email_address() + + elif notification_type == SMS_TYPE: + if sender_id: + reply_to = dao_get_service_sms_senders_by_id(service.id, sender_id).sms_sender + else: + reply_to = service.get_default_sms_sender() + + elif notification_type == LETTER_TYPE: + reply_to = service.get_default_letter_contact() + + return reply_to diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index b3a046897..2014a8fc0 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -61,8 +61,8 @@ from tests.app.db import ( create_service_inbound_api, create_service, create_template, - create_user -) + create_user, + create_reply_to_email, create_service_sms_sender, create_service_with_defined_sms_sender) class AnyStringWith(str): @@ -541,6 +541,45 @@ def test_should_save_email_if_restricted_service_and_non_team_email_address_with ) +def test_save_email_should_save_default_email_reply_to_text_on_notification(notify_db_session, mocker): + service = create_service() + create_reply_to_email(service=service, email_address='reply_to@digital.gov.uk', is_default=True) + template = create_template(service=service, template_type='email', subject='Hello') + + notification = _notification_json(template, to="test@example.com") + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + + notification_id = uuid.uuid4() + save_email( + service.id, + notification_id, + encryption.encrypt(notification), + key_type=KEY_TYPE_TEST + ) + + persisted_notification = Notification.query.one() + assert persisted_notification.reply_to_text == 'reply_to@digital.gov.uk' + + +def test_save_sms_should_save_default_smm_sender_notification_reply_to_text_on(notify_db_session, mocker): + service = create_service_with_defined_sms_sender(sms_sender_value='12345') + template = create_template(service=service) + + notification = _notification_json(template, to="07700 900205") + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + + notification_id = uuid.uuid4() + save_sms( + service.id, + notification_id, + encryption.encrypt(notification), + key_type=KEY_TYPE_TEST + ) + + persisted_notification = Notification.query.one() + assert persisted_notification.reply_to_text == '12345' + + def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): user = create_user(mobile_number="07700 900205") service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) diff --git a/tests/app/db.py b/tests/app/db.py index 0457b3c0a..687b456c0 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -69,10 +69,11 @@ def create_service( active=True, email_from=None, prefix_sms=True, + message_limit=1000 ): service = Service( name=service_name, - message_limit=1000, + message_limit=message_limit, restricted=restricted, email_from=email_from if email_from else service_name.lower().replace(' ', '.'), created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())), diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index ff9521906..7c6a88914 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -244,6 +244,23 @@ def test_persist_notification_increments_cache_if_key_exists(sample_template, sa sample_template.id) +def test_persist_notification_saves_reply_to_text(sample_template): + service_default_sms_sender = sample_template.service.get_default_sms_sender() + persist_notification(template_id=sample_template.id, + template_version=1, + recipient="+447111111122", + personalisation=None, + service=sample_template.service, + notification_type='sms', + api_key_id=None, + key_type='normal', + reply_to_text=service_default_sms_sender + ) + + notification = Notification.query.one() + assert notification.reply_to_text == service_default_sms_sender + + @pytest.mark.parametrize('research_mode, requested_queue, expected_queue, notification_type, key_type', [(True, None, 'research-mode-tasks', 'sms', 'normal'), (True, None, 'research-mode-tasks', 'email', 'normal'), diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 05a1d8734..c47f5ddce 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2348,16 +2348,18 @@ def test_fetch_service_inbound_api(client, sample_service): assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize() -def test_send_one_off_notification(admin_request, sample_template, mocker): +def test_send_one_off_notification(admin_request, mocker): + service = create_service() + template = create_template(service=service) mocker.patch('app.service.send_notification.send_notification_to_queue') response = admin_request.post( 'service.create_one_off_notification', - service_id=sample_template.service_id, + service_id=service.id, _data={ - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': '07700900001', - 'created_by': str(sample_template.service.created_by_id) + 'created_by': str(service.created_by_id) }, _expected_status=201 ) diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 64650434c..67500bf8f 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -17,7 +17,13 @@ from app.models import ( NotificationSmsSender ) -from tests.app.db import create_user, create_reply_to_email, create_service_sms_sender +from tests.app.db import ( + create_user, + create_reply_to_email, + create_service_sms_sender, + create_service, + create_template +) @pytest.fixture @@ -31,11 +37,14 @@ def celery_mock(mocker): return mocker.patch('app.service.send_notification.send_notification_to_queue') -def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_mock, sample_template): - service = sample_template.service +def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_mock, notify_db_session): + service = create_service() + template = create_template(service=service) + + service = template.service post_data = { - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': '07700 900 001', 'created_by': str(service.created_by_id) } @@ -56,10 +65,10 @@ def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_m def test_send_one_off_notification_calls_persist_correctly( persist_mock, celery_mock, - sample_template_with_placeholders + notify_db_session ): - template = sample_template_with_placeholders - service = template.service + service = create_service() + template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon") post_data = { 'template_id': str(template.id), @@ -79,16 +88,17 @@ def test_send_one_off_notification_calls_persist_correctly( notification_type=SMS_TYPE, api_key_id=None, key_type=KEY_TYPE_NORMAL, - created_by_id=str(service.created_by_id) + created_by_id=str(service.created_by_id), + reply_to_text='testing' ) -def test_send_one_off_notification_honors_research_mode(persist_mock, celery_mock, sample_template): - service = sample_template.service - service.research_mode = True +def test_send_one_off_notification_honors_research_mode(notify_db_session, persist_mock, celery_mock): + service = create_service(research_mode=True) + template = create_template(service=service) post_data = { - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': '07700 900 001', 'created_by': str(service.created_by_id) } @@ -98,12 +108,13 @@ def test_send_one_off_notification_honors_research_mode(persist_mock, celery_moc assert celery_mock.call_args[1]['research_mode'] is True -def test_send_one_off_notification_honors_priority(persist_mock, celery_mock, sample_template): - service = sample_template.service - sample_template.process_type = PRIORITY +def test_send_one_off_notification_honors_priority(notify_db_session, persist_mock, celery_mock): + service = create_service() + template = create_template(service=service) + template.process_type = PRIORITY post_data = { - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': '07700 900 001', 'created_by': str(service.created_by_id) } @@ -113,11 +124,12 @@ def test_send_one_off_notification_honors_priority(persist_mock, celery_mock, sa assert celery_mock.call_args[1]['queue'] == QueueNames.PRIORITY -def test_send_one_off_notification_raises_if_invalid_recipient(sample_template): - service = sample_template.service +def test_send_one_off_notification_raises_if_invalid_recipient(notify_db_session): + service = create_service() + template = create_template(service=service) post_data = { - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': 'not a phone number', 'created_by': str(service.created_by_id) } @@ -126,12 +138,12 @@ def test_send_one_off_notification_raises_if_invalid_recipient(sample_template): send_one_off_notification(service.id, post_data) -def test_send_one_off_notification_raises_if_cant_send_to_recipient(sample_template): - service = sample_template.service - service.restricted = True +def test_send_one_off_notification_raises_if_cant_send_to_recipient(notify_db_session): + service = create_service(restricted=True) + template = create_template(service=service) post_data = { - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': '07700 900 001', 'created_by': str(service.created_by_id) } @@ -142,12 +154,12 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient(sample_templ assert 'service is in trial mode' in e.value.message -def test_send_one_off_notification_raises_if_over_limit(sample_template): - service = sample_template.service - service.message_limit = 0 +def test_send_one_off_notification_raises_if_over_limit(notify_db_session): + service = create_service(message_limit=0) + template = create_template(service=service) post_data = { - 'template_id': str(sample_template.id), + 'template_id': str(template.id), 'to': '07700 900 001', 'created_by': str(service.created_by_id) } @@ -156,9 +168,9 @@ def test_send_one_off_notification_raises_if_over_limit(sample_template): send_one_off_notification(service.id, post_data) -def test_send_one_off_notification_raises_if_message_too_long(persist_mock, sample_template_with_placeholders): - template = sample_template_with_placeholders - service = template.service +def test_send_one_off_notification_raises_if_message_too_long(persist_mock, notify_db_session): + service = create_service() + template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon") post_data = { 'template_id': str(template.id),