diff --git a/app/broadcast_message/broadcast_message_schema.py b/app/broadcast_message/broadcast_message_schema.py index fc4527511..dc7996707 100644 --- a/app/broadcast_message/broadcast_message_schema.py +++ b/app/broadcast_message/broadcast_message_schema.py @@ -14,6 +14,7 @@ create_broadcast_message_schema = { 'starts_at': {'type': 'string', 'format': 'datetime'}, 'finishes_at': {'type': 'string', 'format': 'datetime'}, 'areas': {"type": "array", "items": {"type": "string"}}, + 'areas_2': {'type': 'object'}, 'simple_polygons': {"type": "array", "items": {"type": "array"}}, 'content': {'type': 'string', 'minLength': 1}, 'reference': {'type': 'string', 'minLength': 1, 'maxLength': 255}, @@ -42,6 +43,7 @@ update_broadcast_message_schema = { 'starts_at': {'type': 'string', 'format': 'datetime'}, 'finishes_at': {'type': 'string', 'format': 'datetime'}, 'areas': {"type": "array", "items": {"type": "string"}}, + 'areas_2': {'type': 'object'}, 'simple_polygons': {"type": "array", "items": {"type": "array"}}, }, 'required': [], diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 9a50c5cfe..50cf6908c 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -133,12 +133,25 @@ 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"] + if "simple_polygons" in data: + areas["simple_polygons"] = data["simple_polygons"] + if "areas" in areas_2: + areas["areas"] = areas_2["areas"] + 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": data.get("areas", []), "simple_polygons": data.get("simple_polygons", [])}, + areas=areas, status=BroadcastStatusType.DRAFT, starts_at=_parse_nullable_datetime(data.get('starts_at')), finishes_at=_parse_nullable_datetime(data.get('finishes_at')), @@ -170,7 +183,20 @@ def update_broadcast_message(service_id, broadcast_message_id): status_code=400 ) - if ('areas' in data and 'simple_polygons' not in data) or ('areas' not in data and 'simple_polygons' in data): + # TEMPORARY: while we repurpose "areas" + areas = {} + areas_2 = data.get("areas_2", {}) + + if "areas" in data: + areas["areas"] = data["areas"] + if "simple_polygons" in data: + areas["simple_polygons"] = data["simple_polygons"] + if "areas" in areas_2: + areas["areas"] = areas_2["areas"] + if "simple_polygons" in areas_2: + areas["simple_polygons"] = areas_2["simple_polygons"] + + if ('areas' in areas and 'simple_polygons' not in areas) or ('areas' not in areas and 'simple_polygons' in areas): raise InvalidRequest( f'Cannot update broadcast_message {broadcast_message.id}, areas or polygons are missing.', status_code=400 @@ -182,8 +208,8 @@ 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 data and 'simple_polygons' in data: - broadcast_message.areas = {"areas": data["areas"], "simple_polygons": data["simple_polygons"]} + if 'areas' in areas and 'simple_polygons' in areas: + broadcast_message.areas = areas dao_save_object(broadcast_message) diff --git a/app/models.py b/app/models.py index cda2b3509..e94277e58 100644 --- a/app/models.py +++ b/app/models.py @@ -2336,6 +2336,11 @@ class BroadcastMessage(db.Model): self._personalisation = encryption.encrypt(personalisation or {}) def serialize(self): + # TEMPORARY: while we repurpose "areas" + areas_2 = dict(self.areas) + areas_2["simple_polygons"] = areas_2.get("simple_polygons", []) + areas_2["areas"] = areas_2.get("areas", []) + return { 'id': str(self.id), 'reference': self.reference, @@ -2348,6 +2353,8 @@ class BroadcastMessage(db.Model): 'personalisation': self.personalisation if self.template else None, '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", []), diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index 5c94fd6e1..1e927d5de 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -51,6 +51,7 @@ def test_get_broadcast_message(admin_request, sample_broadcast_service): assert response['created_at'] is not None assert response['starts_at'] is None assert response['areas'] == ['place A', 'region B'] + assert response['areas_2']['areas'] == ['place A', 'region B'] assert response['personalisation'] == {'thing': 'test'} @@ -80,6 +81,7 @@ def test_get_broadcast_message_without_template(admin_request, sample_broadcast_ assert response['created_at'] is not None assert response['starts_at'] is None assert response['areas'] == ['place A', 'region B'] + assert response['areas_2']['areas'] == ['place A', 'region B'] assert response['personalisation'] is None @@ -153,6 +155,8 @@ def test_create_broadcast_message(admin_request, sample_broadcast_service, train assert response['created_by_id'] == str(t.created_by_id) assert response['personalisation'] == {} assert response['areas'] == [] + assert response['areas_2']['areas'] == [] + assert response['areas_2']['simple_polygons'] == [] assert response['content'] == 'Some content\n€ŷŵ~\n\'\'""---' broadcast_message = dao_get_broadcast_message_by_id_and_service_id(response["id"], sample_broadcast_service.id) @@ -354,12 +358,29 @@ 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_2": { + "areas": ["london", "glasgow"], + "simple_polygons": [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] + }, + } +]) @pytest.mark.parametrize('status', [ BroadcastStatusType.DRAFT, BroadcastStatusType.PENDING_APPROVAL, BroadcastStatusType.REJECTED, ]) -def test_update_broadcast_message_allows_edit_while_not_yet_live(admin_request, sample_broadcast_service, status): +def test_update_broadcast_message_allows_edit_while_not_yet_live( + area_data, + admin_request, + sample_broadcast_service, + status +): t = create_template(sample_broadcast_service, BROADCAST_TYPE) bm = create_broadcast_message( t, @@ -371,8 +392,7 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(admin_request, 'broadcast_message.update_broadcast_message', _data={ 'starts_at': '2020-06-01 20:00:01', - 'areas': ['london', 'glasgow'], - "simple_polygons": [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]] + **area_data, }, service_id=t.service_id, broadcast_message_id=bm.id, @@ -381,7 +401,9 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(admin_request, assert response['starts_at'] == '2020-06-01T20:00:01.000000Z' assert response['areas'] == ['london', 'glasgow'] + assert response['areas_2']['areas'] == ['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 @@ -389,6 +411,10 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(admin_request, False, pytest.param(True, marks=pytest.mark.xfail) ]) +@pytest.mark.parametrize('area_data', [ # TEMPORARY: while we repurpose "areas" + {'areas': ['london', 'glasgow']}, + {'areas_2': {'areas': ['london', 'glasgow']}}, +]) @pytest.mark.parametrize('status', [ BroadcastStatusType.BROADCASTING, BroadcastStatusType.CANCELLED, @@ -396,8 +422,9 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(admin_request, BroadcastStatusType.TECHNICAL_FAILURE, ]) def test_update_broadcast_message_doesnt_allow_edits_after_broadcast_goes_live( - admin_request, force_override, + area_data, + admin_request, sample_broadcast_service, status ): @@ -406,7 +433,7 @@ def test_update_broadcast_message_doesnt_allow_edits_after_broadcast_goes_live( response = admin_request.post( 'broadcast_message.update_broadcast_message', - _data={'areas': ['london', 'glasgow'], 'force_override': force_override}, + _data={'force_override': force_override, **area_data}, service_id=t.service_id, broadcast_message_id=bm.id, _expected_status=400 @@ -418,7 +445,7 @@ 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={"areas": ["london"], "simple_polygons": [[[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]]} ) response = admin_request.post( @@ -456,15 +483,26 @@ def test_update_broadcast_message_allows_sensible_datetime_formats(admin_request assert response['updated_at'] is not None -def test_update_broadcast_message_doesnt_let_you_update_status(admin_request, sample_broadcast_service): +@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_2": { + "areas": ["glasgow"], + "simple_polygons": [[[55.86, -4.25], [55.85, -4.25], [55.87, -4.24]]], + } + }, +]) +def test_update_broadcast_message_doesnt_let_you_update_status(area_data, admin_request, sample_broadcast_service): t = create_template(sample_broadcast_service, BROADCAST_TYPE) bm = create_broadcast_message(t) response = admin_request.post( 'broadcast_message.update_broadcast_message', _data={ - 'areas': ['glasgow'], - "simple_polygons": [[[55.86, -4.25], [55.85, -4.25], [55.87, -4.24]]], + **area_data, 'status': BroadcastStatusType.BROADCASTING}, service_id=t.service_id, broadcast_message_id=bm.id, @@ -478,8 +516,10 @@ def test_update_broadcast_message_doesnt_let_you_update_status(admin_request, sa @pytest.mark.parametrize("incomplete_area_data", [ - {"areas": ["cardiff"]}, - {"simple_polygons": [[[51.28, -3.11], [51.29, -3.12], [51.27, -3.10]]]}, + {"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_2": {"areas": ["cardiff"]}}, + {"areas_2": {"simple_polygons": [[[51.28, -3.11], [51.29, -3.12], [51.27, -3.10]]]}}, ]) def test_update_broadcast_message_doesnt_let_you_update_areas_but_not_polygons( admin_request, sample_broadcast_service, incomplete_area_data @@ -516,13 +556,26 @@ def test_update_broadcast_message_status(admin_request, sample_broadcast_service assert response['updated_at'] is not None -def test_update_broadcast_message_status_doesnt_let_you_update_other_things(admin_request, sample_broadcast_service): +@pytest.mark.parametrize(('area_data', 'error'), [ # TEMPORARY: while we repurpose "areas" + ({'areas': ['glasgow']}, 'areas was unexpected'), + ({'areas_2': {'areas': ['glasgow']}}, 'areas_2 was unexpected') +]) +def test_update_broadcast_message_status_doesnt_let_you_update_other_things( + area_data, + error, + admin_request, + sample_broadcast_service +): t = create_template(sample_broadcast_service, BROADCAST_TYPE) bm = create_broadcast_message(t) response = admin_request.post( 'broadcast_message.update_broadcast_message_status', - _data={'areas': ['glasgow'], 'status': BroadcastStatusType.BROADCASTING, 'created_by': str(t.created_by_id)}, + _data={ + **area_data, + 'status': BroadcastStatusType.BROADCASTING, + 'created_by': str(t.created_by_id) + }, service_id=t.service_id, broadcast_message_id=bm.id, _expected_status=400 @@ -530,7 +583,7 @@ def test_update_broadcast_message_status_doesnt_let_you_update_other_things(admi assert response['errors'] == [{ 'error': 'ValidationError', - 'message': 'Additional properties are not allowed (areas was unexpected)' + 'message': f'Additional properties are not allowed ({error})', }] diff --git a/tests/app/v2/broadcast/test_post_broadcast.py b/tests/app/v2/broadcast/test_post_broadcast.py index 0fd8de0c2..ddbedaba0 100644 --- a/tests/app/v2/broadcast/test_post_broadcast.py +++ b/tests/app/v2/broadcast/test_post_broadcast.py @@ -84,9 +84,13 @@ 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'] == [ 'River Steeping in Wainfleet All Saints' ] + assert response_json['areas_2']['areas'] == [ + 'River Steeping in Wainfleet All Saints' + ] assert response_json['cancelled_at'] is None assert response_json['cancelled_by_id'] is None assert response_json['content'].startswith( @@ -102,10 +106,19 @@ def test_valid_post_cap_xml_broadcast_returns_201( assert response_json['id'] == ANY assert response_json['personalisation'] is None 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_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] + assert response_json['areas_2']['simple_polygons'][0][-1] == [53.10562, 0.244127] + assert response_json['areas_2']['simple_polygons'][0][-1] == [53.10562, 0.244127] + assert response_json['starts_at'] is None assert response_json['status'] == 'pending-approval' assert response_json['template_id'] is None