Merge pull request #3085 from alphagov/fix-broadcast-migration

Fix broadcast migration

app/broadcast_message/rest.py

@@ -189,11 +189,19 @@ def _create_broadcast_event(broadcast_message):
     else:
         transmitted_finishes_at = broadcast_message.finishes_at
 
+    # TODO: Remove this if statement after broadcast message content is guaranteed to always be populated.
+    if broadcast_message.content:
+        content = broadcast_message.content
+    else:
+        content = broadcast_message.template._as_utils_template_with_personalisation(
+            broadcast_message.personalisation
+        ).content_with_placeholders_filled_in
+
     event = BroadcastEvent(
         service=broadcast_message.service,
         broadcast_message=broadcast_message,
         message_type=msg_types[broadcast_message.status],
-        transmitted_content={"body": broadcast_message.content},
+        transmitted_content={"body": 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

migrations/versions/0335_broadcast_msg_content.py

@@ -14,7 +14,7 @@ down_revision = '0334_broadcast_message_number'
 
 
 def upgrade():
-    op.add_column('broadcast_message', sa.Column('content', sa.Text(), nullable=False))
+    op.add_column('broadcast_message', sa.Column('content', sa.Text(), nullable=True))
     op.alter_column('broadcast_message', 'template_id', nullable=True)
     op.alter_column('broadcast_message', 'template_version', nullable=True)
 

tests/app/broadcast_message/test_rest.py

@@ -477,6 +477,43 @@ def test_update_broadcast_message_status_creates_event_with_correct_content_if_b
     assert alert_event.transmitted_content == {"body": "tailor made emergency broadcast content"}
 
 
+# TODO: Remove this unit test after broadcast message content is guaranteed to always be populated.
+def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_content_field(
+    admin_request,
+    sample_broadcast_service,
+    mocker
+):
+    t = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast')
+    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]]]}
+    )
+    # simulate having no content because the broadcast message was created before migration 0335 came in
+    bm.content = None
+
+    approver = create_user(email='approver@gov.uk')
+    sample_broadcast_service.users.append(approver)
+    mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
+
+    response = admin_request.post(
+        'broadcast_message.update_broadcast_message_status',
+        _data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(approver.id)},
+        service_id=t.service_id,
+        broadcast_message_id=bm.id,
+        _expected_status=200
+    )
+
+    assert response['status'] == BroadcastStatusType.BROADCASTING
+
+    assert len(bm.events) == 1
+    alert_event = bm.events[0]
+
+    mock_task.assert_called_once_with(kwargs={'broadcast_event_id': str(alert_event.id)}, queue='notify-internal-tasks')
+
+    assert alert_event.transmitted_content == {"body": "emergency broadcast"}
+
+
 def test_update_broadcast_message_status_rejects_approval_from_creator(
     admin_request,
     sample_broadcast_service,