Merge pull request #3317 from alphagov/remove-areas-2-support-178986763

Remove support for "areas_2" field
2026-02-02 09:26:08 -05:00 · 2021-09-03 11:16:40 +01:00
parent be7272b44f d50c563f08
commit 3779146cc5
5 changed files with 16 additions and 78 deletions
--- a/app/broadcast_message/broadcast_message_schema.py
+++ b/app/broadcast_message/broadcast_message_schema.py
@@ -14,8 +14,6 @@ create_broadcast_message_schema = {
        'starts_at': {'type': 'string', 'format': 'datetime'},
        'finishes_at': {'type': 'string', 'format': 'datetime'},
        'areas': {'type': 'object'},
-        'areas_2': {'type': 'object'},
-        'simple_polygons': {"type": "array", "items": {"type": "array"}},
        'content': {'type': 'string', 'minLength': 1},
        'reference': {'type': 'string', 'minLength': 1, 'maxLength': 255},
    },
@@ -43,8 +41,6 @@ update_broadcast_message_schema = {
        'starts_at': {'type': 'string', 'format': 'datetime'},
        'finishes_at': {'type': 'string', 'format': 'datetime'},
        'areas': {'type': 'object'},
-        'areas_2': {'type': 'object'},
-        'simple_polygons': {"type": "array", "items": {"type": "array"}},
    },
    'required': [],
    'additionalProperties': False
--- a/app/broadcast_message/rest.py
+++ b/app/broadcast_message/rest.py
@@ -138,7 +138,7 @@ def create_broadcast_message(service_id):
        template_id=template_id,
        template_version=template.version if template else None,
        personalisation=personalisation,
-        areas=data.get("areas", data.get("areas_2", {})),  # TEMPORARY: while we repurpose "areas"
+        areas=data.get("areas", {}),
        status=BroadcastStatusType.DRAFT,
        starts_at=_parse_nullable_datetime(data.get('starts_at')),
        finishes_at=_parse_nullable_datetime(data.get('finishes_at')),
@@ -170,8 +170,7 @@ def update_broadcast_message(service_id, broadcast_message_id):
            status_code=400
        )

