darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-06-21 05:30:55 -04:00
Code Issues Packages Projects Releases Wiki Activity

Introduce "areas_2" so we can repurpose "areas"

Browse Source 
Currently we have:

- An "areas" column in the DB that stores a JSON blob.
- An "areas" field inside the "areas" JSON that stores area IDs.
- Each field has to be manually copied into the JSON column.

We want to move to:

- An "areas" column in the DB (unchanged).
- An "ids" field inside the "areas" JSON (to replace "areas").
- The Admin app sending other data inside an "areas" JSON field.

The API design for areas is confusing and difficult to extend.
Here we duplicate the current API functionality using an "areas_2"
field. Once the Admin app is using this field, we'll be able to
rename it to just "areas", which is where we want to get to.

In the next commits we'll build on this to support the migration
from "areas"."areas" to "areas"."ids".
This commit is contained in:
Ben Thorner
2021-08-25 13:49:18 +01:00
parent 277db4e6c7
commit fd7ebbebb0
5 changed files with 119 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/broadcast_message/broadcast_message_schema.py
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': [],

34
app/broadcast_message/rest.py
View File

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

7
app/models.py
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["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", []),

81
tests/app/broadcast_message/test_rest.py
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']['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})',
}]

13
tests/app/v2/broadcast/test_post_broadcast.py
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']['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