Merge pull request #3530 from alphagov/confirm-cancel-broadcast

Add a confirmation step to cancelling a broadcast

app/main/views/broadcast.py

@@ -1,4 +1,12 @@
-from flask import abort, jsonify, redirect, render_template, url_for
+from flask import (
+    abort,
+    flash,
+    jsonify,
+    redirect,
+    render_template,
+    request,
+    url_for,
+)
 
 from app import current_service
 from app.main import main
@@ -228,16 +236,39 @@ def reject_broadcast_message(service_id, broadcast_message_id):
     ))
 
 
-@main.route('/services/<uuid:service_id>/broadcast/<uuid:broadcast_message_id>/cancel')
+@main.route(
+    '/services/<uuid:service_id>/broadcast/<uuid:broadcast_message_id>/cancel',
+    methods=['GET', 'POST'],
+)
 @user_has_permissions('send_messages')
 @service_has_permission('broadcast')
 def cancel_broadcast_message(service_id, broadcast_message_id):
-    BroadcastMessage.from_id(
+    broadcast_message = BroadcastMessage.from_id(
         broadcast_message_id,
         service_id=current_service.id,
-    ).cancel_broadcast()
-    return redirect(url_for(
-        '.view_broadcast_message',
-        service_id=current_service.id,
-        broadcast_message_id=broadcast_message_id,
-    ))
+    )
+
+    if broadcast_message.status != 'broadcasting':
+        return redirect(url_for(
+            '.view_broadcast_message',
+            service_id=current_service.id,
+            broadcast_message_id=broadcast_message.id,
+        ))
+
+    if request.method == 'POST':
+        broadcast_message.cancel_broadcast()
+        return redirect(url_for(
+            '.view_broadcast_message',
+            service_id=current_service.id,
+            broadcast_message_id=broadcast_message.id,
+        ))
+
+    flash([
+        'Are you sure you want to stop this broadcast now?'
+    ], 'stop broadcasting')
+
+    return render_template(
+        'views/broadcast/view-message.html',
+        broadcast_message=broadcast_message,
+        hide_stop_link=True,
+    )

app/templates/flash_messages.html

@@ -2,7 +2,7 @@
 {% with messages = get_flashed_messages(with_categories=true) %}
   {% if messages %}
     {% for category, message in messages %}
-      {% if category in ['cancel', 'delete', 'suspend', 'resume', 'remove', 'revoke this API key'] %}
+      {% if category in ['cancel', 'delete', 'suspend', 'resume', 'remove', 'revoke this API key', 'stop broadcasting'] %}
         {% set delete_button_text = "Yes, {}".format(category) %}
       {% elif category == 'try again' %}
         {% set delete_button_text = category|capitalize %}

app/templates/views/broadcast/view-message.html

@@ -43,7 +43,10 @@
       {% if broadcast_message.status == 'pending-approval' %}
         Will broadcast until {{ broadcast_message.finishes_at|format_datetime_relative }}.
       {% elif broadcast_message.status == 'broadcasting' %}
-        Live until {{ broadcast_message.finishes_at|format_datetime_relative }}&ensp;<a href="{{ url_for('.cancel_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id) }}" class="destructive-link destructive-link--no-visited-state">Stop broadcast early</a>
+        Live until {{ broadcast_message.finishes_at|format_datetime_relative }}&ensp;
+        {%- if not hide_stop_link %}
+          <a href="{{ url_for('.cancel_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id) }}" class="destructive-link destructive-link--no-visited-state">Stop broadcast early</a>
+        {% endif %}
       {% elif broadcast_message.status == 'cancelled' %}
         Stopped by {{ broadcast_message.cancelled_by.name }}
         {{ broadcast_message.cancelled_at|format_datetime_human }}.

tests/app/main/views/test_broadcast.py

@@ -10,25 +10,66 @@ from tests.conftest import SERVICE_ONE_ID, normalize_spaces
 sample_uuid = sample_uuid()
 
 