-    # TEMPORARY: while we repurpose "areas"
-    areas = data.get("areas", data.get("areas_2", {}))
+    areas = data.get("areas", {})

    if ('ids' in areas and 'simple_polygons' not in areas) or ('ids' not in areas and 'simple_polygons' in areas):
        raise InvalidRequest(
--- a/app/models.py
+++ b/app/models.py
@@ -2353,10 +2353,7 @@ 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,
            'areas': areas,
-
            'status': self.status,

            'starts_at': get_dt_string_or_none(self.starts_at),
--- a/tests/app/broadcast_message/test_rest.py
+++ b/tests/app/broadcast_message/test_rest.py
@@ -61,8 +61,6 @@ def test_get_broadcast_message(
    assert response['starts_at'] is None
    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'}


@@ -102,8 +100,6 @@ def test_get_broadcast_message_without_template(
    assert response['starts_at'] is None
    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


@@ -178,8 +174,6 @@ def test_create_broadcast_message(admin_request, sample_broadcast_service, train
    assert response['personalisation'] == {}
    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\'\'""---'

    broadcast_message = dao_get_broadcast_message_by_id_and_service_id(response["id"], sample_broadcast_service.id)
@@ -381,27 +375,12 @@ def test_create_broadcast_message_400s_if_no_content_or_template(
    )


-@pytest.mark.parametrize('area_data', [  # TEMPORARY: while we repurpose "areas"
-    {
-        "areas": {
-            "ids": ["london", "glasgow"],
-            "simple_polygons": [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]]
-        },
-    },
-    {
-        "areas_2": {
-            "ids": ["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(
-    area_data,
    admin_request,
    sample_broadcast_service,
    status
@@ -419,8 +398,11 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(
    response = admin_request.post(
        'broadcast_message.update_broadcast_message',
        _data={
-            'starts_at': '2020-06-01 20:00:01',
-            **area_data,
+            "starts_at": "2020-06-01 20:00:01",
+            "areas": {
+                "ids": ["london", "glasgow"],
+                "simple_polygons": [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]]
+            },
        },
        service_id=t.service_id,
        broadcast_message_id=bm.id,
@@ -430,8 +412,6 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(
    assert response['starts_at'] == '2020-06-01T20:00:01.000000Z'
    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['areas_2']['simple_polygons'] == [[[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]]]
    assert response['updated_at'] is not None


@@ -439,10 +419,6 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(
    False,
    pytest.param(True, marks=pytest.mark.xfail)
 ])
-@pytest.mark.parametrize('area_data', [  # TEMPORARY: while we repurpose "areas"
-    {'areas': {'ids': ['london', 'glasgow']}},
-    {'areas_2': {'ids': ['london', 'glasgow']}},
-])
@pytest.mark.parametrize('status', [
    BroadcastStatusType.BROADCASTING,
    BroadcastStatusType.CANCELLED,
@@ -451,7 +427,6 @@ def test_update_broadcast_message_allows_edit_while_not_yet_live(
 ])
 def test_update_broadcast_message_doesnt_allow_edits_after_broadcast_goes_live(
    force_override,
-    area_data,
    admin_request,
    sample_broadcast_service,
    status
@@ -461,7 +436,7 @@ def test_update_broadcast_message_doesnt_allow_edits_after_broadcast_goes_live(

    response = admin_request.post(
        'broadcast_message.update_broadcast_message',
-        _data={'force_override': force_override, **area_data},
+        _data={'force_override': force_override, 'areas': {'ids': ['london', 'glasgow']}},
        service_id=t.service_id,
        broadcast_message_id=bm.id,
        _expected_status=400
@@ -514,29 +489,18 @@ def test_update_broadcast_message_allows_sensible_datetime_formats(admin_request
    assert response['updated_at'] is not None


-@pytest.mark.parametrize('area_data', [  # TEMPORARY: while we repurpose "areas"
-    {
-        "areas": {
-            "ids": ["glasgow"],
-            "simple_polygons": [[[55.86, -4.25], [55.85, -4.25], [55.87, -4.24]]],
-        }
-    },
-    {
-        "areas_2": {
-            "ids": ["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):
+def test_update_broadcast_message_doesnt_let_you_update_status(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={
-            **area_data,
-            'status': BroadcastStatusType.BROADCASTING},
+            "areas": {
+                "ids": ["glasgow"],
+                "simple_polygons": [[[55.86, -4.25], [55.85, -4.25], [55.87, -4.24]]],
+            },
+            "status": BroadcastStatusType.BROADCASTING},
        service_id=t.service_id,
        broadcast_message_id=bm.id,
        _expected_status=400
@@ -551,9 +515,6 @@ def test_update_broadcast_message_doesnt_let_you_update_status(area_data, admin_
@pytest.mark.parametrize("incomplete_area_data", [
    {"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]]]}},
 ])
 def test_update_broadcast_message_doesnt_let_you_update_areas_but_not_polygons(
    admin_request, sample_broadcast_service, incomplete_area_data
@@ -590,13 +551,7 @@ def test_update_broadcast_message_status(admin_request, sample_broadcast_service
    assert response['updated_at'] is not None


-@pytest.mark.parametrize(('area_data', 'error'), [  # TEMPORARY: while we repurpose "areas"
-    ({'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(
-    area_data,
-    error,
    admin_request,
    sample_broadcast_service
 ):
@@ -606,7 +561,7 @@ def test_update_broadcast_message_status_doesnt_let_you_update_other_things(
    response = admin_request.post(
        'broadcast_message.update_broadcast_message_status',
        _data={
-            **area_data,
+            'areas': {'ids': ['glasgow']},
            'status': BroadcastStatusType.BROADCASTING,
            'created_by': str(t.created_by_id)
        },
@@ -617,7 +572,7 @@ def test_update_broadcast_message_status_doesnt_let_you_update_other_things(

    assert response['errors'] == [{
        'error': 'ValidationError',
-        'message': f'Additional properties are not allowed ({error})',
+        'message': 'Additional properties are not allowed (areas was unexpected)',
    }]


--- a/tests/app/v2/broadcast/test_post_broadcast.py
+++ b/tests/app/v2/broadcast/test_post_broadcast.py
@@ -87,10 +87,6 @@ def test_valid_post_cap_xml_broadcast_returns_201(
    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'
-    ]
    assert response_json['cancelled_at'] is None
    assert response_json['cancelled_by_id'] is None
    assert response_json['content'].startswith(
@@ -113,11 +109,6 @@ def test_valid_post_cap_xml_broadcast_returns_201(
    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]
-    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'