From 986f2752e009e7c1ac556c6e93d7cdda27bc684c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 21 Apr 2020 13:55:23 +0100 Subject: [PATCH 1/3] 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. Users of the API will also be able to search by reference, same as for emails and text messages. But we only show this hint text to users who have some API keys. --- app/main/views/jobs.py | 8 +++- app/templates/views/notifications.html | 65 ++++++++++++-------------- tests/app/main/views/test_activity.py | 21 +++++++-- 3 files changed, 53 insertions(+), 41 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 94bd5cf19..4258d91be 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -169,8 +169,12 @@ def view_notifications(service_id, message_type=None): things_you_can_search_by={ 'email': ['email address'], 'sms': ['phone number'], - 'letter': [], - None: ['email address', 'phone number'], + # This should become ‘postal address’ not ‘first line…’ once + # we’ve finished populating normalised addresses + 'letter': ['first line of address', 'file name'], + # We say recipient here because combining all 3 types, plus + # reference gets too long for the hint text + None: ['recipient'], }.get(message_type) + { True: ['reference'], False: [], diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 7c9bbabc0..b675555af 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -34,42 +34,39 @@ 'counts' ) }} - {% call form_wrapper( - action=url_for('.view_notifications', service_id=current_service.id, message_type=message_type), - class="govuk-grid-row" - ) %} -
- {{ textbox( - search_form.to, - width='1-1', - label=things_you_can_search_by|formatted_list( - conjunction='or', - before_each='', - after_each='', - prefix='Search by', - prefix_plural='Search by' - ) - ) }} -
-
- - {{ govukButton({ - "text": "Search", - "classes": "search-form__button" - }) }} -
- {% endcall %} - - {% call form_wrapper(id="search-form") %} - - - {% endcall %} - {% else %} - {% call form_wrapper(id="search-form") %} - - {% endcall %} {% endif %} + {% call form_wrapper( + action=url_for('.view_notifications', service_id=current_service.id, message_type=message_type), + class="govuk-grid-row" + ) %} +
+ {{ textbox( + search_form.to, + width='1-1', + label=things_you_can_search_by|formatted_list( + conjunction='or', + before_each='', + after_each='', + prefix='Search by', + prefix_plural='Search by' + ) + ) }} +
+
+ + {{ govukButton({ + "text": "Search", + "classes": "search-form__button" + }) }} +
+ {% endcall %} + + {% call form_wrapper(id="search-form") %} + + + {% endcall %} + {% if current_user.has_permissions('view_activity') %}

Download this report diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 8ed04e75c..75e9f0b13 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -340,7 +340,7 @@ def test_shows_message_when_no_notifications( ( {}, {}, - 'Search by email address or phone number', + 'Search by recipient', '', ), ( @@ -373,6 +373,16 @@ def test_shows_message_when_no_notifications( 'Search by email address', 'test@example.com', ), + ( + { + 'message_type': 'letter', + }, + { + 'to': 'Firstname Lastname', + }, + 'Search by first line of address or file name', + 'Firstname Lastname', + ), ]) def test_search_recipient_form( client_request, @@ -413,9 +423,10 @@ def test_search_recipient_form( @pytest.mark.parametrize('message_type, expected_search_box_label', [ - (None, 'Search by email address, phone number or reference'), + (None, 'Search by recipient or reference'), ('sms', 'Search by phone number or reference'), ('email', 'Search by email address or reference'), + ('letter', 'Search by first line of address, file name or reference'), ]) def test_api_users_are_told_they_can_search_by_reference_when_service_has_api_keys( client_request, @@ -437,7 +448,7 @@ def test_api_users_are_told_they_can_search_by_reference_when_service_has_api_ke @pytest.mark.parametrize('message_type, expected_search_box_label', [ - (None, 'Search by email address or phone number'), + (None, 'Search by recipient'), ('sms', 'Search by phone number'), ('email', 'Search by email address'), ]) @@ -615,10 +626,10 @@ def test_redacts_templates_that_should_be_redacted( "message_type, tablist_visible, search_bar_visible", [ ('email', True, True), ('sms', True, True), - ('letter', False, False) + ('letter', False, True) ] ) -def test_big_numbers_and_search_dont_show_for_letters( +def test_big_numbers_dont_show_for_letters( client_request, service_one, mock_get_notifications, From b01bc17dbbaa198dade5aa19c7b4813914341fd7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 27 Apr 2020 12:47:30 +0100 Subject: [PATCH 2/3] Remove `search_bar_visible` test param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s always `True` now, so we can just `assert True` instead. --- tests/app/main/views/test_activity.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 75e9f0b13..01c428be2 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -623,10 +623,10 @@ def test_redacts_templates_that_should_be_redacted( @pytest.mark.parametrize( - "message_type, tablist_visible, search_bar_visible", [ - ('email', True, True), - ('sms', True, True), - ('letter', False, True) + "message_type, tablist_visible", [ + ('email', True), + ('sms', True), + ('letter', False) ] ) def test_big_numbers_dont_show_for_letters( @@ -639,7 +639,6 @@ def test_big_numbers_dont_show_for_letters( mock_get_no_api_keys, message_type, tablist_visible, - search_bar_visible ): page = client_request.get( 'main.view_notifications', @@ -650,7 +649,7 @@ def test_big_numbers_dont_show_for_letters( ) assert (len(page.select("[role=tablist]")) > 0) == tablist_visible - assert (len(page.select("[type=search]")) > 0) == search_bar_visible + assert (len(page.select("[type=search]")) > 0) is True @freeze_time("2017-09-27 16:30:00.000000") From c835674df25bca3ae8cff49c761c3451bf7e3611 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 27 Apr 2020 12:49:57 +0100 Subject: [PATCH 3/3] Add test case for searching by letter type --- tests/app/main/views/test_activity.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 01c428be2..1f8a2dcd3 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -451,6 +451,7 @@ def test_api_users_are_told_they_can_search_by_reference_when_service_has_api_ke (None, 'Search by recipient'), ('sms', 'Search by phone number'), ('email', 'Search by email address'), + ('letter', 'Search by first line of address or file name'), ]) def test_api_users_are_not_told_they_can_search_by_reference_when_service_has_no_api_keys( client_request,