From 5bfe5f86de286d85df8d88e3ea819eaa6752c8e6 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 7 Jun 2021 17:23:48 +0100 Subject: [PATCH] Simplify channel selection using radio buttons This takes a similar approach as in the previous commit. Since the "training channel" doesn't really exist, we need some extra code to pre-select it if a service is already in training mode. As in the previous commit, I've removed a few non-critical test cases where we really don't need to test exhaustively. Note that we also need some specific code to avoid pre-selecting an option for non-broadcast services, which only used to work by fluke: we would try to populate the field with (False, None, 'all'), which isn't a valid combination, so nothing was selected. --- app/main/forms.py | 10 +++--- app/main/views/service_settings.py | 22 +++++++------ tests/app/main/views/test_service_settings.py | 32 ++++++------------- 3 files changed, 26 insertions(+), 38 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 8007001d9..b54f78de4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2392,14 +2392,14 @@ class ServiceBroadcastAccountTypeField(GovukRadiosField): class ServiceBroadcastChannelForm(StripWhitespaceForm): - channel = ServiceBroadcastAccountTypeField( + channel = GovukRadiosField( 'Emergency alerts settings', thing='mode or channel', choices=[ - ("training-test-all", "Training mode"), - ("live-test-all", "Test channel"), - ("live-severe-all", "Live channel"), - ("live-government-all", "Government channel"), + ("training", "Training mode"), + ("test", "Test channel"), + ("severe", "Live channel"), + ("government", "Government channel"), ], ) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 2ea9de991..fd45e8ebc 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -321,25 +321,27 @@ def service_set_permission(service_id, permission): @main.route("/services//service-settings/broadcasts", methods=["GET", "POST"]) @user_is_platform_admin def service_set_broadcast_channel(service_id): - form = ServiceBroadcastChannelForm( - channel=( - current_service.live, - current_service.broadcast_channel, - 'all', - ) - ) + if current_service.has_permission('broadcast'): + if current_service.live: + channel = current_service.broadcast_channel + else: + channel = 'training' + else: + channel = None + + form = ServiceBroadcastChannelForm(channel=channel) if form.validate_on_submit(): - if form.channel.service_mode == 'training': + if form.channel.data == 'training': return redirect(url_for( '.service_confirm_broadcast_account_type', service_id=current_service.id, - account_type=form.channel.data, + account_type='training-test-all' )) return redirect(url_for( '.service_set_broadcast_network', service_id=current_service.id, - broadcast_channel=form.channel.broadcast_channel, + broadcast_channel=form.channel.data, )) return render_template( diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 2633e548c..6da5f1a62 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -5537,28 +5537,14 @@ def test_service_set_broadcast_channel_has_no_radio_selected_for_non_broadcast_s "test", "all", "Training mode", - "training-test-all", - ), - ( - "live", - "test", - "vodafone", - "Test channel", - "live-test-all", - ), - ( - "live", - "severe", - "all", - "Live channel", - "live-severe-all", + "training", ), ( "live", "government", "all", "Government channel", - "live-government-all", + "government", ), ] ) @@ -5598,25 +5584,25 @@ def test_service_set_broadcast_channel_has_radio_selected_for_broadcast_service( @pytest.mark.parametrize( - 'account_type,expected_redirect_endpoint,extra_args', + 'channel,expected_redirect_endpoint,extra_args', [ ( - 'training-test-all', + 'training', '.service_confirm_broadcast_account_type', {'account_type': 'training-test-all'}, ), ( - 'live-test-all', + 'test', '.service_set_broadcast_network', {'broadcast_channel': 'test'}, ), ( - 'live-severe-all', + 'severe', '.service_set_broadcast_network', {'broadcast_channel': 'severe'}, ), ( - 'live-government-all', + 'government', '.service_set_broadcast_network', {'broadcast_channel': 'government'}, ), @@ -5626,7 +5612,7 @@ def test_service_set_broadcast_channel_redirects( client_request, platform_admin_user, mocker, - account_type, + channel, expected_redirect_endpoint, extra_args, ): @@ -5635,7 +5621,7 @@ def test_service_set_broadcast_channel_redirects( 'main.service_set_broadcast_channel', service_id=SERVICE_ONE_ID, _data={ - 'channel': account_type, + 'channel': channel, }, _expected_redirect=url_for( expected_redirect_endpoint,