Merge pull request #3314 from alphagov/next-stage-areas-migration-178986763

Switch "areas" field to "areas_2" format
2026-07-30 03:08:32 -04:00 · 2021-09-01 16:16:30 +01:00
parent 6a2cd7e9bf bf0bf4e31c
commit be7272b44f
10 changed files with 90 additions and 79 deletions
--- 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"}},
    },
--- 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)
--- 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]}"'
            '',
--- 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,

--- 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']
            ],
--- 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')

--- 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']

--- 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)
    ]
--- 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
    )
--- 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]