Don't create broadcast event unless necessary

If we're not going to send a broadcast, we don't need to create the
BroadcastEvent in the database. The BroadcastMessage contains all the
data we need - the BroadcastEvent is not used.

Not creating the event when we won't send the broadcast (e.g. when the
broadcast message was created when the service was in trial mode) adds
an extra layer of security.

app/broadcast_message/rest.py

@@ -192,29 +192,29 @@ def _create_broadcast_event(broadcast_message):
     """
     Creates a broadcast event, stores it in the database, and triggers the task to send the CAP XML off
     """
-    msg_types = {
-        BroadcastStatusType.BROADCASTING: BroadcastEventMessageType.ALERT,
-        BroadcastStatusType.CANCELLED: BroadcastEventMessageType.CANCEL,
-    }
-
-    event = BroadcastEvent(
-        service=broadcast_message.service,
-        broadcast_message=broadcast_message,
-        message_type=msg_types[broadcast_message.status],
-        transmitted_content={"body": broadcast_message.content},
-        transmitted_areas=broadcast_message.areas,
-        # TODO: Probably move this somewhere more standalone too and imply that it shouldn't change. Should it include
-        # a service based identifier too? eg "flood-warnings@notifications.service.gov.uk" or similar
-        transmitted_sender='notifications.service.gov.uk',
-
-        # TODO: Should this be set to now? Or the original starts_at?
-        transmitted_starts_at=broadcast_message.starts_at,
-        transmitted_finishes_at=broadcast_message.finishes_at,
-    )
-
-    dao_save_object(event)
-
     if not broadcast_message.stubbed:
+        msg_types = {
+            BroadcastStatusType.BROADCASTING: BroadcastEventMessageType.ALERT,
+            BroadcastStatusType.CANCELLED: BroadcastEventMessageType.CANCEL,
+        }
+
+        event = BroadcastEvent(
+            service=broadcast_message.service,
+            broadcast_message=broadcast_message,
+            message_type=msg_types[broadcast_message.status],
+            transmitted_content={"body": broadcast_message.content},
+            transmitted_areas=broadcast_message.areas,
+            # TODO: Probably move this somewhere more standalone too and imply that it shouldn't change. Should it
+            # include a service based identifier too? eg "flood-warnings@notifications.service.gov.uk" or similar
+            transmitted_sender='notifications.service.gov.uk',
+
+            # TODO: Should this be set to now? Or the original starts_at?
+            transmitted_starts_at=broadcast_message.starts_at,
+            transmitted_finishes_at=broadcast_message.finishes_at,
+        )
+
+        dao_save_object(event)
+
         send_broadcast_event.apply_async(
             kwargs={'broadcast_event_id': str(event.id)},
             queue=QueueNames.BROADCASTS

tests/app/broadcast_message/test_rest.py

@@ -591,17 +591,10 @@ def test_update_broadcast_message_status_updates_details_but_does_not_queue_task
     assert response['approved_at'] is not None
     assert response['approved_by_id'] == str(approver.id)
 
-    assert len(bm.events) == 1
-    alert_event = bm.events[0]
-
+    # The broadcast can be approved, but does not create a broadcast_event in the database or put a task on the queue
+    assert len(bm.events) == 0
     assert len(mock_task.mock_calls) == 0
 
-    assert alert_event.service_id == sample_broadcast_service.id
-    assert alert_event.transmitted_areas == bm.areas
-    assert alert_event.message_type == BroadcastEventMessageType.ALERT
-    assert alert_event.transmitted_finishes_at == bm.finishes_at
-    assert alert_event.transmitted_content == {"body": "emergency broadcast"}
-
 
 def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template(
     admin_request,