diff --git a/app/broadcast_message/broadcast_message_schema.py b/app/broadcast_message/broadcast_message_schema.py index 743ca54d7..3d617533a 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"}}, + 'simple_polygons': {"type": "array", "items": {"type": "array"}}, }, 'required': ['template_id', 'service_id', 'created_by'], 'additionalProperties': False @@ -29,6 +30,7 @@ update_broadcast_message_schema = { 'starts_at': {'type': 'string', 'format': 'datetime'}, 'finishes_at': {'type': 'string', 'format': 'datetime'}, 'areas': {"type": "array", "items": {"type": "string"}}, + 'simple_polygons': {"type": "array", "items": {"type": "array"}}, }, 'required': [], 'additionalProperties': False diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index db7f459c8..b034b08b3 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -59,6 +59,11 @@ def _update_broadcast_message(broadcast_message, new_status, updating_user): f'User {updating_user.id} cannot approve their own broadcast_message {broadcast_message.id}', status_code=400 ) + elif not broadcast_message.areas: + raise InvalidRequest( + f'broadcast_message {broadcast_message.id} has no selected areas and so cannot be broadcasted.', + status_code=400 + ) else: broadcast_message.approved_at = datetime.utcnow() broadcast_message.approved_by = updating_user @@ -101,7 +106,7 @@ def create_broadcast_message(service_id): template_id=template.id, template_version=template.version, personalisation=data.get('personalisation', {}), - areas=data.get('areas', []), + areas={"areas": data.get("areas", []), "simple_polygons": data.get("simple_polygons", [])}, status=BroadcastStatusType.DRAFT, starts_at=_parse_nullable_datetime(data.get('starts_at')), finishes_at=_parse_nullable_datetime(data.get('finishes_at')), @@ -127,14 +132,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): + raise InvalidRequest( + f'Cannot update broadcast_message {broadcast_message.id}, areas or polygons are missing.', + status_code=400 + ) + if 'personalisation' in data: broadcast_message.personalisation = data['personalisation'] if 'starts_at' in data: 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: - broadcast_message.areas = data['areas'] + if 'areas' in data and 'simple_polygons' in data: + broadcast_message.areas = {"areas": data["areas"], "simple_polygons": data["simple_polygons"]} dao_save_object(broadcast_message) diff --git a/app/models.py b/app/models.py index 72e900af4..4d98f6545 100644 --- a/app/models.py +++ b/app/models.py @@ -2263,7 +2263,8 @@ class BroadcastMessage(db.Model): 'template_name': self.template.name, 'personalisation': self.personalisation, - 'areas': self.areas, + 'areas': self.areas.get("areas", []), + 'simple_polygons': self.areas.get("simple_polygons", []), 'status': self.status, diff --git a/migrations/versions/0329_purge_broadcast_data.py b/migrations/versions/0329_purge_broadcast_data.py new file mode 100644 index 000000000..b8c698c53 --- /dev/null +++ b/migrations/versions/0329_purge_broadcast_data.py @@ -0,0 +1,24 @@ +""" + +Revision ID: 0329_purge_broadcast_data +Revises: 0328_international_letters_perm +Create Date: 2020-09-07 16:00:27.545673 + +""" +from alembic import op + + +revision = '0329_purge_broadcast_data' +down_revision = '0328_international_letters_perm' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.execute("TRUNCATE broadcast_event, broadcast_message;") + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + pass + # ### end Alembic commands ### diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index d9c04cc9d..29cd892af 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -11,7 +11,10 @@ from tests.app.db import create_broadcast_message, create_template, create_servi def test_get_broadcast_message(admin_request, sample_service): t = create_template(sample_service, BROADCAST_TYPE) - bm = create_broadcast_message(t, areas=['place A', 'region B']) + bm = create_broadcast_message(t, areas={ + "areas": ['place A', 'region B'], + "simple_polygons": [[50.1, 1.2], [50.12, 1.2]] + }) response = admin_request.get( 'broadcast_message.get_broadcast_message', @@ -140,11 +143,19 @@ def test_create_broadcast_message_400s_if_json_schema_fails_validation( ]) def test_update_broadcast_message_allows_edit_while_not_yet_live(admin_request, sample_service, status): t = create_template(sample_service, BROADCAST_TYPE) - bm = create_broadcast_message(t, areas=['manchester'], status=status) + bm = create_broadcast_message( + t, + areas={"areas": ['manchester'], "simple_polygons": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]}, + status=status + ) response = admin_request.post( 'broadcast_message.update_broadcast_message', - _data={'starts_at': '2020-06-01 20:00:01', 'areas': ['london', 'glasgow']}, + _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]] + }, service_id=t.service_id, broadcast_message_id=bm.id, _expected_status=200 @@ -152,6 +163,7 @@ 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['simple_polygons'] == [[51.12, 0.2], [50.13, 0.4], [50.14, 0.45]] assert response['updated_at'] is not None @@ -177,7 +189,10 @@ def test_update_broadcast_message_doesnt_allow_edits_after_broadcast_goes_live(a def test_update_broadcast_message_sets_finishes_at_separately(admin_request, sample_service): t = create_template(sample_service, BROADCAST_TYPE) - bm = create_broadcast_message(t, areas=['manchester']) + bm = create_broadcast_message( + t, + areas={"areas": ['london'], "simple_polygons": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]]} + ) response = admin_request.post( 'broadcast_message.update_broadcast_message', @@ -220,7 +235,10 @@ def test_update_broadcast_message_doesnt_let_you_update_status(admin_request, sa response = admin_request.post( 'broadcast_message.update_broadcast_message', - _data={'areas': ['glasgow'], 'status': BroadcastStatusType.BROADCASTING}, + _data={ + 'areas': ['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 @@ -232,6 +250,29 @@ 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]]}, +]) +def test_update_broadcast_message_doesnt_let_you_update_areas_but_not_polygons( + admin_request, sample_service, incomplete_area_data +): + template = create_template(sample_service, BROADCAST_TYPE) + broadcast_message = create_broadcast_message(template) + + response = admin_request.post( + 'broadcast_message.update_broadcast_message', + _data=incomplete_area_data, + service_id=template.service_id, + broadcast_message_id=broadcast_message.id, + _expected_status=400 + ) + + assert response[ + 'message' + ] == f'Cannot update broadcast_message {broadcast_message.id}, areas or polygons are missing.' + + def test_update_broadcast_message_status(admin_request, sample_service): t = create_template(sample_service, BROADCAST_TYPE) bm = create_broadcast_message(t, status=BroadcastStatusType.DRAFT) @@ -304,7 +345,11 @@ def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_ mocker ): t = create_template(sample_service, BROADCAST_TYPE, content='emergency broadcast') - bm = create_broadcast_message(t, status=BroadcastStatusType.PENDING_APPROVAL) + 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]]} + ) approver = create_user(email='approver@gov.uk') sample_service.users.append(approver) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') @@ -354,6 +399,31 @@ def test_update_broadcast_message_status_rejects_approval_from_creator( assert f'cannot approve their own broadcast' in response['message'] +def test_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_areas( + admin_request, + sample_service, + mocker +): + template = create_template(sample_service, BROADCAST_TYPE) + broadcast = create_broadcast_message(template, status=BroadcastStatusType.PENDING_APPROVAL) + approver = create_user(email='approver@gov.uk') + sample_service.users.append(approver) + mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') + + response = admin_request.post( + 'broadcast_message.update_broadcast_message_status', + _data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(approver.id)}, + service_id=template.service_id, + broadcast_message_id=broadcast.id, + _expected_status=400 + ) + + assert mock_task.called is False + assert response[ + 'message' + ] == f'broadcast_message {broadcast.id} has no selected areas and so cannot be broadcasted.' + + def test_update_broadcast_message_status_allows_platform_admin_to_approve_own_message( notify_db, admin_request, @@ -363,7 +433,11 @@ def test_update_broadcast_message_status_allows_platform_admin_to_approve_own_me user = sample_service.created_by user.platform_admin = True t = create_template(sample_service, BROADCAST_TYPE) - bm = create_broadcast_message(t, status=BroadcastStatusType.PENDING_APPROVAL) + 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]]} + ) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') response = admin_request.post( @@ -392,7 +466,11 @@ def test_update_broadcast_message_status_allows_trial_mode_services_to_approve_o ): sample_service.restricted = True t = create_template(sample_service, BROADCAST_TYPE) - bm = create_broadcast_message(t, status=BroadcastStatusType.PENDING_APPROVAL) + 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]]} + ) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') response = admin_request.post( diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index b39744e2f..e6c82c35a 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -11,7 +11,11 @@ from tests.app.db import create_template, create_broadcast_message, create_broad def test_send_broadcast_message_sends_data_correctly(sample_service): template = create_template(sample_service, BROADCAST_TYPE) - 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 + ) with requests_mock.Mocker() as request_mock: request_mock.post("http://test-cbc-proxy/broadcasts/stub-1", json={'valid': 'true'}, status_code=200) @@ -29,7 +33,11 @@ def test_send_broadcast_message_sends_data_correctly(sample_service): def test_send_broadcast_message_sends_old_version_of_template(sample_service): template = create_template(sample_service, BROADCAST_TYPE, content='first 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 + ) template.content = 'second content' dao_update_template(template) diff --git a/tests/app/db.py b/tests/app/db.py index 75d4edeaa..89e42ec15 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1006,7 +1006,7 @@ def create_broadcast_message( status=BroadcastStatusType.DRAFT, starts_at=None, finishes_at=None, - areas=[], + areas={}, ): broadcast_message = BroadcastMessage( service_id=template.service_id,