Merge pull request #3312 from alphagov/prepare-broadcast-refactor-178986763

Prepare to enhance area data for broadcasts
This commit is contained in:
Ben Thorner
2021-08-26 15:41:21 +01:00
committed by GitHub
9 changed files with 142 additions and 26 deletions

View File

@@ -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': [],

View File

@@ -133,12 +133,27 @@ 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": 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,9 +185,24 @@ 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"]
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"]
if ('ids' in areas and 'simple_polygons' not in areas) or ('ids' not in areas and 'simple_polygons' in areas):
raise InvalidRequest(
f'Cannot update broadcast_message {broadcast_message.id}, areas or polygons are missing.',
f'Cannot update broadcast_message {broadcast_message.id}, area IDs or polygons are missing.',
status_code=400
)
@@ -182,8 +212,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)

View File

@@ -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("areas")}.',
f'This broadcast is targeted at areas {broadcast_message.areas.get("ids", broadcast_message.areas.get("areas"))}.', # noqa
''
f'This broadcast\'s content starts "{broadcast_message.content[:100]}"'
'',

View File

@@ -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["ids"] = areas_2.pop("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", []),

View File

@@ -70,6 +70,9 @@ def create_broadcast():
'areas': [
area['name'] for area in broadcast_json['areas']
],
'names': [
area['name'] for area in broadcast_json['areas']
],
'simple_polygons': polygons.smooth.simplify.as_coordinate_pairs_long_lat,
},
status=BroadcastStatusType.PENDING_APPROVAL,

View File

@@ -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']['ids'] == ['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']['ids'] == ['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']['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)
@@ -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": {
"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(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']['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
@@ -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': {'ids': ['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": {
"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):
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": {"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
@@ -497,7 +537,7 @@ def test_update_broadcast_message_doesnt_let_you_update_areas_but_not_polygons(
assert response[
'message'
] == f'Cannot update broadcast_message {broadcast_message.id}, areas or polygons are missing.'
] == f'Cannot update broadcast_message {broadcast_message.id}, area IDs or polygons are missing.'
def test_update_broadcast_message_status(admin_request, sample_broadcast_service):
@@ -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': {'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
):
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})',
}]
@@ -584,7 +637,11 @@ def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_
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={
"areas": ["london"],
"ids": ["london"],
"simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]
}
)
approver = create_user(email='approver@gov.uk')
sample_broadcast_service.users.append(approver)
@@ -683,7 +740,11 @@ def test_update_broadcast_message_status_creates_event_with_correct_content_if_b
template=None,
content='tailor made emergency broadcast content',
status=BroadcastStatusType.PENDING_APPROVAL,
areas={"areas": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]}
areas={
"areas": ["london"],
"ids": ["london"],
"simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]
}
)
approver = create_user(email='approver@gov.uk')
sample_broadcast_service.users.append(approver)

View File

@@ -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': [], 'simple_polygons': []}, 'broadcasting', datetime(2021, 6, 20, 12, 0),
{'areas': [], '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': [], 'simple_polygons': []}, 'cancelled', datetime(2021, 6, 15, 12, 0),
{'areas': [], 'ids': [], 'simple_polygons': []}, 'cancelled', datetime(2021, 6, 15, 12, 0),
None, None, None)
]

View File

@@ -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': [], 'simple_polygons': []},
areas=areas or {'areas': [], 'ids': [], 'simple_polygons': []},
content=content,
stubbed=stubbed
)

View File

@@ -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']['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(
@@ -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