From e3a75d1b7d750244e5c533c5ed4ff14f553e7a67 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 7 Mar 2018 18:13:40 +0000 Subject: [PATCH] Notification_type is a required parameter, admin app always passes it in. Normalise for notificaiton type. Throw InvalidRequest exception is the notification type is invalid. --- app/dao/notifications_dao.py | 32 +++++++++------- app/service/rest.py | 7 +++- .../notification_dao/test_notification_dao.py | 37 ++++++++++--------- tests/app/service/test_rest.py | 32 ++++++++-------- 4 files changed, 62 insertions(+), 46 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 46ee487a8..3e7e4ff12 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -8,10 +8,9 @@ from datetime import ( from flask import current_app from notifications_utils.recipients import ( - validate_and_format_phone_number, validate_and_format_email_address, - InvalidPhoneError, InvalidEmailError, + try_validate_and_format_phone_number ) from notifications_utils.statsd_decorators import statsd from werkzeug.datastructures import MultiDict @@ -23,6 +22,7 @@ from notifications_utils.international_billing_rates import INTERNATIONAL_BILLIN from app import db, create_uuid from app.dao import days_ago +from app.errors import InvalidRequest from app.models import ( Notification, NotificationHistory, @@ -40,7 +40,9 @@ from app.models import ( NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, - NOTIFICATION_SENT + NOTIFICATION_SENT, + SMS_TYPE, + EMAIL_TYPE ) from app.dao.dao_utils import transactional @@ -435,19 +437,23 @@ def dao_update_notifications_by_reference(references, update_dict): @statsd(namespace="dao") -def dao_get_notifications_by_to_field(service_id, search_term, statuses=None, notification_type=None): - try: - normalised = validate_and_format_phone_number(search_term) - except InvalidPhoneError: +def dao_get_notifications_by_to_field(service_id, search_term, notification_type, statuses=None): + + if notification_type == SMS_TYPE: + normalised = try_validate_and_format_phone_number(search_term) + + for character in {'(', ')', ' ', '-'}: + normalised = normalised.replace(character, '') + + normalised = normalised.lstrip('+0') + + elif notification_type == EMAIL_TYPE: try: normalised = validate_and_format_email_address(search_term) except InvalidEmailError: - normalised = search_term - - for character in ['(', ')', ' ']: - normalised = normalised.replace(character, '') - - normalised = normalised.lstrip('+0') + normalised = search_term.lower() + else: + raise InvalidRequest("Only email and SMS can use search by recipient", 400) filters = [ Notification.service_id == service_id, diff --git a/app/service/rest.py b/app/service/rest.py index 1ea69509f..5ed4a5dee 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -385,7 +385,12 @@ def get_notification_for_service(service_id, notification_id): def search_for_notification_by_to_field(service_id, search_term, statuses, notification_type): - results = notifications_dao.dao_get_notifications_by_to_field(service_id, search_term, statuses, notification_type) + results = notifications_dao.dao_get_notifications_by_to_field( + service_id=service_id, + search_term=search_term, + statuses=statuses, + notification_type=notification_type + ) return jsonify( notifications=notification_with_template_schema.dump(results, many=True).data ), 200 diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index a565c76ee..db9d999a3 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1733,32 +1733,35 @@ def test_dao_get_notifications_by_to_field(sample_template): results = dao_get_notifications_by_to_field( notification1.service_id, - recipient_to_search_for["to_field"] + recipient_to_search_for["to_field"], + notification_type='sms' ) assert len(results) == 1 assert notification1.id == results[0].id -def test_dao_get_notifications_by_to_field_search_is_not_case_sensitive(sample_template): +@pytest.mark.parametrize("search_term", + ["JACK", "JACK@gmail.com", "jack@gmail.com"]) +def test_dao_get_notifications_by_to_field_search_is_not_case_sensitive(sample_email_template, search_term): notification = create_notification( - template=sample_template, to_field='jack@gmail.com', normalised_to='jack@gmail.com' + template=sample_email_template, to_field='jack@gmail.com', normalised_to='jack@gmail.com' ) - results = dao_get_notifications_by_to_field(notification.service_id, 'JACK@gmail.com') + results = dao_get_notifications_by_to_field(notification.service_id, search_term, notification_type='email') notification_ids = [notification.id for notification in results] assert len(results) == 1 assert notification.id in notification_ids -def test_dao_get_notifications_by_to_field_matches_partial_emails(sample_template): +def test_dao_get_notifications_by_to_field_matches_partial_emails(sample_email_template): notification_1 = create_notification( - template=sample_template, to_field='jack@gmail.com', normalised_to='jack@gmail.com' + template=sample_email_template, to_field='jack@gmail.com', normalised_to='jack@gmail.com' ) notification_2 = create_notification( - template=sample_template, to_field='jacque@gmail.com', normalised_to='jacque@gmail.com' + template=sample_email_template, to_field='jacque@gmail.com', normalised_to='jacque@gmail.com' ) - results = dao_get_notifications_by_to_field(notification_1.service_id, 'ack') + results = dao_get_notifications_by_to_field(notification_1.service_id, 'ack', notification_type='email') notification_ids = [notification.id for notification in results] assert len(results) == 1 @@ -1793,7 +1796,7 @@ def test_dao_get_notifications_by_to_field_matches_partial_phone_numbers( to_field='+447700900200', normalised_to='447700900200', ) - results = dao_get_notifications_by_to_field(notification_1.service_id, search_term) + results = dao_get_notifications_by_to_field(notification_1.service_id, search_term, notification_type='sms') notification_ids = [notification.id for notification in results] assert len(results) == 1 @@ -1811,7 +1814,7 @@ def test_dao_get_notifications_by_to_field_accepts_invalid_phone_numbers_and_ema notification = create_notification( template=sample_template, to_field='test@example.com', normalised_to='test@example.com' ) - results = dao_get_notifications_by_to_field(notification.service_id, to) + results = dao_get_notifications_by_to_field(notification.service_id, to, notification_type='email') assert len(results) == 0 @@ -1829,7 +1832,7 @@ def test_dao_get_notifications_by_to_field_search_ignores_spaces(sample_template template=sample_template, to_field='jaCK@gmail.com', normalised_to='jack@gmail.com' ) - results = dao_get_notifications_by_to_field(notification1.service_id, '+447700900855') + results = dao_get_notifications_by_to_field(notification1.service_id, '+447700900855', notification_type='sms') notification_ids = [notification.id for notification in results] assert len(results) == 3 @@ -1848,8 +1851,6 @@ def test_dao_get_notifications_by_to_field_only_searches_for_notification_type( email = create_notification( template=email_template, to_field='077@example.com', normalised_to='077@example.com' ) - results = dao_get_notifications_by_to_field(service.id, "077") - assert len(results) == 2 results = dao_get_notifications_by_to_field(service.id, "077", notification_type='sms') assert len(results) == 1 assert results[0].id == sms.id @@ -1908,7 +1909,9 @@ def test_dao_get_notifications_by_to_field_filters_status(sample_template): normalised_to='447700900855', status='temporary-failure' ) - notifications = dao_get_notifications_by_to_field(notification.service_id, "+447700900855", statuses=['delivered']) + notifications = dao_get_notifications_by_to_field(notification.service_id, "+447700900855", + statuses=['delivered'], + notification_type='sms') assert len(notifications) == 1 assert notification.id == notifications[0].id @@ -1925,7 +1928,7 @@ def test_dao_get_notifications_by_to_field_filters_multiple_statuses(sample_temp ) notifications = dao_get_notifications_by_to_field( - notification1.service_id, "+447700900855", statuses=['delivered', 'sending'] + notification1.service_id, "+447700900855", statuses=['delivered', 'sending'], notification_type='sms' ) notification_ids = [notification.id for notification in notifications] @@ -1945,7 +1948,7 @@ def test_dao_get_notifications_by_to_field_returns_all_if_no_status_filter(sampl ) notifications = dao_get_notifications_by_to_field( - notification1.service_id, "+447700900855" + notification1.service_id, "+447700900855", notification_type='sms' ) notification_ids = [notification.id for notification in notifications] @@ -1967,7 +1970,7 @@ def test_dao_get_notifications_by_to_field_orders_by_created_at_desc(sample_temp notification = notification(created_at=datetime.utcnow()) notifications = dao_get_notifications_by_to_field( - sample_template.service_id, '+447700900855' + sample_template.service_id, '+447700900855', notification_type='sms' ) assert len(notifications) == 2 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 710164f24..6747afe5d 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1914,13 +1914,15 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[2]["is_precompiled_letter"] is False -def test_search_for_notification_by_to_field(client, notify_db, notify_db_session): - create_notification = partial(create_sample_notification, notify_db, notify_db_session) - notification1 = create_notification(to_field='+447700900855', normalised_to='447700900855') - notification2 = create_notification(to_field='jack@gmail.com', normalised_to='jack@gmail.com') +def test_search_for_notification_by_to_field(client, sample_template, sample_email_template): + + notification1 = create_notification(template=sample_template, to_field='+447700900855', + normalised_to='447700900855') + notification2 = create_notification(template=sample_email_template, to_field='jack@gmail.com', + normalised_to='jack@gmail.com') response = client.get( - '/service/{}/notifications?to={}'.format(notification1.service_id, 'jack@gmail.com'), + '/service/{}/notifications?to={}&template_type={}'.format(notification1.service_id, 'jack@gmail.com', 'email'), headers=[create_authorization_header()] ) notifications = json.loads(response.get_data(as_text=True))['notifications'] @@ -1938,7 +1940,7 @@ def test_search_for_notification_by_to_field_return_empty_list_if_there_is_no_ma create_notification(to_field='jack@gmail.com') response = client.get( - '/service/{}/notifications?to={}'.format(notification1.service_id, '+447700900800'), + '/service/{}/notifications?to={}&template_type={}'.format(notification1.service_id, '+447700900800', 'sms'), headers=[create_authorization_header()] ) notifications = json.loads(response.get_data(as_text=True))['notifications'] @@ -1955,7 +1957,7 @@ def test_search_for_notification_by_to_field_return_multiple_matches(client, not notification4 = create_notification(to_field='jack@gmail.com', normalised_to='jack@gmail.com') response = client.get( - '/service/{}/notifications?to={}'.format(notification1.service_id, '+447700900855'), + '/service/{}/notifications?to={}&template_type={}'.format(notification1.service_id, '+447700900855', 'sms'), headers=[create_authorization_header()] ) notifications = json.loads(response.get_data(as_text=True))['notifications'] @@ -2049,8 +2051,8 @@ def test_search_for_notification_by_to_field_filters_by_status(client, notify_db create_notification(status='sending') response = client.get( - '/service/{}/notifications?to={}&status={}'.format( - notification1.service_id, '+447700900855', 'delivered' + '/service/{}/notifications?to={}&status={}&template_type={}'.format( + notification1.service_id, '+447700900855', 'delivered', 'sms' ), headers=[create_authorization_header()] ) @@ -2074,8 +2076,8 @@ def test_search_for_notification_by_to_field_filters_by_statuses(client, notify_ notification2 = create_notification(status='sending') response = client.get( - '/service/{}/notifications?to={}&status={}&status={}'.format( - notification1.service_id, '+447700900855', 'delivered', 'sending' + '/service/{}/notifications?to={}&status={}&status={}&template_type={}'.format( + notification1.service_id, '+447700900855', 'delivered', 'sending', 'sms' ), headers=[create_authorization_header()] ) @@ -2104,8 +2106,8 @@ def test_search_for_notification_by_to_field_returns_content( ) response = client.get( - '/service/{}/notifications?to={}'.format( - sample_template_with_placeholders.service_id, '+447700900855' + '/service/{}/notifications?to={}&template_type={}'.format( + sample_template_with_placeholders.service_id, '+447700900855', 'sms' ), headers=[create_authorization_header()] ) @@ -2225,8 +2227,8 @@ def test_search_for_notification_by_to_field_returns_personlisation( ) response = client.get( - '/service/{}/notifications?to={}'.format( - sample_template_with_placeholders.service_id, '+447700900855' + '/service/{}/notifications?to={}&template_type={}'.format( + sample_template_with_placeholders.service_id, '+447700900855', 'sms' ), headers=[create_authorization_header()] )