From b54d49196b4b3b39f8d136d8fa93fc5f96df7410 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 15 Oct 2020 13:15:22 +0100 Subject: [PATCH] Ensure correct selected nav item on broadcast page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once a broadcast has been submitted for approval it either lives on the ‘Current alerts’ or ‘Previous alerts’ page, depending on where it is in its lifecycle. Therefore when clicking into a broadcast from one of those pages the same navigation item should remain selected. Because we select the navigation items based on the request endpoint, this means we need an endpoint for each navigation page, even if the content of the pages will be the same in both cases. This commit adds the two new end points, removes the old, single endpoint and updates links to point to the new endpoint. --- app/main/views/broadcast.py | 28 ++++++++---- app/navigation.py | 20 ++++++--- app/templates/main_nav.html | 4 +- .../broadcast/partials/dashboard-table.html | 2 +- tests/app/main/views/test_broadcast.py | 43 +++++++++++-------- 5 files changed, 61 insertions(+), 36 deletions(-) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 27844ef3e..f20eb3fcd 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -48,6 +48,7 @@ def broadcast_dashboard_previous(service_id): 'views/broadcast/previous-broadcasts.html', broadcasts=BroadcastMessages(service_id).with_status('cancelled', 'completed'), empty_message='You do not have any previous alerts', + single_endpoint='.view_previous_broadcast', ) @@ -65,11 +66,13 @@ def get_broadcast_dashboard_partials(service_id): 'views/broadcast/partials/dashboard-table.html', broadcasts=broadcast_messages.with_status('pending-approval'), empty_message='You do not have any alerts waiting for approval', + single_endpoint='.view_current_broadcast', ), live_broadcasts=render_template( 'views/broadcast/partials/dashboard-table.html', broadcasts=broadcast_messages.with_status('broadcasting'), empty_message='You do not have any live alerts at the moment', + single_endpoint='.view_current_broadcast', ), ) @@ -261,7 +264,7 @@ def preview_broadcast_message(service_id, broadcast_message_id): if request.method == 'POST': broadcast_message.request_approval() return redirect(url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=current_service.id, broadcast_message_id=broadcast_message.id, )) @@ -272,10 +275,17 @@ def preview_broadcast_message(service_id, broadcast_message_id): ) -@main.route('/services//broadcast/') +@main.route( + '/services//current-alerts/', + endpoint='view_current_broadcast', +) +@main.route( + '/services//previous-alerts/', + endpoint='view_previous_broadcast', +) @user_has_permissions() @service_has_permission('broadcast') -def view_broadcast_message(service_id, broadcast_message_id): +def view_broadcast(service_id, broadcast_message_id): broadcast_message = BroadcastMessage.from_id( broadcast_message_id, service_id=current_service.id, @@ -288,7 +298,7 @@ def view_broadcast_message(service_id, broadcast_message_id): ) -@main.route('/services//broadcast/', methods=['POST']) +@main.route('/services//current-alerts/', methods=['POST']) @user_has_permissions('send_messages') @service_has_permission('broadcast') def approve_broadcast_message(service_id, broadcast_message_id): @@ -300,7 +310,7 @@ def approve_broadcast_message(service_id, broadcast_message_id): if broadcast_message.status != 'pending-approval': return redirect(url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=current_service.id, broadcast_message_id=broadcast_message.id, )) @@ -315,7 +325,7 @@ def approve_broadcast_message(service_id, broadcast_message_id): )) return redirect(url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=current_service.id, broadcast_message_id=broadcast_message.id, )) @@ -333,7 +343,7 @@ def reject_broadcast_message(service_id, broadcast_message_id): if broadcast_message.status != 'pending-approval': return redirect(url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=current_service.id, broadcast_message_id=broadcast_message.id, )) @@ -360,7 +370,7 @@ def cancel_broadcast_message(service_id, broadcast_message_id): if broadcast_message.status != 'broadcasting': return redirect(url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=current_service.id, broadcast_message_id=broadcast_message.id, )) @@ -368,7 +378,7 @@ def cancel_broadcast_message(service_id, broadcast_message_id): if request.method == 'POST': broadcast_message.cancel_broadcast() return redirect(url_for( - '.view_broadcast_message', + '.view_previous_broadcast', service_id=current_service.id, broadcast_message_id=broadcast_message.id, )) diff --git a/app/navigation.py b/app/navigation.py index 4cb319cbc..79e534991 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -368,7 +368,8 @@ class HeaderNavigation(Navigation): 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', - 'view_broadcast_message', + 'view_current_broadcast', + 'view_previous_broadcast', 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', @@ -384,8 +385,6 @@ class MainNavigation(Navigation): mapping = { 'dashboard': { 'broadcast_tour', - 'broadcast_dashboard', - 'broadcast_dashboard_updates', 'conversation', 'inbox', 'monthly', @@ -396,8 +395,14 @@ class MainNavigation(Navigation): 'view_notification', 'view_notifications', }, - 'previous_broadcasts': { + 'current-broadcasts': { + 'broadcast_dashboard', + 'broadcast_dashboard_updates', + 'view_current_broadcast', + }, + 'previous-broadcasts': { 'broadcast_dashboard_previous', + 'view_previous_broadcast', }, 'templates': { 'action_blocked', @@ -432,7 +437,6 @@ class MainNavigation(Navigation): 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', - 'view_broadcast_message', 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', @@ -1041,7 +1045,8 @@ class CaseworkNavigation(Navigation): 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', - 'view_broadcast_message', + 'view_current_broadcast', + 'view_previous_broadcast', 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', @@ -1372,7 +1377,8 @@ class OrgNavigation(Navigation): 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', - 'view_broadcast_message', + 'view_current_broadcast', + 'view_previous_broadcast', 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index f800c193b..f4d7c13a0 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -5,8 +5,8 @@
    {% if current_user.has_permissions() %} {% if current_service.has_permission('broadcast') %} -
  • Current alerts
    • -
  • Previous alerts
    • +
  • Current alerts
    • +
  • Previous alerts
    • {% elif current_user.has_permissions('view_activity') %}
  • Dashboard
    • {% endif %} diff --git a/app/templates/views/broadcast/partials/dashboard-table.html b/app/templates/views/broadcast/partials/dashboard-table.html index 200bc9484..a323eec3d 100644 --- a/app/templates/views/broadcast/partials/dashboard-table.html +++ b/app/templates/views/broadcast/partials/dashboard-table.html @@ -14,7 +14,7 @@ ) %} {% call row_heading() %}
    - {{ item.template_name }} + {{ item.template_name }} {{ item.content }} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 774875cdd..c7f41d3af 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -52,9 +52,13 @@ sample_uuid = sample_uuid() 403, 403, ), ( - '.view_broadcast_message', {'broadcast_message_id': sample_uuid}, + '.view_current_broadcast', {'broadcast_message_id': sample_uuid}, 403, 403, ), + ( + '.view_previous_broadcast', {'broadcast_message_id': sample_uuid}, + 403, 405, + ), ( '.cancel_broadcast_message', {'broadcast_message_id': sample_uuid}, 403, 403, @@ -1019,7 +1023,7 @@ def test_start_broadcasting( service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _expected_redirect=url_for( - 'main.view_broadcast_message', + 'main.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _external=True, @@ -1099,7 +1103,7 @@ def test_view_broadcast_message_page( service_one['permissions'] += ['broadcast'] page = client_request.get( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, ) @@ -1136,7 +1140,7 @@ def test_view_pending_broadcast( service_one['permissions'] += ['broadcast'] page = client_request.get( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, ) @@ -1190,7 +1194,7 @@ def test_cant_approve_own_broadcast( service_one['permissions'] += ['broadcast'] page = client_request.get( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, ) @@ -1243,7 +1247,7 @@ def test_can_approve_own_broadcast_in_trial_mode( service_one['permissions'] += ['broadcast'] page = client_request.get( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, ) @@ -1319,7 +1323,7 @@ def test_view_only_user_cant_approve_broadcast( service_one['permissions'] += ['broadcast'] page = client_request.get( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, ) @@ -1338,7 +1342,7 @@ def test_view_only_user_cant_approve_broadcast( @pytest.mark.parametrize('trial_mode, initial_status, expected_approval, expected_redirect', ( (True, 'draft', False, partial( url_for, - '.view_broadcast_message', + '.view_current_broadcast', broadcast_message_id=sample_uuid, )), (True, 'pending-approval', True, partial( @@ -1348,22 +1352,22 @@ def test_view_only_user_cant_approve_broadcast( )), (False, 'pending-approval', True, partial( url_for, - '.view_broadcast_message', + '.view_current_broadcast', broadcast_message_id=sample_uuid, )), (True, 'rejected', False, partial( url_for, - '.view_broadcast_message', + '.view_current_broadcast', broadcast_message_id=sample_uuid, )), (True, 'broadcasting', False, partial( url_for, - '.view_broadcast_message', + '.view_current_broadcast', broadcast_message_id=sample_uuid, )), (True, 'cancelled', False, partial( url_for, - '.view_broadcast_message', + '.view_current_broadcast', broadcast_message_id=sample_uuid, )), )) @@ -1396,7 +1400,7 @@ def test_request_approval( service_one['permissions'] += ['broadcast'] client_request.post( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _expected_redirect=expected_redirect( @@ -1502,7 +1506,7 @@ def test_cant_reject_broadcast_in_wrong_state( service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _expected_redirect=url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _external=True, @@ -1513,15 +1517,20 @@ def test_cant_reject_broadcast_in_wrong_state( assert mock_update_broadcast_message_status.called is False +@pytest.mark.parametrize('endpoint', ( + '.view_current_broadcast', + '.view_previous_broadcast', +)) def test_no_view_page_for_draft( client_request, service_one, mock_get_draft_broadcast_message, fake_uuid, + endpoint, ): service_one['permissions'] += ['broadcast'] client_request.get( - '.view_broadcast_message', + endpoint, service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _expected_status=404, @@ -1574,7 +1583,7 @@ def test_confirm_cancel_broadcast( service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _expected_redirect=url_for( - '.view_broadcast_message', + '.view_previous_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _external=True, @@ -1602,7 +1611,7 @@ def test_cant_cancel_broadcast_in_a_different_state( service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _expected_redirect=url_for( - '.view_broadcast_message', + '.view_current_broadcast', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, _external=True,