From 7897ae70ceedd5853f60fa4d67f5241ae31a7ce1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 21 Apr 2020 14:19:41 +0100 Subject: [PATCH] Let users search for letters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Like we have search by email address or phone number, finding an individual letter is a common task. At the moment users are having to click through pages and pages of letters to find the one they’re looking for. We have to search in the `to` and `normalised_to` fields for now because we’re not populating the `normalised_to` column for letters at the moment. --- app/dao/notifications_dao.py | 7 ++-- .../notification_dao/test_notification_dao.py | 35 +++++++++++++++++-- tests/app/service/test_rest.py | 24 +++++++++---- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d959ecb13..a06174f49 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -26,7 +26,6 @@ from werkzeug.datastructures import MultiDict from app import db, create_uuid from app.aws.s3 import remove_s3_object, get_s3_bucket_objects from app.dao.dao_utils import transactional -from app.errors import InvalidRequest from app.letters.utils import get_letter_pdf_filename from app.models import ( FactNotificationStatus, @@ -600,11 +599,8 @@ def dao_get_notifications_by_recipient_or_reference(service_id, search_term, not except InvalidEmailError: normalised = search_term.lower() - elif notification_type == LETTER_TYPE: - raise InvalidRequest("Only email and SMS can use search by recipient", 400) - else: - normalised = search_term.lower() + normalised = ''.join(search_term.split()).lower() normalised = escape_special_characters(normalised) search_term = escape_special_characters(search_term) @@ -612,6 +608,7 @@ def dao_get_notifications_by_recipient_or_reference(service_id, search_term, not filters = [ Notification.service_id == service_id, or_( + Notification.to.ilike("%{}%".format(search_term)), Notification.normalised_to.like("%{}%".format(normalised)), Notification.client_reference.ilike("%{}%".format(search_term)), ), diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 3a9ddf75f..1f3290913 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1293,6 +1293,7 @@ def test_dao_get_notifications_by_reference( service = create_service() sms_template = create_template(service=service) email_template = create_template(service=service, template_type='email') + letter_template = create_template(service=service, template_type='letter') sms = create_notification( template=sms_template, to_field='07711111111', @@ -1305,15 +1306,28 @@ def test_dao_get_notifications_by_reference( normalised_to='077@example.com', client_reference='77bB', ) + letter = create_notification( + template=letter_template, + to_field='123 Example Street\nXX1X 1XX', + normalised_to='123examplestreetxx1x1xx', + client_reference='77bB', + ) results = dao_get_notifications_by_recipient_or_reference(service.id, '77') + assert len(results) == 3 + assert results[0].id == letter.id + assert results[1].id == email.id + assert results[2].id == sms.id + + # If notification_type isn’t specified then we can’t normalise the phone number + # to 4477… but this query will still find the 077… variant in the `to` field, + # as well as the email + results = dao_get_notifications_by_recipient_or_reference(service.id, '077') assert len(results) == 2 assert results[0].id == email.id assert results[1].id == sms.id - # If notification_type isn’t specified then we can’t normalise the phone number - # to 4477… so this query will only find the email sent to 077@example.com - results = dao_get_notifications_by_recipient_or_reference(service.id, '077') + results = dao_get_notifications_by_recipient_or_reference(service.id, '077@') assert len(results) == 1 assert results[0].id == email.id @@ -1343,6 +1357,21 @@ def test_dao_get_notifications_by_reference( results = dao_get_notifications_by_recipient_or_reference(service.id, 'aA', notification_type='email') assert len(results) == 0 + results = dao_get_notifications_by_recipient_or_reference(service.id, 'aA', notification_type='letter') + assert len(results) == 0 + + results = dao_get_notifications_by_recipient_or_reference(service.id, '123') + assert len(results) == 1 + assert results[0].id == letter.id + + results = dao_get_notifications_by_recipient_or_reference(service.id, 'xX 1x1 Xx') + assert len(results) == 1 + assert results[0].id == letter.id + + results = dao_get_notifications_by_recipient_or_reference(service.id, '77', notification_type='letter') + assert len(results) == 1 + assert results[0].id == letter.id + def test_dao_created_scheduled_notification(sample_notification): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 403d5d6fd..eb5788c03 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2103,16 +2103,28 @@ def test_search_for_notification_by_to_field_return_multiple_matches(client, sam assert str(notification4.id) not in notification_ids -def test_search_for_notification_by_to_field_return_400_for_letter_type( - client, notify_db, notify_db_session, sample_service +def test_search_for_notification_by_to_field_for_letter( + client, + notify_db, + notify_db_session, + sample_letter_template, + sample_email_template, + sample_template, ): + letter_notification = create_notification(sample_letter_template, to_field='A. Name', normalised_to='a.name') + create_notification(sample_email_template, to_field='A.Name@example.com', normalised_to='a.name@example.com') + create_notification(sample_template, to_field='44770900123', normalised_to='44770900123') response = client.get( - '/service/{}/notifications?to={}&template_type={}'.format(sample_service.id, 'A. Name', 'letter'), + '/service/{}/notifications?to={}&template_type={}'.format( + sample_letter_template.service_id, 'A. Name', 'letter', + ), headers=[create_authorization_header()] ) - response.status_code = 400 - error_message = json.loads(response.get_data(as_text=True)) - assert error_message['message'] == 'Only email and SMS can use search by recipient' + notifications = json.loads(response.get_data(as_text=True))['notifications'] + + assert response.status_code == 200 + assert len(notifications) == 1 + assert notifications[0]['id'] == str(letter_notification.id) def test_update_service_calls_send_notification_as_service_becomes_live(notify_db, notify_db_session, client, mocker):