Return 400 if references missing from cancel broadcast

If someone tries to cancel a broadcast but the references don’t match
and existing broadcast we correctly return a 404.

If they don’t provide any references then we get an exception. This
commit catches the missing references and returns a 400. I think this
is more appropriate because it’s malformed request, rather than a
well-formed request that doesn’t match our data. It also lets us write a
more specific and helpful error message.

app/broadcast_message/translators.py

@@ -7,7 +7,10 @@ def cap_xml_to_dict(cap_xml):
     return {
         "msgType": cap.alert.msgType.text,
         "reference": cap.alert.identifier.text,
-        "references": cap.alert.references.text,  # references to previous events belonging to the same alert
+        "references": (
+            # references to previous events belonging to the same alert
+            cap.alert.references.text if cap.alert.references else None
+        ),
         "cap_event": cap.alert.info.event.text,
         "category": cap.alert.info.category.text,
         "expires": cap.alert.info.expires.text,

app/v2/broadcast/post_broadcast.py

@@ -48,6 +48,11 @@ def create_broadcast():
     validate(broadcast_json, post_broadcast_schema)
 
     if broadcast_json["msgType"] == "Cancel":
+        if broadcast_json["references"] is None:
+            raise BadRequestError(
+                message='Missing <references>',
+                status_code=400,
+            )
         broadcast_message = _cancel_or_reject_broadcast(
             broadcast_json["references"].split(","),
             authenticated_service.id

tests/app/v2/broadcast/sample_cap_xml_documents.py

@@ -45,7 +45,7 @@ WAINFLEET_CANCEL = """
         <source>Flood warning service</source>
         <scope>Public</scope>
         <code></code>
-        <references>www.gov.uk/environment-agency,50385fcb0ab7aa447bbd46d848ce8466E,2020-02-16T23:01:13-00:00</references>
+        {}
         <info>
                 <language>en-GB</language>
                 <category>Met</category>
@@ -70,6 +70,16 @@ WAINFLEET_CANCEL = """
     </alert>
 """
 
+WAINFLEET_CANCEL_WITH_REFERENCES = WAINFLEET_CANCEL.format(
+    "<references>www.gov.uk/environment-agency,50385fcb0ab7aa447bbd46d848ce8466E,2020-02-16T23:01:13-00:00</references>"
+)
+WAINFLEET_CANCEL_WITH_MISSING_REFERENCES = WAINFLEET_CANCEL.format(
+    ""
+)
+WAINFLEET_CANCEL_WITH_EMPTY_REFERENCES = WAINFLEET_CANCEL.format(
+    "<references></references>"
+)
+
 UPDATE = """
     <alert xmlns="urn:oasis:names:tc:emergency:cap:1.2">
       <identifier>PAAQ-4-mg5a94</identifier>

tests/app/v2/broadcast/test_post_broadcast.py

@@ -156,7 +156,7 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess
     # cancel broadcast
     response_for_cancel = client.post(
         path='/v2/broadcast',
-        data=sample_cap_xml_documents.WAINFLEET_CANCEL,
+        data=sample_cap_xml_documents.WAINFLEET_CANCEL_WITH_REFERENCES,
         headers=[('Content-Type', 'application/cap+xml'), auth_header],
     )
     assert response_for_cancel.status_code == 201
@@ -167,9 +167,29 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess
     )
 
 
+@pytest.mark.parametrize('cap_xml_document, expected_status, expected_error', (
+    (
+        sample_cap_xml_documents.WAINFLEET_CANCEL_WITH_REFERENCES,
+        404,
+        [{'error': 'NoResultFound', 'message': 'No result found'}],
+    ),
+    (
+        sample_cap_xml_documents.WAINFLEET_CANCEL_WITH_EMPTY_REFERENCES,
+        404,
+        [{'error': 'NoResultFound', 'message': 'No result found'}],
+    ),
+    (
+        sample_cap_xml_documents.WAINFLEET_CANCEL_WITH_MISSING_REFERENCES,
+        400,
+        [{'error': 'BadRequestError', 'message': 'Missing <references>'}],
+    ),
+))
 def test_cancel_request_does_not_cancel_broadcast_if_reference_does_not_match(
     client,
-    sample_broadcast_service
+    sample_broadcast_service,
+    cap_xml_document,
+    expected_status,
+    expected_error,
 ):
     auth_header = create_service_authorization_header(service_id=sample_broadcast_service.id)
 
@@ -191,11 +211,13 @@ def test_cancel_request_does_not_cancel_broadcast_if_reference_does_not_match(
     # try to cancel broadcast, but reference doesn't match
     response_for_cancel = client.post(
         path='/v2/broadcast',
-        data=sample_cap_xml_documents.WAINFLEET_CANCEL,
+        data=cap_xml_document,
         headers=[('Content-Type', 'application/cap+xml'), auth_header],
     )
+    response_for_cancel_json = json.loads(response_for_cancel.get_data(as_text=True))
 
-    assert response_for_cancel.status_code == 404
+    assert response_for_cancel.status_code == expected_status
+    assert response_for_cancel_json["errors"] == expected_error
 
 
 def test_cancel_request_does_not_cancel_broadcast_if_service_id_does_not_match(
@@ -224,7 +246,7 @@ def test_cancel_request_does_not_cancel_broadcast_if_service_id_does_not_match(
     auth_header_2 = create_service_authorization_header(service_id=sample_broadcast_service_2.id)
     response_for_cancel = client.post(
         path='/v2/broadcast',
-        data=sample_cap_xml_documents.WAINFLEET_CANCEL,
+        data=sample_cap_xml_documents.WAINFLEET_CANCEL_WITH_REFERENCES,
         headers=[('Content-Type', 'application/cap+xml'), auth_header_2],
     )