-@pytest.mark.parametrize('endpoint, extra_args', (
-    ('.broadcast_dashboard', {}),
-    ('.broadcast_dashboard_updates', {}),
-    ('.broadcast', {'template_id': sample_uuid}),
-    ('.preview_broadcast_areas', {'broadcast_message_id': sample_uuid}),
-    ('.choose_broadcast_library', {'broadcast_message_id': sample_uuid}),
-    ('.choose_broadcast_area', {'broadcast_message_id': sample_uuid, 'library_slug': 'countries'}),
-    ('.remove_broadcast_area', {'broadcast_message_id': sample_uuid, 'area_slug': 'england'}),
-    ('.preview_broadcast_message', {'broadcast_message_id': sample_uuid}),
+@pytest.mark.parametrize('endpoint, extra_args, expected_get_status, expected_post_status', (
+    (
+        '.broadcast_dashboard', {},
+        403, 405,
+    ),
+    (
+        '.broadcast_dashboard_updates', {},
+        403, 405,
+    ),
+    (
+        '.broadcast',
+        {'template_id': sample_uuid},
+        403, 405,
+    ),
+    (
+        '.preview_broadcast_areas', {'broadcast_message_id': sample_uuid},
+        403, 405,
+    ),
+    (
+        '.choose_broadcast_library', {'broadcast_message_id': sample_uuid},
+        403, 405,
+    ),
+    (
+        '.choose_broadcast_area', {'broadcast_message_id': sample_uuid, 'library_slug': 'countries'},
+        403, 403,
+    ),
+    (
+        '.remove_broadcast_area', {'broadcast_message_id': sample_uuid, 'area_slug': 'england'},
+        403, 405,
+    ),
+    (
+        '.preview_broadcast_message', {'broadcast_message_id': sample_uuid},
+        403, 403,
+    ),
+    (
+        '.view_broadcast_message', {'broadcast_message_id': sample_uuid},
+        403, 403,
+    ),
+    (
+        '.cancel_broadcast_message', {'broadcast_message_id': sample_uuid},
+        403, 403,
+    ),
 ))
 def test_broadcast_pages_403_without_permission(
     client_request,
     endpoint,
     extra_args,
+    expected_get_status,
+    expected_post_status,
 ):
     client_request.get(
         endpoint,
         service_id=SERVICE_ONE_ID,
-        _expected_status=403,
+        _expected_status=expected_get_status,
+        **extra_args
+    )
+    client_request.post(
+        endpoint,
+        service_id=SERVICE_ONE_ID,
+        _expected_status=expected_post_status,
         **extra_args
     )
 
@@ -704,12 +745,45 @@ def test_no_view_page_for_draft(
 def test_cancel_broadcast(
     client_request,
     service_one,
-    mock_get_draft_broadcast_message,
+    mock_get_live_broadcast_message,
+    mock_get_broadcast_template,
     mock_update_broadcast_message_status,
     fake_uuid,
 ):
     service_one['permissions'] += ['broadcast']
-    client_request.get(
+    page = client_request.get(
+        '.cancel_broadcast_message',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+    assert normalize_spaces(page.select_one('.banner-dangerous').text) == (
+        'Are you sure you want to stop this broadcast now? '
+        'Yes, stop broadcasting'
+    )
+    form = page.select_one('form')
+    assert form['method'] == 'post'
+    assert 'action' not in form
+    assert normalize_spaces(form.select_one('button[type=submit]').text) == (
+        'Yes, stop broadcasting'
+    )
+    assert mock_update_broadcast_message_status.called is False
+    assert url_for(
+        '.cancel_broadcast_message',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    ) not in page
+
+
+def test_confirm_cancel_broadcast(
+    client_request,
+    service_one,
+    mock_get_live_broadcast_message,
+    mock_get_broadcast_template,
+    mock_update_broadcast_message_status,
+    fake_uuid,
+):
+    service_one['permissions'] += ['broadcast']
+    client_request.post(
         '.cancel_broadcast_message',
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
@@ -719,9 +793,33 @@ def test_cancel_broadcast(
             broadcast_message_id=fake_uuid,
             _external=True,
         ),
-    ),
+    )
     mock_update_broadcast_message_status.assert_called_once_with(
         'cancelled',
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
     )
+
+
+@pytest.mark.parametrize('method', ('post', 'get'))
+def test_cant_cancel_broadcast_in_a_different_state(
+    client_request,
+    service_one,
+    mock_get_draft_broadcast_message,
+    mock_update_broadcast_message_status,
+    fake_uuid,
+    method,
+):
+    service_one['permissions'] += ['broadcast']
+    getattr(client_request, method)(
+        '.cancel_broadcast_message',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+        _expected_redirect=url_for(
+            '.view_broadcast_message',
+            service_id=SERVICE_ONE_ID,
+            broadcast_message_id=fake_uuid,
+            _external=True,
+        ),
+    )
+    assert mock_update_broadcast_message_status.called is False