From ca2bea8ae6b4e245f3cf25955b66cb0cd79be83a Mon Sep 17 00:00:00 2001 From: Toby Lorne Date: Mon, 26 Oct 2020 15:14:26 +0000 Subject: [PATCH 1/2] broadcasts: test helper uses areas from message instead of looking at "transmitted_areas" argument in the create_broadcast_event, we should use the areas from the broadcast message we should be explicit in our tests about which areas we are sending, the tests were implicitly using the default, rather than the areas from the broadcast message which was confusing Signed-off-by: Toby Lorne Co-authored-by: Richard Co-authored-by: David --- .../celery/test_broadcast_message_tasks.py | 58 ++++++++++++++++--- tests/app/db.py | 4 +- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 77b98d62d..bc84dd56b 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -11,7 +11,13 @@ def test_create_broadcast_event_sends_data_correctly(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( template, - areas={"areas": ['london'], "simple_polygons": [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]]}, + areas={ + 'areas': ['london', 'glasgow'], + 'simple_polygons': [ + [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], + [[-4.53, 55.72], [-3.88, 55.72], [-3.88, 55.96], [-4.53, 55.96]], + ], + }, status=BroadcastStatusType.BROADCASTING ) event = create_broadcast_event(broadcast_message) @@ -24,18 +30,34 @@ def test_create_broadcast_event_sends_data_correctly(mocker, sample_service): mock_create_broadcast.assert_called_once_with( identifier=str(event.id), - headline="GOV.UK Notify Broadcast", + headline='GOV.UK Notify Broadcast', description='this is an emergency broadcast message', areas=[{ - "description": "london", - "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], + 'polygon': [ + [50.12, 1.2], [50.13, 1.2], [50.14, 1.21], + ], + }, { + 'polygon': [ + [-4.53, 55.72], [-3.88, 55.72], [-3.88, 55.96], [-4.53, 55.96], + ], }], ) def test_update_broadcast_event_sends_references(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE, content='content') - broadcast_message = create_broadcast_message(template, areas=['london'], status=BroadcastStatusType.BROADCASTING) + + broadcast_message = create_broadcast_message( + template, + areas={ + 'areas': ['london'], + 'simple_polygons': [ + [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], + ], + }, + status=BroadcastStatusType.BROADCASTING + ) + alert_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.ALERT) update_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.UPDATE) @@ -59,7 +81,18 @@ def test_update_broadcast_event_sends_references(mocker, sample_service): def test_cancel_broadcast_event_sends_references(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE, content='content') - broadcast_message = create_broadcast_message(template, areas=['london'], status=BroadcastStatusType.BROADCASTING) + + broadcast_message = create_broadcast_message( + template, + areas={ + 'areas': ['london'], + 'simple_polygons': [ + [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], + ], + }, + status=BroadcastStatusType.BROADCASTING + ) + alert_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.ALERT) update_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.UPDATE) cancel_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.CANCEL) @@ -84,7 +117,18 @@ def test_cancel_broadcast_event_sends_references(mocker, sample_service): def test_send_broadcast_event_errors(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE) - broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING) + + broadcast_message = create_broadcast_message( + template, + areas={ + 'areas': ['london'], + 'simple_polygons': [ + [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], + ], + }, + status=BroadcastStatusType.BROADCASTING + ) + event = create_broadcast_event(broadcast_message) mock_create_broadcast = mocker.patch( diff --git a/tests/app/db.py b/tests/app/db.py index 6fc816b08..969a20e28 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1042,9 +1042,7 @@ def create_broadcast_event( sent_at=sent_at or datetime.utcnow(), message_type=message_type, transmitted_content=transmitted_content or {'body': 'this is an emergency broadcast message'}, - transmitted_areas=transmitted_areas or { - 'areas': ['london'], 'simple_polygons': [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]] - }, + transmitted_areas=transmitted_areas or broadcast_message.areas, transmitted_sender=transmitted_sender or 'www.notifications.service.gov.uk', transmitted_starts_at=transmitted_starts_at, transmitted_finishes_at=transmitted_finishes_at or datetime.utcnow(), From d22adb7813dd056c81d1e03da713ea9effed5c60 Mon Sep 17 00:00:00 2001 From: Toby Lorne Date: Mon, 26 Oct 2020 15:16:33 +0000 Subject: [PATCH 2/2] broadcasts: do not send description to cbc_proxy "areas" and "simple_polygons" in "transmitted_areas" do not have the same length as an example, choosing the area "england" results in a single item in "areas" but many polygons in "simple_polygons" therefore zipping these two together gives a list of areas: * of length 1 * containing only new grimsby which is not what we want as the CBC does not care about the areaDesc field within CAP, we should omit it from the function invocation and delegate the contents of areaDesc to the CBC Proxy implementation Signed-off-by: Toby Lorne Co-authored-by: Richard Co-authored-by: David --- app/celery/broadcast_message_tasks.py | 7 ++----- tests/app/celery/test_broadcast_message_tasks.py | 3 --- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 53940c4da..c6ed84d18 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -19,11 +19,8 @@ def send_broadcast_event(broadcast_event_id): ) areas = [ - {"description": desc, "polygon": polygon} - for desc, polygon in zip( - broadcast_event.transmitted_areas["areas"], - broadcast_event.transmitted_areas["simple_polygons"], - ) + {"polygon": polygon} + for polygon in broadcast_event.transmitted_areas["simple_polygons"] ] if broadcast_event.message_type == BroadcastEventMessageType.ALERT: diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index bc84dd56b..5373780a7 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -72,7 +72,6 @@ def test_update_broadcast_event_sends_references(mocker, sample_service): headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ - "description": "london", "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], }], references=[alert_event.reference], @@ -108,7 +107,6 @@ def test_cancel_broadcast_event_sends_references(mocker, sample_service): headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ - "description": "london", "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], }], references=[alert_event.reference, update_event.reference], @@ -146,7 +144,6 @@ def test_send_broadcast_event_errors(mocker, sample_service): headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ - 'description': 'london', 'polygon': [ [50.12, 1.2], [50.13, 1.2],