From a4c20e8ba64d631b3ca0e16b072edfde8c224f98 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 14 Jan 2022 16:46:20 +0000 Subject: [PATCH] Return 404 if reference from cancel message does not match If the reference from cancel CAP XML we received via API does not match with any existing broadcast, return 404. Do the same if service id doesn't match. Also refactor code to cancel broadcast out into separate function It should be a separate function that is only called by create_broadcast function. This will prevent create_broadcast from becoming too big and complex and doing too many things. --- app/dao/broadcast_message_dao.py | 3 +- app/v2/broadcast/post_broadcast.py | 43 +++++++++++++------ .../v2/broadcast/sample_cap_xml_documents.py | 2 +- tests/app/v2/broadcast/test_post_broadcast.py | 34 ++++++++++++++- 4 files changed, 65 insertions(+), 17 deletions(-) diff --git a/app/dao/broadcast_message_dao.py b/app/dao/broadcast_message_dao.py index a42c61a40..087702eb4 100644 --- a/app/dao/broadcast_message_dao.py +++ b/app/dao/broadcast_message_dao.py @@ -24,9 +24,10 @@ def dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service ).one() -def dao_get_broadcast_message_by_references(references_to_original_broadcast): +def dao_get_broadcast_message_by_references_and_service_id(references_to_original_broadcast, service_id): return BroadcastMessage.query.filter( BroadcastMessage.reference.in_(references_to_original_broadcast), + BroadcastMessage.service_id == service_id ).one() diff --git a/app/v2/broadcast/post_broadcast.py b/app/v2/broadcast/post_broadcast.py index 7f1259040..404b511ea 100644 --- a/app/v2/broadcast/post_broadcast.py +++ b/app/v2/broadcast/post_broadcast.py @@ -3,6 +3,7 @@ from itertools import chain from flask import current_app, jsonify, request from notifications_utils.polygons import Polygons from notifications_utils.template import BroadcastMessageTemplate +from sqlalchemy.orm.exc import NoResultFound from app import api_user, authenticated_service from app.broadcast_message.rest import ( @@ -10,7 +11,7 @@ from app.broadcast_message.rest import ( ) from app.broadcast_message.translators import cap_xml_to_dict from app.dao.broadcast_message_dao import ( - dao_get_broadcast_message_by_references, + dao_get_broadcast_message_by_references_and_service_id, ) from app.dao.dao_utils import dao_save_object from app.models import BROADCAST_TYPE, BroadcastMessage, BroadcastStatusType @@ -48,18 +49,9 @@ def create_broadcast(): validate(broadcast_json, post_broadcast_schema) if broadcast_json["msgType"] == "Cancel": - references_to_original_broadcast = broadcast_json["references"].split(",") - broadcast_message = dao_get_broadcast_message_by_references(references_to_original_broadcast) - # do we need to check if service is active? - if broadcast_message.status == BroadcastStatusType.PENDING_APPROVAL: - new_status = BroadcastStatusType.REJECTED - else: - new_status = BroadcastStatusType.CANCELLED - validate_and_update_broadcast_message_status( - broadcast_message, - new_status, - updating_user=None, - from_api=True + broadcast_message = _cancel_or_reject_broadcast( + broadcast_json["references"].split(","), + authenticated_service.id ) return jsonify(broadcast_message.serialize()), 201 @@ -108,6 +100,31 @@ def create_broadcast(): return jsonify(broadcast_message.serialize()), 201 +def _cancel_or_reject_broadcast(references_to_original_broadcast, service_id): + try: + broadcast_message = dao_get_broadcast_message_by_references_and_service_id( + references_to_original_broadcast, + service_id + ) + except NoResultFound: + raise BadRequestError( + message="Broadcast message reference and service id didn't match with any existing broadcasts", + status_code=404, + ) + # do we need to check if service is active? + if broadcast_message.status == BroadcastStatusType.PENDING_APPROVAL: + new_status = BroadcastStatusType.REJECTED + else: + new_status = BroadcastStatusType.CANCELLED + validate_and_update_broadcast_message_status( + broadcast_message, + new_status, + updating_user=None, + from_api=True + ) + return broadcast_message + + def _validate_template(broadcast_json): template = BroadcastMessageTemplate.from_content( broadcast_json['content'] diff --git a/tests/app/v2/broadcast/sample_cap_xml_documents.py b/tests/app/v2/broadcast/sample_cap_xml_documents.py index 518db8cf7..8eed119d2 100644 --- a/tests/app/v2/broadcast/sample_cap_xml_documents.py +++ b/tests/app/v2/broadcast/sample_cap_xml_documents.py @@ -189,7 +189,7 @@ WITH_PLACEHOLDER_FOR_CONTENT = """ WINDEMERE = """ - 50385fcb0ab7aa447bbd46d848ce8466E + 4f6d28b10ab7aa447bbd46d85f1e9effE www.gov.uk/environment-agency 2020-02-16T23:01:13-00:00 Actual diff --git a/tests/app/v2/broadcast/test_post_broadcast.py b/tests/app/v2/broadcast/test_post_broadcast.py index c33ceec61..f4a5e25c3 100644 --- a/tests/app/v2/broadcast/test_post_broadcast.py +++ b/tests/app/v2/broadcast/test_post_broadcast.py @@ -205,8 +205,38 @@ def test_valid_cancel_broadcast_request_cancels_active_alert_and_returns_201( assert broadcast_message.updated_at is not None -def test_cancel_request_does_not_cancel_broadcast_if_reference_does_not_match(): - pass +def test_cancel_request_does_not_cancel_broadcast_if_reference_does_not_match( + client, + sample_broadcast_service +): + auth_header = create_service_authorization_header(service_id=sample_broadcast_service.id) + + # create a broadcast + response_for_create = client.post( + path='/v2/broadcast', + data=sample_cap_xml_documents.WINDEMERE, + headers=[('Content-Type', 'application/cap+xml'), auth_header], + ) + assert response_for_create.status_code == 201 + + response_json_for_create = json.loads(response_for_create.get_data(as_text=True)) + + assert response_json_for_create['cancelled_at'] is None + assert response_json_for_create['cancelled_by_id'] is None + assert response_json_for_create['reference'] == '4f6d28b10ab7aa447bbd46d85f1e9effE' + assert response_json_for_create['status'] == 'pending-approval' + + # try to cancel broadcast, but reference doesn't match + response_for_cancel = client.post( + path='/v2/broadcast', + data=sample_cap_xml_documents.WAINFLEET_CANCEL, + headers=[('Content-Type', 'application/cap+xml'), auth_header], + ) + + assert response_for_cancel.status_code == 404 + response = json.loads(response_for_cancel.get_data(as_text=True)) + expected_error_message = "Broadcast message reference and service id didn't match with any existing broadcasts" + assert response["errors"][0]["message"] == expected_error_message def test_large_polygon_is_simplified(