From 1334538cadca232ad4d8d7ac1ff7dba175243ab9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Oct 2021 18:13:39 +0100 Subject: [PATCH 1/4] Prefer `cap_event` to `reference` when referring to an alert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `reference` isn’t very human-friendly – the Environment Agency just supply a UUID in this field. The Environment Agency also populate the ``` field with some human readable text, for example: > 013 Issue Severe Flood Warning EA (013 is an ‘area code’ which will be meaningful to the Flood Warning Service team) This commit changes the frontend to display the value of the `cap_event` field, if it’s present, which is where the API stores the value of the `` field from the original CAP XML. *** Depends on: - [x] https://github.com/alphagov/notifications-api/pull/3344/files --- app/models/broadcast_message.py | 2 +- tests/__init__.py | 2 ++ tests/app/main/views/test_broadcast.py | 36 +++++++++++++++++++++----- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 3385ccc08..3339c07ff 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -162,7 +162,7 @@ class BroadcastMessage(JSONModel): def reference(self): if self.template_id: return self._dict['template_name'] - return self._dict['reference'] + return self._dict['cap_event'] or self._dict['reference'] @property def template(self): diff --git a/tests/__init__.py b/tests/__init__.py index 2dca4e6d3..58e67ec82 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -684,6 +684,7 @@ def broadcast_message_json( simple_polygons=None, content=None, reference=None, + cap_event=None, template_name='Example template', ): return { @@ -696,6 +697,7 @@ def broadcast_message_json( 'template_name': template_name, 'content': content or 'This is a test', 'reference': reference, + 'cap_event': cap_event, 'personalisation': {}, 'areas': areas or { diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 8d83ca66d..239f7630f 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -1811,12 +1811,19 @@ def test_view_broadcast_message_shows_correct_highlighted_navigation( ) +@pytest.mark.parametrize('extra_broadcast_json_fields', ( + # These should be ignored for broadcasts with a template + {'reference': 'foo'}, + {'cap_event': 'bar'}, + {}, +)) def test_view_pending_broadcast( mocker, client_request, service_one, fake_uuid, active_user_approve_broadcasts_permission, + extra_broadcast_json_fields, ): broadcast_creator = create_active_user_create_broadcasts_permissions(with_unique_id=True) mocker.patch( @@ -1828,6 +1835,7 @@ def test_view_pending_broadcast( created_by_id=broadcast_creator['id'], finishes_at=None, status='pending-approval', + **extra_broadcast_json_fields ), ) client_request.login(active_user_approve_broadcasts_permission) @@ -1866,12 +1874,31 @@ def test_view_pending_broadcast( ) +@pytest.mark.parametrize('extra_broadcast_json_fields, expected_banner_text', ( + ({'reference': 'No template test'}, ( + 'Test User Create Broadcasts Permission wants to broadcast No template test ' + 'No phones will get this alert. ' + 'Start broadcasting now Reject this alert' + )), + ({'cap_event': 'No template test'}, ( + 'Test User Create Broadcasts Permission wants to broadcast No template test ' + 'No phones will get this alert. ' + 'Start broadcasting now Reject this alert' + )), + ({'cap_event': 'EVENT', 'reference': 'REFERENCE'}, ( + 'Test User Create Broadcasts Permission wants to broadcast EVENT ' + 'No phones will get this alert. ' + 'Start broadcasting now Reject this alert' + )), +)) def test_view_pending_broadcast_without_template( mocker, client_request, service_one, fake_uuid, active_user_approve_broadcasts_permission, + extra_broadcast_json_fields, + expected_banner_text, ): broadcast_creator = create_active_user_create_broadcasts_permissions(with_unique_id=True) mocker.patch( @@ -1883,8 +1910,8 @@ def test_view_pending_broadcast_without_template( created_by_id=broadcast_creator['id'], finishes_at=None, status='pending-approval', - reference='No template test', content='Uh-oh', + **extra_broadcast_json_fields, ), ) client_request.login(active_user_approve_broadcasts_permission) @@ -1902,11 +1929,8 @@ def test_view_pending_broadcast_without_template( assert ( normalize_spaces(page.select_one('.banner').text) - ) == ( - 'Test User Create Broadcasts Permission wants to broadcast No template test ' - 'No phones will get this alert. ' - 'Start broadcasting now Reject this alert' - ) + ) == expected_banner_text + assert ( normalize_spaces(page.select_one('.broadcast-message-wrapper').text) ) == ( From cd86035588f4e175d27f747428950a7252f18474 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 26 Oct 2021 15:30:59 +0100 Subject: [PATCH 2/4] :Use more realistic test data --- tests/app/main/views/test_broadcast.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 239f7630f..f7cc63d75 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -1875,18 +1875,18 @@ def test_view_pending_broadcast( @pytest.mark.parametrize('extra_broadcast_json_fields, expected_banner_text', ( - ({'reference': 'No template test'}, ( - 'Test User Create Broadcasts Permission wants to broadcast No template test ' + ({'reference': 'ABC123'}, ( + 'Test User Create Broadcasts Permission wants to broadcast ABC123 ' 'No phones will get this alert. ' 'Start broadcasting now Reject this alert' )), - ({'cap_event': 'No template test'}, ( - 'Test User Create Broadcasts Permission wants to broadcast No template test ' + ({'cap_event': 'Severe flood warning'}, ( + 'Test User Create Broadcasts Permission wants to broadcast Severe flood warning ' 'No phones will get this alert. ' 'Start broadcasting now Reject this alert' )), - ({'cap_event': 'EVENT', 'reference': 'REFERENCE'}, ( - 'Test User Create Broadcasts Permission wants to broadcast EVENT ' + ({'cap_event': 'Severe flood warning', 'reference': 'ABC123'}, ( + 'Test User Create Broadcasts Permission wants to broadcast Severe flood warning ' 'No phones will get this alert. ' 'Start broadcasting now Reject this alert' )), From ca3d696bc1a5a79ace5fbd758c8a84d190d05602 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 26 Oct 2021 15:32:10 +0100 Subject: [PATCH 3/4] Delete unexpected test case --- tests/app/main/views/test_broadcast.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index f7cc63d75..02fa97e8f 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -1880,11 +1880,6 @@ def test_view_pending_broadcast( 'No phones will get this alert. ' 'Start broadcasting now Reject this alert' )), - ({'cap_event': 'Severe flood warning'}, ( - 'Test User Create Broadcasts Permission wants to broadcast Severe flood warning ' - 'No phones will get this alert. ' - 'Start broadcasting now Reject this alert' - )), ({'cap_event': 'Severe flood warning', 'reference': 'ABC123'}, ( 'Test User Create Broadcasts Permission wants to broadcast Severe flood warning ' 'No phones will get this alert. ' From bc06b47a926d02300000b2e4bd682fe1e95b99b3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 27 Oct 2021 14:47:11 +0100 Subject: [PATCH 4/4] Delete unexpected test case --- tests/app/main/views/test_broadcast.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 02fa97e8f..9fe537c47 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -1811,19 +1811,12 @@ def test_view_broadcast_message_shows_correct_highlighted_navigation( ) -@pytest.mark.parametrize('extra_broadcast_json_fields', ( - # These should be ignored for broadcasts with a template - {'reference': 'foo'}, - {'cap_event': 'bar'}, - {}, -)) def test_view_pending_broadcast( mocker, client_request, service_one, fake_uuid, active_user_approve_broadcasts_permission, - extra_broadcast_json_fields, ): broadcast_creator = create_active_user_create_broadcasts_permissions(with_unique_id=True) mocker.patch( @@ -1835,7 +1828,6 @@ def test_view_pending_broadcast( created_by_id=broadcast_creator['id'], finishes_at=None, status='pending-approval', - **extra_broadcast_json_fields ), ) client_request.login(active_user_approve_broadcasts_permission)