Show error message if checkbox wasn’t checked

Because we were redirecting in all cases the error message wasn’t being
shown.

This commit changes the endpoint to respond with content (including an
error message) if the `POST` is not successful.

app/main/forms.py

@@ -1297,7 +1297,9 @@ class ConfirmBroadcastForm(StripWhitespaceForm):
         self.confirm.label.text = self.generate_label(channel, max_phones)
 
         if service_is_live:
-            self.confirm.validators += (DataRequired(),)
+            self.confirm.validators += (
+                DataRequired('You need to confirm that you understand'),
+            )
 
     confirm = GovukCheckboxField("Confirm")
 

app/main/views/broadcast.py

@@ -22,6 +22,15 @@ from app.models.broadcast_message import BroadcastMessage, BroadcastMessages
 from app.utils import service_has_permission, user_has_permissions
 
 
+def get_back_link_endpoint():
+    return {
+        'main.view_current_broadcast': '.broadcast_dashboard',
+        'main.view_previous_broadcast': '.broadcast_dashboard_previous',
+        'main.view_rejected_broadcast': '.broadcast_dashboard_rejected',
+        'main.approve_broadcast_message': '.broadcast_dashboard',
+    }[request.endpoint]
+
+
 @main.route('/services/<uuid:service_id>/broadcast-tour/<int:step_index>')
 @user_has_permissions()
 @service_has_permission('broadcast')
@@ -389,17 +398,11 @@ def view_broadcast(service_id, broadcast_message_id):
                 broadcast_message_id=broadcast_message.id,
             ))
 
-    back_link_endpoint = {
-        'main.view_current_broadcast': '.broadcast_dashboard',
-        'main.view_previous_broadcast': '.broadcast_dashboard_previous',
-        'main.view_rejected_broadcast': '.broadcast_dashboard_rejected',
-    }[request.endpoint]
-
     return render_template(
         'views/broadcast/view-message.html',
         broadcast_message=broadcast_message,
         back_link=url_for(
-            back_link_endpoint,
+            get_back_link_endpoint(),
             service_id=current_service.id,
         ),
         form=ConfirmBroadcastForm(
@@ -442,6 +445,16 @@ def approve_broadcast_message(service_id, broadcast_message_id):
         ))
     elif form.validate_on_submit():
         broadcast_message.approve_broadcast()
