From 5495de0b3b67f1289fa94a735af04596f2713b23 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 4 May 2021 15:58:57 +0100 Subject: [PATCH 1/2] Remove some of the option we have for broadcast services. This is the first step in making the UI easier for setting the options for a broadcast service. Here we remove the options for "Training mode" test channels. When we create a broadcast message for a trail mode service it is marked as stubbed and does not create a broadcast event that is sent to a provider. The label for the form and setting page have been updated to reflect the change. --- app/main/forms.py | 18 ++--- app/templates/views/service-settings.html | 4 +- tests/app/main/views/test_service_settings.py | 65 +++++++------------ 3 files changed, 32 insertions(+), 55 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 4a4b2b683..dfd7d839a 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2342,17 +2342,13 @@ class ServiceBroadcastAccountTypeForm(StripWhitespaceForm): 'Change cell broadcast service type', thing='which type of account this cell broadcast service is', choices=[ - ("training-test-ee", "Training mode - EE network - Test channel only"), - ("training-test-o2", "Training mode - O2 network - Test channel only"), - ("training-test-three", "Training mode - Three network - Test channel only"), - ("training-test-vodafone", "Training mode - Vodafone network - Test channel only"), - ("training-severe", "Training mode - All networks - Public channel"), - ("live-test-ee", "Live - EE network - Test channel only"), - ("live-test-o2", "Live - O2 network - Test channel only"), - ("live-test-three", "Live - Three network - Test channel only"), - ("live-test-vodafone", "Live - Vodafone network - Test channel only"), - ("live-test", "Live - All networks - Test channel only"), - ("live-severe", "Live - All networks - Public channel"), + ("training-test", "Training mode"), + ("live-test-ee", "Test channel (EE)"), + ("live-test-o2", "Test channel (O2)"), + ("live-test-three", "Test channel (Three)"), + ("live-test-vodafone", "Test channel (Vodafone)"), + ("live-test", "Test channel (all networks)"), + ("live-severe", "Live (all networks)"), ], validators=[DataRequired()] ) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index b41491dde..99f5078be 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -434,7 +434,9 @@ {% if not current_service.broadcast_channel %} Off {% else %} - {% if current_service.live %}Live{% else %}Training mode{% endif %} - {% if current_service.allowed_broadcast_provider %}{{ current_service.allowed_broadcast_provider|format_mobile_network }} network{% else %}All networks{% endif %} - {% if current_service.broadcast_channel == "test" %}Test channel only{% else %}Public channel{% endif %} + {% if current_service.live and current_service.broadcast_channel == "severe" %}Live - Public channel{% endif %} + {% if current_service.live and current_service.broadcast_channel == "test" %}Test channel {% if current_service.allowed_broadcast_provider %}({{ current_service.allowed_broadcast_provider|format_mobile_network }} network){% else %} (All networks){% endif %}{%endif%} + {% if not current_service.live%}Training mode {% endif%} {% endif %} {% endcall %} {{ edit_field( diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 49bd9f282..8fd0d79ce 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -186,8 +186,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( 'Label Value Action', 'Notes None Change the notes for the service', 'Email authentication Off Change your settings for Email authentication', - 'Send cell broadcasts Training mode - All networks - Public channel ' - + 'Change your settings for Send cell broadcasts', + 'Send cell broadcasts Training mode Change your settings for Send cell broadcasts', ] assert len(rows) == len(expected_rows) @@ -201,11 +200,11 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( [ (False, "training", None, None, "Off"), (False, "live", None, None, "Off"), - (True, "training", "severe", None, "Training mode - All networks - Public channel"), - (True, "training", "test", "ee", "Training mode - EE network - Test channel only"), - (True, "live", "test", "three", "Live - Three network - Test channel only"), - (True, "live", "test", None, "Live - All networks - Test channel only"), - (True, "live", "severe", None, "Live - All networks - Public channel"), + (True, "training", "test", None, "Training mode"), + (True, "live", "test", "ee", "Test channel (EE network)"), + (True, "live", "test", "three", "Test channel (Three network)"), + (True, "live", "test", None, "Test channel (All networks)"), + (True, "live", "severe", None, "Live - Public channel"), ] ) def test_platform_admin_sees_correct_description_of_broadcast_service_setting( @@ -5429,17 +5428,13 @@ def test_get_service_set_broadcast_account_type( assert page.select_one('h1').text.strip() == "Change cell broadcast service type" expected_labels = [ - "Training mode - EE network - Test channel only", - "Training mode - O2 network - Test channel only", - "Training mode - Three network - Test channel only", - "Training mode - Vodafone network - Test channel only", - "Training mode - All networks - Public channel", - "Live - EE network - Test channel only", - "Live - O2 network - Test channel only", - "Live - Three network - Test channel only", - "Live - Vodafone network - Test channel only", - "Live - All networks - Test channel only", - "Live - All networks - Public channel", + "Training mode", + "Test channel (EE)", + "Test channel (O2)", + "Test channel (Three)", + "Test channel (Vodafone)", + "Test channel (all networks)", + "Live (all networks)", ] labels = page.find_all('label', class_="govuk-radios__label") assert len(labels) == len(expected_labels) @@ -5470,37 +5465,23 @@ def test_get_service_set_broadcast_account_type_has_no_radio_selected_for_non_br [ ( "training", - "severe", + "test", None, - "Training mode - All networks - Public channel", - "training-severe", + "Training mode", + "training-test", ), ( - "training", + "live", "test", "vodafone", - "Training mode - Vodafone network - Test channel only", - "training-test-vodafone", - ), - ( - "live", - "test", - "o2", - "Live - O2 network - Test channel only", - "live-test-o2", - ), - ( - "live", - "test", - None, - "Live - All networks - Test channel only", - "live-test", + "Test channel (Vodafone)", + "live-test-vodafone", ), ( "live", "severe", None, - "Live - All networks - Public channel", + "Live (all networks)", "live-severe", ), ] @@ -5543,10 +5524,8 @@ def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast @pytest.mark.parametrize( 'value,service_mode,broadcast_channel,allowed_broadcast_provider', [ - ("training-severe", "training", "severe", None), - ("training-test-vodafone", "training", "test", "vodafone"), - ("live-test-o2", "live", "test", "o2"), - ("live-test", "live", "test", None), + ("training-test", "training", "test", None), + ("live-test-vodafone", "live", "test", "vodafone"), ("live-severe", "live", "severe", None), ] ) From cf160c3ae1d824e57e4e282071ca5636ac217b7b Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 6 May 2021 07:29:21 +0100 Subject: [PATCH 2/2] Update labels Use more suscinct labels for the service settings page --- app/templates/views/service-settings.html | 6 +++--- tests/app/main/views/test_service_settings.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 99f5078be..1c7f9443b 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -434,9 +434,9 @@ {% if not current_service.broadcast_channel %} Off {% else %} - {% if current_service.live and current_service.broadcast_channel == "severe" %}Live - Public channel{% endif %} - {% if current_service.live and current_service.broadcast_channel == "test" %}Test channel {% if current_service.allowed_broadcast_provider %}({{ current_service.allowed_broadcast_provider|format_mobile_network }} network){% else %} (All networks){% endif %}{%endif%} - {% if not current_service.live%}Training mode {% endif%} + {% if current_service.live and current_service.broadcast_channel == "severe" %}Live{% endif %} + {% if current_service.live and current_service.broadcast_channel == "test" %}Test {% if current_service.allowed_broadcast_provider %}({{ current_service.allowed_broadcast_provider|format_mobile_network }}){% else %}(All networks){% endif %}{%endif%} + {% if not current_service.live%}Training {% endif%} {% endif %} {% endcall %} {{ edit_field( diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 8fd0d79ce..6b53a8cc2 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -186,7 +186,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( 'Label Value Action', 'Notes None Change the notes for the service', 'Email authentication Off Change your settings for Email authentication', - 'Send cell broadcasts Training mode Change your settings for Send cell broadcasts', + 'Send cell broadcasts Training Change your settings for Send cell broadcasts', ] assert len(rows) == len(expected_rows) @@ -200,11 +200,11 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( [ (False, "training", None, None, "Off"), (False, "live", None, None, "Off"), - (True, "training", "test", None, "Training mode"), - (True, "live", "test", "ee", "Test channel (EE network)"), - (True, "live", "test", "three", "Test channel (Three network)"), - (True, "live", "test", None, "Test channel (All networks)"), - (True, "live", "severe", None, "Live - Public channel"), + (True, "training", "test", None, "Training"), + (True, "live", "test", "ee", "Test (EE)"), + (True, "live", "test", "three", "Test (Three)"), + (True, "live", "test", None, "Test (All networks)"), + (True, "live", "severe", None, "Live"), ] ) def test_platform_admin_sees_correct_description_of_broadcast_service_setting(