diff --git a/app/broadcast_message/broadcast_message_schema.py b/app/broadcast_message/broadcast_message_schema.py index dc7996707..4b67e0ee7 100644 --- a/app/broadcast_message/broadcast_message_schema.py +++ b/app/broadcast_message/broadcast_message_schema.py @@ -13,7 +13,7 @@ create_broadcast_message_schema = { 'personalisation': {'type': 'object'}, 'starts_at': {'type': 'string', 'format': 'datetime'}, 'finishes_at': {'type': 'string', 'format': 'datetime'}, - 'areas': {"type": "array", "items": {"type": "string"}}, + 'areas': {'type': 'object'}, 'areas_2': {'type': 'object'}, 'simple_polygons': {"type": "array", "items": {"type": "array"}}, 'content': {'type': 'string', 'minLength': 1}, @@ -42,7 +42,7 @@ update_broadcast_message_schema = { 'personalisation': {'type': 'object'}, 'starts_at': {'type': 'string', 'format': 'datetime'}, 'finishes_at': {'type': 'string', 'format': 'datetime'}, - 'areas': {"type": "array", "items": {"type": "string"}}, + 'areas': {'type': 'object'}, 'areas_2': {'type': 'object'}, 'simple_polygons': {"type": "array", "items": {"type": "array"}}, }, diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 02b734ec2..bf1c30d49 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -133,27 +133,12 @@ def create_broadcast_message(service_id): content = str(temporary_template) reference = data['reference'] - # TEMPORARY: while we repurpose "areas" - areas = {} - areas_2 = data.get("areas_2", {}) - - if "areas" in data: - areas["areas"] = data["areas"] - areas["ids"] = data["areas"] - if "simple_polygons" in data: - areas["simple_polygons"] = data["simple_polygons"] - if "ids" in areas_2: - areas["areas"] = areas_2["ids"] - areas["ids"] = areas_2["ids"] - if "simple_polygons" in areas_2: - areas["simple_polygons"] = areas_2["simple_polygons"] - broadcast_message = BroadcastMessage( service_id=service.id, template_id=template_id, template_version=template.version if template else None, personalisation=personalisation, - areas=areas, + areas=data.get("areas", data.get("areas_2", {})), # TEMPORARY: while we repurpose "areas" status=BroadcastStatusType.DRAFT, starts_at=_parse_nullable_datetime(data.get('starts_at')), finishes_at=_parse_nullable_datetime(data.get('finishes_at')), @@ -186,19 +171,7 @@ def update_broadcast_message(service_id, broadcast_message_id): ) # TEMPORARY: while we repurpose "areas" - areas = {} - areas_2 = data.get("areas_2", {}) - - if "areas" in data: - areas["areas"] = data["areas"] - areas["ids"] = data["areas"] - if "simple_polygons" in data: - areas["simple_polygons"] = data["simple_polygons"] - if "ids" in areas_2: - areas["areas"] = areas_2["ids"] - areas["ids"] = areas_2["ids"] - if "simple_polygons" in areas_2: - areas["simple_polygons"] = areas_2["simple_polygons"] + areas = data.get("areas", data.get("areas_2", {})) if ('ids' in areas and 'simple_polygons' not in areas) or ('ids' not in areas and 'simple_polygons' in areas): raise InvalidRequest( @@ -212,7 +185,7 @@ def update_broadcast_message(service_id, broadcast_message_id): broadcast_message.starts_at = _parse_nullable_datetime(data['starts_at']) if 'finishes_at' in data: broadcast_message.finishes_at = _parse_nullable_datetime(data['finishes_at']) - if 'areas' in areas and 'simple_polygons' in areas: + if 'ids' in areas and 'simple_polygons' in areas: broadcast_message.areas = areas dao_save_object(broadcast_message) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 6a0b06886..641e63f21 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -135,7 +135,7 @@ def send_broadcast_event(broadcast_event_id): f'https://www.notifications.service.gov.uk/services/{broadcast_message.service_id}/current-alerts/{broadcast_message.id}', # noqa '', f'This broacast has been sent on channel {broadcast_message.service.broadcast_channel}.', - f'This broadcast is targeted at areas {broadcast_message.areas.get("ids", broadcast_message.areas.get("areas"))}.', # noqa + f'This broadcast is targeted at areas {broadcast_message.areas.get("names", [])}.', # noqa '' f'This broadcast\'s content starts "{broadcast_message.content[:100]}"' '', diff --git a/app/models.py b/app/models.py index 6895cd029..f24aa23a8 100644 --- a/app/models.py +++ b/app/models.py @@ -2337,9 +2337,9 @@ class BroadcastMessage(db.Model): def serialize(self): # TEMPORARY: while we repurpose "areas" - areas_2 = dict(self.areas) - areas_2["simple_polygons"] = areas_2.get("simple_polygons", []) - areas_2["ids"] = areas_2.pop("areas", []) + areas = dict(self.areas) + areas["simple_polygons"] = areas.get("simple_polygons", []) + areas["ids"] = areas.pop("areas", areas.get("ids", [])) return { 'id': str(self.id), @@ -2354,9 +2354,8 @@ class BroadcastMessage(db.Model): 'content': self.content, # TEMPORARY: switch to this so we can repurpose "areas" - 'areas_2': areas_2, - 'areas': self.areas.get("areas", []), - 'simple_polygons': self.areas.get("simple_polygons", []), + 'areas_2': areas, + 'areas': areas, 'status': self.status, diff --git a/app/v2/broadcast/post_broadcast.py b/app/v2/broadcast/post_broadcast.py index afebc0265..ae5f0823d 100644 --- a/app/v2/broadcast/post_broadcast.py +++ b/app/v2/broadcast/post_broadcast.py @@ -67,9 +67,6 @@ def create_broadcast(): content=broadcast_json['content'], reference=broadcast_json['reference'], areas={ - 'areas': [ - area['name'] for area in broadcast_json['areas'] - ], 'names': [ area['name'] for area in broadcast_json['areas'] ], diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index d2f50776e..2715361d9 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -19,7 +19,16 @@ from tests.app.db import ( ) -def test_get_broadcast_message(admin_request, sample_broadcast_service): +# TEMPORARY: while we repurpose "areas" +@pytest.mark.parametrize("area_data", [ + {"areas": ["place A", "region B"]}, + {"ids": ["place A", "region B"]}, +]) +def test_get_broadcast_message( + area_data, + admin_request, + sample_broadcast_service +): t = create_template( sample_broadcast_service, BROADCAST_TYPE, @@ -28,7 +37,7 @@ def test_get_broadcast_message(admin_request, sample_broadcast_service): bm = create_broadcast_message( t, areas={ - "areas": ['place A', 'region B'], + **area_data, "simple_polygons": [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]], }, personalisation={ @@ -50,17 +59,28 @@ def test_get_broadcast_message(admin_request, sample_broadcast_service): assert response['status'] == BroadcastStatusType.DRAFT assert response['created_at'] is not None assert response['starts_at'] is None - assert response['areas'] == ['place A', 'region B'] + assert response['areas']['ids'] == ['place A', 'region B'] + assert response['areas']['simple_polygons'] == [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]] assert response['areas_2']['ids'] == ['place A', 'region B'] + assert response['areas_2']['simple_polygons'] == [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]] assert response['personalisation'] == {'thing': 'test'} -def test_get_broadcast_message_without_template(admin_request, sample_broadcast_service): +# TEMPORARY: while we repurpose "areas" +@pytest.mark.parametrize("area_data", [ + {"areas": ["place A", "region B"]}, + {"ids": ["place A", "region B"]}, +]) +def test_get_broadcast_message_without_template( + area_data, + admin_request, + sample_broadcast_service +): bm = create_broadcast_message( service=sample_broadcast_service, content='emergency broadcast content', areas={ - "areas": ['place A', 'region B'], + **area_data, "simple_polygons": [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]], }, ) @@ -80,8 +100,10 @@ def test_get_broadcast_message_without_template(admin_request, sample_broadcast_ assert response['status'] == BroadcastStatusType.DRAFT assert response['created_at'] is not None assert response['starts_at'] is None - assert response['areas'] == ['place A', 'region B'] + assert response['areas']['ids'] == ['place A', 'region B'] + assert response['areas']['simple_polygons'] == [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]] assert response['areas_2']['ids'] == ['place A', 'region B'] + assert response['areas_2']['simple_polygons'] == [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]] assert response['personalisation'] is None @@ -154,7 +176,8 @@ def test_create_broadcast_message(admin_request, sample_broadcast_service, train assert response['created_at'] is not None assert response['created_by_id'] == str(t.created_by_id) assert response['personalisation'] == {} - assert response['areas'] == [] + assert response['areas']['ids'] == [] + assert response['areas']['simple_polygons'] == [] assert response['areas_2']['ids'] == [] assert response['areas_2']['simple_polygons'] == [] assert response['content'] == 'Some content\n€ŷŵ~\n\'\'""---' @@ -360,8 +383,10 @@ def test_create_broadcast_message_400s_if_no_content_or_template( @pytest.mark.parametrize('area_data', [ # TEMPORARY: while we repurpose "areas" { - "areas": ["london", "glasgow"], - "simple_polygons": [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] + "areas": { + "ids": ["london", "glasgow"], + "simple_polygons": [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] + }, }, { "areas_2": { @@ -384,7 +409,10 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live( t = create_template(sample_broadcast_service, BROADCAST_TYPE) bm = create_broadcast_message( t, - areas={"areas": ['manchester'], "simple_polygons": [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]]}, + areas={ + "ids": ['manchester'], + "simple_polygons": [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]] + }, status=status ) @@ -400,9 +428,9 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live( ) assert response['starts_at'] == '2020-06-01T20:00:01.000000Z' - assert response['areas'] == ['london', 'glasgow'] + assert response['areas']['ids'] == ['london', 'glasgow'] + assert response['areas']['simple_polygons'] == [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] assert response['areas_2']['ids'] == ['london', 'glasgow'] - assert response['simple_polygons'] == [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] assert response['areas_2']['simple_polygons'] == [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] assert response['updated_at'] is not None @@ -412,7 +440,7 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live( pytest.param(True, marks=pytest.mark.xfail) ]) @pytest.mark.parametrize('area_data', [ # TEMPORARY: while we repurpose "areas" - {'areas': ['london', 'glasgow']}, + {'areas': {'ids': ['london', 'glasgow']}}, {'areas_2': {'ids': ['london', 'glasgow']}}, ]) @pytest.mark.parametrize('status', [ @@ -429,7 +457,7 @@ def test_update_broadcast_message_doesnt_allow_edits_after_broadcast_goes_live( status ): t = create_template(sample_broadcast_service, BROADCAST_TYPE) - bm = create_broadcast_message(t, areas=['manchester'], status=status) + bm = create_broadcast_message(t, status=status) response = admin_request.post( 'broadcast_message.update_broadcast_message', @@ -445,7 +473,10 @@ def test_update_broadcast_message_sets_finishes_at_separately(admin_request, sam t = create_template(sample_broadcast_service, BROADCAST_TYPE) bm = create_broadcast_message( t, - areas={"areas": ["london"], "simple_polygons": [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]]} + areas={ + "ids": ["london"], + "simple_polygons": [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]] + } ) response = admin_request.post( @@ -485,8 +516,10 @@ def test_update_broadcast_message_allows_sensible_datetime_formats(admin_request @pytest.mark.parametrize('area_data', [ # TEMPORARY: while we repurpose "areas" { - "areas": ["glasgow"], - "simple_polygons": [[[55.86, -4.25], [55.85, -4.25], [55.87, -4.24]]], + "areas": { + "ids": ["glasgow"], + "simple_polygons": [[[55.86, -4.25], [55.85, -4.25], [55.87, -4.24]]], + } }, { "areas_2": { @@ -516,8 +549,9 @@ def test_update_broadcast_message_doesnt_let_you_update_status(area_data, admin_ @pytest.mark.parametrize("incomplete_area_data", [ - {"areas": ["cardiff"]}, # TEMPORARY: while we repurpose "areas" - {"simple_polygons": [[[51.28, -3.11], [51.29, -3.12], [51.27, -3.10]]]}, # TEMPORARY: while we repurpose "areas" + {"areas": {"ids": ["cardiff"]}}, + {"areas": {"simple_polygons": [[[51.28, -3.11], [51.29, -3.12], [51.27, -3.10]]]}}, + # TEMPORARY: while we repurpose "areas" {"areas_2": {"ids": ["cardiff"]}}, {"areas_2": {"simple_polygons": [[[51.28, -3.11], [51.29, -3.12], [51.27, -3.10]]]}}, ]) @@ -557,7 +591,7 @@ def test_update_broadcast_message_status(admin_request, sample_broadcast_service @pytest.mark.parametrize(('area_data', 'error'), [ # TEMPORARY: while we repurpose "areas" - ({'areas': ['glasgow']}, 'areas was unexpected'), + ({'areas': {'ids': ['glasgow']}}, 'areas was unexpected'), ({'areas_2': {'ids': ['glasgow']}}, 'areas_2 was unexpected') ]) def test_update_broadcast_message_status_doesnt_let_you_update_other_things( @@ -638,7 +672,6 @@ def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_ t, status=BroadcastStatusType.PENDING_APPROVAL, areas={ - "areas": ["london"], "ids": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]] } @@ -688,7 +721,7 @@ def test_update_broadcast_message_status_updates_details_but_does_not_queue_task bm = create_broadcast_message( t, status=BroadcastStatusType.PENDING_APPROVAL, - areas={"areas": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]}, + areas={"ids": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]}, stubbed=broadcast_message_stubbed ) approver = create_user(email='approver@gov.uk') @@ -741,7 +774,6 @@ def test_update_broadcast_message_status_creates_event_with_correct_content_if_b content='tailor made emergency broadcast content', status=BroadcastStatusType.PENDING_APPROVAL, areas={ - "areas": ["london"], "ids": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]] } @@ -829,7 +861,7 @@ def test_update_broadcast_message_status_allows_trial_mode_services_to_approve_o bm = create_broadcast_message( t, status=BroadcastStatusType.PENDING_APPROVAL, - areas={"areas": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]} + areas={"ids": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]} ) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index fce2d2e91..5411419a5 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -95,12 +95,22 @@ def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabl assert mock_send_broadcast_provider_message.apply_async.called is False -def test_send_broadcast_event_creates_zendesk_p1(mocker, notify_api, sample_broadcast_service): +@pytest.mark.parametrize('area_data,expected_message', [ + ({'names': ['England', 'Scotland']}, ['England', 'Scotland']), + ({}, []) +]) +def test_send_broadcast_event_creates_zendesk_p1( + area_data, + expected_message, + mocker, + notify_api, + sample_broadcast_service +): template = create_template(sample_broadcast_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( template, status=BroadcastStatusType.BROADCASTING, - areas={'areas': ['wd20-S13002775', 'wd20-S13002773'], 'simple_polygons': []}, + areas={**area_data, 'simple_polygons': []}, ) event = create_broadcast_event(broadcast_message) mock_create_ticket = mocker.patch("app.celery.broadcast_message_tasks.zendesk_client.create_ticket") @@ -117,7 +127,7 @@ def test_send_broadcast_event_creates_zendesk_p1(mocker, notify_api, sample_broa assert str(broadcast_message.id) in zendesk_args['message'] assert 'channel severe' in zendesk_args['message'] - assert "areas ['wd20-S13002775', 'wd20-S13002773']" in zendesk_args['message'] + assert f"areas {expected_message}" in zendesk_args['message'] # the start of the content from the broadcast template assert "Dear Sir/Madam" in zendesk_args['message'] diff --git a/tests/app/dao/test_broadcast_message_dao.py b/tests/app/dao/test_broadcast_message_dao.py index 144e896a2..fa52cb066 100644 --- a/tests/app/dao/test_broadcast_message_dao.py +++ b/tests/app/dao/test_broadcast_message_dao.py @@ -122,10 +122,10 @@ def test_dao_get_all_broadcast_messages(sample_broadcast_service): assert broadcast_messages == [ ( broadcast_message_2.id, None, 'severe', 'Dear Sir/Madam, Hello. Yours Truly, The Government.', - {'areas': [], 'ids': [], 'simple_polygons': []}, 'broadcasting', datetime(2021, 6, 20, 12, 0), + {'ids': [], 'simple_polygons': []}, 'broadcasting', datetime(2021, 6, 20, 12, 0), None, None, None), ( broadcast_message_1.id, None, 'severe', 'Dear Sir/Madam, Hello. Yours Truly, The Government.', - {'areas': [], 'ids': [], 'simple_polygons': []}, 'cancelled', datetime(2021, 6, 15, 12, 0), + {'ids': [], 'simple_polygons': []}, 'cancelled', datetime(2021, 6, 15, 12, 0), None, None, None) ] diff --git a/tests/app/db.py b/tests/app/db.py index 58f50824f..a71abc313 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1146,7 +1146,7 @@ def create_broadcast_message( starts_at=starts_at, finishes_at=finishes_at, created_by_id=created_by.id if created_by else service.created_by_id, - areas=areas or {'areas': [], 'ids': [], 'simple_polygons': []}, + areas=areas or {'ids': [], 'simple_polygons': []}, content=content, stubbed=stubbed ) diff --git a/tests/app/v2/broadcast/test_post_broadcast.py b/tests/app/v2/broadcast/test_post_broadcast.py index 6b3b57b05..5f6dd10d1 100644 --- a/tests/app/v2/broadcast/test_post_broadcast.py +++ b/tests/app/v2/broadcast/test_post_broadcast.py @@ -84,10 +84,10 @@ def test_valid_post_cap_xml_broadcast_returns_201( assert response_json['approved_at'] is None assert response_json['approved_by_id'] is None - # TEMPORARY: while we repurpose "areas" - assert response_json['areas'] == [ + assert response_json['areas']['names'] == [ 'River Steeping in Wainfleet All Saints' ] + # TEMPORARY: while we repurpose "areas" assert response_json['areas_2']['names'] == [ 'River Steeping in Wainfleet All Saints' ] @@ -108,11 +108,11 @@ def test_valid_post_cap_xml_broadcast_returns_201( assert response_json['service_id'] == str(sample_broadcast_service.id) # TEMPORARY: while we repurpose "areas" - assert len(response_json['simple_polygons']) == 1 - assert len(response_json['simple_polygons'][0]) == 23 - assert response_json['simple_polygons'][0][0] == [53.10562, 0.244127] - assert response_json['simple_polygons'][0][-1] == [53.10562, 0.244127] - assert response_json['simple_polygons'][0][-1] == [53.10562, 0.244127] + assert len(response_json['areas']['simple_polygons']) == 1 + assert len(response_json['areas']['simple_polygons'][0]) == 23 + assert response_json['areas']['simple_polygons'][0][0] == [53.10562, 0.244127] + assert response_json['areas']['simple_polygons'][0][-1] == [53.10562, 0.244127] + assert response_json['areas']['simple_polygons'][0][-1] == [53.10562, 0.244127] assert len(response_json['areas_2']['simple_polygons']) == 1 assert len(response_json['areas_2']['simple_polygons'][0]) == 23 assert response_json['areas_2']['simple_polygons'][0][0] == [53.10562, 0.244127]