+    else:
+        return render_template(
+            'views/broadcast/view-message.html',
+            broadcast_message=broadcast_message,
+            back_link=url_for(
+                get_back_link_endpoint(),
+                service_id=current_service.id,
+            ),
+            form=form,
+        )
 
     return redirect(url_for(
         '.view_current_broadcast',

tests/app/main/views/test_broadcast.py

@@ -1878,10 +1878,6 @@ def test_checkbox_to_confirm_non_training_broadcasts(
     'severe',
     'government',
 ))
-@pytest.mark.parametrize('post_data', (
-    pytest.param({}, marks=pytest.mark.xfail),
-    {'confirm': 'y'},
-))
 @freeze_time('2020-02-22T22:22:22.000000')
 def test_confirm_approve_non_training_broadcasts(
     mocker,
@@ -1892,7 +1888,6 @@ def test_confirm_approve_non_training_broadcasts(
     mock_update_broadcast_message,
     mock_update_broadcast_message_status,
     channel,
-    post_data,
 ):
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
@@ -1913,7 +1908,7 @@ def test_confirm_approve_non_training_broadcasts(
         '.view_current_broadcast',
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
-        _data=post_data
+        _data={'confirm': 'y'}
     )
 
     mock_update_broadcast_message.assert_called_once_with(
@@ -1931,6 +1926,58 @@ def test_confirm_approve_non_training_broadcasts(
     )
 
 
+@pytest.mark.parametrize('channel', (
+    'test',
+    'severe',
+    'government',
+))
+@freeze_time('2020-02-22T22:22:22.000000')
+def test_confirm_approve_non_training_broadcasts_errors_if_not_ticked(
+    mocker,
+    client_request,
+    service_one,
+    active_user_with_permissions,
+    fake_uuid,
+    mock_update_broadcast_message,
+    mock_update_broadcast_message_status,
+    channel,
+):
+    mocker.patch(
+        'app.broadcast_message_api_client.get_broadcast_message',
+        return_value=broadcast_message_json(
+            id_=fake_uuid,
+            service_id=SERVICE_ONE_ID,
+            template_id=None,
+            created_by_id=None,
+            status='pending-approval',
+        ),
+    )
+    service_one['permissions'] += ['broadcast']
+    service_one['restricted'] = False
+    service_one['allowed_broadcast_provider'] = 'all'
+    service_one['broadcast_channel'] = channel
+
+    page = client_request.post(
+        '.view_current_broadcast',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+        _data={},
+        _expected_status=200,
+    )
+
+    error_message = page.select_one('form.banner .govuk-error-message')
+    assert error_message['id'] == 'confirm-error'
+    assert normalize_spaces(error_message.text) == (
+        'Error: You need to confirm that you understand'
+    )
+    assert page.select_one('form.banner input#confirm')['aria-describedby'] == (
+        error_message['id']
+    )
+
+    assert mock_update_broadcast_message.called is False
+    assert mock_update_broadcast_message_status.called is False
+
+
 @freeze_time('2020-02-22T22:22:22.000000')
 def test_cant_approve_own_broadcast(
     mocker,
@@ -2111,39 +2158,44 @@ def test_view_only_user_cant_approve_broadcast(
     assert not page.select_one('.banner a')
 
 
-@pytest.mark.parametrize('trial_mode, initial_status, post_data, expected_approval, expected_redirect', (
-    (True, 'draft', {}, False, partial(
-        url_for,
-        '.view_current_broadcast',
-        broadcast_message_id=sample_uuid,
-    )),
-    (True, 'pending-approval', {}, True, partial(
-        url_for,
-        '.broadcast_tour',
-        step_index=6,
-    )),
-    (False, 'pending-approval', {}, False, lambda service_id, _external: None),
-    (False, 'pending-approval', {'confirm': 'y'}, True, partial(
-        url_for,
-        '.view_current_broadcast',
-        broadcast_message_id=sample_uuid,
-    )),
-    (True, 'rejected', {}, False, partial(
-        url_for,
-        '.view_current_broadcast',
-        broadcast_message_id=sample_uuid,
-    )),
-    (True, 'broadcasting', {}, False, partial(
-        url_for,
-        '.view_current_broadcast',
-        broadcast_message_id=sample_uuid,
-    )),
-    (True, 'cancelled', {}, False, partial(
-        url_for,
-        '.view_current_broadcast',
-        broadcast_message_id=sample_uuid,
-    )),
-))
+@pytest.mark.parametrize(
+    'trial_mode, initial_status, post_data, expected_approval, expected_redirect, expected_status',
+    (
+        (True, 'draft', {}, False, partial(
+            url_for,
+            '.view_current_broadcast',
+            broadcast_message_id=sample_uuid,
+        ), 302),
+        (True, 'pending-approval', {}, True, partial(
+            url_for,
+            '.broadcast_tour',
+            step_index=6,
+        ), 302),
+        (False, 'pending-approval', {}, False, (
+            lambda service_id, _external: None
+        ), 200),
+        (False, 'pending-approval', {'confirm': 'y'}, True, partial(
+            url_for,
+            '.view_current_broadcast',
+            broadcast_message_id=sample_uuid,
+        ), 302),
+        (True, 'rejected', {}, False, partial(
+            url_for,
+            '.view_current_broadcast',
+            broadcast_message_id=sample_uuid,
+        ), 302),
+        (True, 'broadcasting', {}, False, partial(
+            url_for,
+            '.view_current_broadcast',
+            broadcast_message_id=sample_uuid,
+        ), 302),
+        (True, 'cancelled', {}, False, partial(
+            url_for,
+            '.view_current_broadcast',
+            broadcast_message_id=sample_uuid,
+        ), 302),
+    )
+)
 @freeze_time('2020-02-22T22:22:22.000000')
 def test_request_approval(
     mocker,
@@ -2158,6 +2210,7 @@ def test_request_approval(
     expected_approval,
     trial_mode,
     expected_redirect,
+    expected_status,
 ):
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
@@ -2181,6 +2234,7 @@ def test_request_approval(
             service_id=SERVICE_ONE_ID,
             _external=True,
         ),
+        _expected_status=expected_status,
         _data=post_data,
     )