From 8dae4f771a083d85a2732bfb7500c334ee4384d6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 13 Oct 2020 15:43:25 +0100 Subject: [PATCH 1/3] Show the content of the alert on the dashboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content is as important as which areas you’ve sent the broadcast to. --- .../views/broadcast/partials/dashboard-table.html | 3 +++ tests/app/main/views/test_broadcast.py | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/templates/views/broadcast/partials/dashboard-table.html b/app/templates/views/broadcast/partials/dashboard-table.html index ea4bbb586..fd7a6eccd 100644 --- a/app/templates/views/broadcast/partials/dashboard-table.html +++ b/app/templates/views/broadcast/partials/dashboard-table.html @@ -15,6 +15,9 @@ {% call row_heading() %}
{{ item.template_name }} + + {{ item.content }} + To {{ item.initial_area_names|formatted_list(before_each='', after_each='') }} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 141c9f85f..fc1c9d285 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -302,7 +302,7 @@ def test_broadcast_dashboard( assert [ normalize_spaces(row.text) for row in page.select('table')[0].select('tbody tr') ] == [ - 'Example template To England and Scotland Live until tomorrow at 2:20am', + 'Example template This is a test To England and Scotland Live until tomorrow at 2:20am', ] assert normalize_spaces(page.select('main h2')[1].text) == ( @@ -311,7 +311,7 @@ def test_broadcast_dashboard( assert [ normalize_spaces(row.text) for row in page.select('table')[1].select('tbody tr') ] == [ - 'Example template To England and Scotland Prepared by Test User', + 'Example template This is a test To England and Scotland Prepared by Test User', ] @@ -361,8 +361,8 @@ def test_previous_broadcasts_page( assert [ normalize_spaces(row.text) for row in page.select('table')[0].select('tbody tr') ] == [ - 'Example template To England and Scotland Stopped 10 February at 2:20am', - 'Example template To England and Scotland Finished yesterday at 8:20pm', + 'Example template This is a test To England and Scotland Stopped 10 February at 2:20am', + 'Example template This is a test To England and Scotland Finished yesterday at 8:20pm', ] From 5c9a886edc74080b8262279296d8d341310d3b29 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 13 Oct 2020 17:37:38 +0100 Subject: [PATCH 2/3] Preview content as hint for broadcast templates Broadcast services only have broadcast templates. But we show the template type under the name of the template. This is redundant. It would be better to preview the content of the template instead. This then makes the templates page consistent with the dashboard. Depends on: - [ ] https://github.com/alphagov/notifications-api/pull/2996 --- app/__init__.py | 2 +- app/models/template_list.py | 20 +++++--- tests/app/main/views/test_templates.py | 69 ++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 8aa82abb7..561b322f8 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -409,7 +409,7 @@ def valid_phone_number(phone_number): def format_notification_type(notification_type): return { 'email': 'Email', - 'sms': 'SMS', + 'sms': 'Text message', 'letter': 'Letter' }[notification_type] diff --git a/app/models/template_list.py b/app/models/template_list.py index def776b08..144a4675a 100644 --- a/app/models/template_list.py +++ b/app/models/template_list.py @@ -1,3 +1,6 @@ +from app import format_notification_type + + class TemplateList(): def __init__( @@ -128,12 +131,17 @@ class TemplateListTemplate(TemplateListItem): ): super().__init__(template, ancestors) self.service_id = service_id - self.hint = { - 'email': 'Email template', - 'sms': 'Text message template', - 'letter': 'Letter template', - 'broadcast': 'Broadcast template', - }.get(template['template_type']) + self.template_type = template['template_type'] + self.content = template.get('content') + + @property + def hint(self): + if self.template_type == 'broadcast': + max_length_in_chars = 40 + if len(self.content) > (max_length_in_chars + 2): + return self.content[:max_length_in_chars].strip() + '…' + return self.content + return format_notification_type(self.template_type) + ' template' class TemplateListFolder(TemplateListItem): diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 8dc5daeb1..e6cf72527 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -199,6 +199,75 @@ def test_should_show_page_for_choosing_a_template( mock_get_template_folders.assert_called_once_with(SERVICE_ONE_ID) +def test_should_show_page_of_broadcast_templates( + mocker, + client_request, + service_one, + fake_uuid, + mock_get_template_folders, +): + service_one['permissions'] += ['broadcast'] + mocker.patch( + 'app.service_api_client.get_service_templates', + return_value={'data': [ + template_json( + SERVICE_ONE_ID, + fake_uuid, + type_='broadcast', + name='A', + content='a' * 40, + ), + template_json( + SERVICE_ONE_ID, + fake_uuid, + type_='broadcast', + name='B', + content='b' * 42, + ), + template_json( + SERVICE_ONE_ID, + fake_uuid, + type_='broadcast', + name='C', + content='c' * 43, + ), + template_json( + SERVICE_ONE_ID, + fake_uuid, + type_='broadcast', + name='D', + # This should be truncated at 40 chars, then have the + # trailing space stripped + content=('d' * 39) + ' ' + ('d' * 40), + ), + ]} + ) + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + assert [ + ( + normalize_spaces(template.select_one('.govuk-link').text), + normalize_spaces(template.select_one('.govuk-hint').text), + ) + for template in page.select('.template-list-item') + ] == [ + ( + 'A', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + ), + ( + 'B', 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + ), + ( + 'C', 'cccccccccccccccccccccccccccccccccccccccc…', + ), + ( + 'D', 'ddddddddddddddddddddddddddddddddddddddd…', + ), + ] + + def test_choose_template_can_pass_through_an_initial_state_to_templates_and_folders_selection_form( client_request, mock_get_template_folders, From e2f16e414dd7ef21f026a041359c537b4d690228 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 14 Oct 2020 10:43:04 +0100 Subject: [PATCH 3/3] Style areas on dashboard the same as alert preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our style for areas is pale blue background with black keylines or bold black text. This commit makes the display of area names on the dashboard consistent with that visual style. This also means that we’re not truncating the list of areas, which is appropriate because no one area is more important than any of the others. --- app/models/broadcast_message.py | 6 ------ .../views/broadcast/partials/dashboard-table.html | 10 ++++++---- tests/app/main/views/test_broadcast.py | 8 ++++---- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 27106be8e..372aec925 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -76,12 +76,6 @@ class BroadcastMessage(JSONModel): for parent in area.parents: yield parent - @property - def initial_area_names(self): - return [ - area.name for area in self.areas - ][:10] - @cached_property def polygons(self): return Polygons( diff --git a/app/templates/views/broadcast/partials/dashboard-table.html b/app/templates/views/broadcast/partials/dashboard-table.html index fd7a6eccd..1e1e559f1 100644 --- a/app/templates/views/broadcast/partials/dashboard-table.html +++ b/app/templates/views/broadcast/partials/dashboard-table.html @@ -15,12 +15,14 @@ {% call row_heading() %}
{{ item.template_name }} - + {{ item.content }} - - To {{ item.initial_area_names|formatted_list(before_each='', after_each='') }} - +
    + {% for area in item.areas %} +
  • {{ area.name }}
    • + {% endfor %} +
{% endcall %} {% call field(align='right') %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index fc1c9d285..be55d22bf 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -302,7 +302,7 @@ def test_broadcast_dashboard( assert [ normalize_spaces(row.text) for row in page.select('table')[0].select('tbody tr') ] == [ - 'Example template This is a test To England and Scotland Live until tomorrow at 2:20am', + 'Example template This is a test England Scotland Live until tomorrow at 2:20am', ] assert normalize_spaces(page.select('main h2')[1].text) == ( @@ -311,7 +311,7 @@ def test_broadcast_dashboard( assert [ normalize_spaces(row.text) for row in page.select('table')[1].select('tbody tr') ] == [ - 'Example template This is a test To England and Scotland Prepared by Test User', + 'Example template This is a test England Scotland Prepared by Test User', ] @@ -361,8 +361,8 @@ def test_previous_broadcasts_page( assert [ normalize_spaces(row.text) for row in page.select('table')[0].select('tbody tr') ] == [ - 'Example template This is a test To England and Scotland Stopped 10 February at 2:20am', - 'Example template This is a test To England and Scotland Finished yesterday at 8:20pm', + 'Example template This is a test England Scotland Stopped 10 February at 2:20am', + 'Example template This is a test England Scotland Finished yesterday at 8:20pm', ]