Allow setting provider for any channel

Previously we could only select a provider when using the test
channel, but this is also required for others channels when we
do tests on the production network with individual MNOs.

In order to reduce duplication and improve consistency, I've reused
the new broadcast_service_name_tag macro to show the setting.
This commit is contained in:
Ben Thorner
2021-05-20 14:46:04 +01:00
parent 5de1c4f6ca
commit c5196fbf07
7 changed files with 163 additions and 96 deletions
@@ -40,7 +40,7 @@
box-shadow: 0 -3px 0 0 $grey-3; box-shadow: 0 -3px 0 0 $grey-3;
} }
&--live { &--live, &--test {
// This uses new Design System colours to match .govuk-tag--red // This uses new Design System colours to match .govuk-tag--red
background: #F6D7D2; background: #F6D7D2;
color: #942514; color: #942514;
+25 -18
View File
@@ -2407,24 +2407,29 @@ class ServiceBroadcastChannelForm(StripWhitespaceForm):
class ServiceBroadcastNetworkForm(StripWhitespaceForm): class ServiceBroadcastNetworkForm(StripWhitespaceForm):
def __init__(self, broadcast_channel, *args, **kwargs):
super().__init__(*args, **kwargs)
self.broadcast_channel = broadcast_channel
self.network_variant.choices = [
(f'live-{broadcast_channel}', 'All networks'),
('', 'A single network'),
]
self.network.choices = [
(f'live-{broadcast_channel}-ee', 'EE'),
(f'live-{broadcast_channel}-o2', 'O2'),
(f'live-{broadcast_channel}-vodafone', 'Vodafone'),
(f'live-{broadcast_channel}-three', 'Three'),
]
network_variant = ServiceBroadcastAccountTypeField( network_variant = ServiceBroadcastAccountTypeField(
'Choose a mobile network', 'Choose a mobile network',
thing='a mobile network', thing='a mobile network',
choices=[
('live-test', 'All networks'),
('', 'A single network'),
]
) )
network = OptionalServiceBroadcastAccountTypeField( network = OptionalServiceBroadcastAccountTypeField(
'Choose a mobile network', 'Choose a mobile network',
thing='a mobile network', thing='a mobile network',
choices=[
('live-test-ee', 'EE'),
('live-test-o2', 'O2'),
('live-test-vodafone', 'Vodafone'),
('live-test-three', 'Three'),
],
) )
def validate_network(self, field): def validate_network(self, field):
@@ -2439,14 +2444,16 @@ class ServiceBroadcastAccountTypeForm(StripWhitespaceForm):
'Change cell broadcast service type', 'Change cell broadcast service type',
thing='which type of account this cell broadcast service is', thing='which type of account this cell broadcast service is',
choices=[ choices=[
("training-test", "Training mode"), ("training-test", "")
("live-test-ee", "Test channel (EE)"), ] +
("live-test-o2", "Test channel (O2)"), [
("live-test-three", "Test channel (Three)"), (f"live-{broadcast_channel}", "")
("live-test-vodafone", "Test channel (Vodafone)"), for broadcast_channel in ["test", "severe", "government"]
("live-test", "Test channel (all networks)"), ] +
("live-severe", "Live (all networks)"), [
("live-government", "Government channel (all networks)"), (f"live-{broadcast_channel}-{provider}", "")
for broadcast_channel in ["test", "severe", "government"]
for provider in ["ee", "o2", "three", "vodafone"]
], ],
validators=[DataRequired()] validators=[DataRequired()]
) )
+30 -21
View File
@@ -330,15 +330,16 @@ def service_set_broadcast_channel(service_id):
) )
if form.validate_on_submit(): if form.validate_on_submit():
if form.channel.data == 'live-test': if form.channel.service_mode == 'training':
return redirect(url_for( return redirect(url_for(
'.service_set_broadcast_network', '.service_confirm_broadcast_account_type',
service_id=current_service.id, service_id=current_service.id,
account_type=form.channel.data,
)) ))
return redirect(url_for( return redirect(url_for(
'.service_confirm_broadcast_account_type', '.service_set_broadcast_network',
service_id=current_service.id, service_id=current_service.id,
account_type=form.channel.data, broadcast_channel=form.channel.broadcast_channel,
)) ))
return render_template( return render_template(
@@ -347,25 +348,33 @@ def service_set_broadcast_channel(service_id):
) )
@main.route("/services/<uuid:service_id>/service-settings/broadcasts/network", methods=["GET", "POST"]) @main.route("/services/<uuid:service_id>/service-settings/broadcasts/<broadcast_channel>", methods=["GET", "POST"])
@user_is_platform_admin @user_is_platform_admin
def service_set_broadcast_network(service_id): def service_set_broadcast_network(service_id, broadcast_channel):
if current_service.allowed_broadcast_provider == 'all': # only populate old settings when the channel is unchanged
form = ServiceBroadcastNetworkForm( if current_service.broadcast_channel == broadcast_channel:
network_variant=( if current_service.allowed_broadcast_provider == 'all':
current_service.live, form = ServiceBroadcastNetworkForm(
current_service.broadcast_channel, broadcast_channel=broadcast_channel,
current_service.allowed_broadcast_provider, network_variant=(
), current_service.live,
) current_service.broadcast_channel,
current_service.allowed_broadcast_provider,
),
)
else:
form = ServiceBroadcastNetworkForm(
broadcast_channel=broadcast_channel,
network_variant='',
network=(
current_service.live,
current_service.broadcast_channel,
current_service.allowed_broadcast_provider
)
)
else: else:
form = ServiceBroadcastNetworkForm( form = ServiceBroadcastNetworkForm(
network_variant='', broadcast_channel=broadcast_channel
network=(
current_service.live,
current_service.broadcast_channel,
current_service.allowed_broadcast_provider
)
) )
if form.validate_on_submit(): if form.validate_on_submit():
@@ -382,7 +391,7 @@ def service_set_broadcast_network(service_id):
@main.route( @main.route(
"/services/<uuid:service_id>/service-settings/broadcasts/<account_type>", "/services/<uuid:service_id>/service-settings/broadcasts/<account_type>/confirm",
methods=["GET", "POST"] methods=["GET", "POST"]
) )
@user_is_platform_admin @user_is_platform_admin
+5 -5
View File
@@ -17,17 +17,17 @@
{% set margin_class = "" if left_margin else "govuk-!-margin-left-0" %} {% set margin_class = "" if left_margin else "govuk-!-margin-left-0" %}
{% if trial_mode %} {% if trial_mode %}
<span class="navigation-service-type navigation-service-type--training {{margin_class}}">Training <span class="navigation-service-type navigation-service-type--training {{margin_class}}">Training
{% elif broadcast_channel == 'severe' %} {% else %}
{% if broadcast_channel == "severe" %}
<span class="navigation-service-type navigation-service-type--live {{margin_class}}">Live <span class="navigation-service-type navigation-service-type--live {{margin_class}}">Live
{% elif broadcast_channel == 'test' %} {% else %}
<span class="navigation-service-type navigation-service-type--live {{margin_class}}">{{ broadcast_channel|title }} <span class="navigation-service-type navigation-service-type--{{broadcast_channel}} {{margin_class}}">{{ broadcast_channel|title }}
{% endif %}
{% if allowed_broadcast_provider == "all" %} {% if allowed_broadcast_provider == "all" %}
(all networks) (all networks)
{% else %} {% else %}
({{ allowed_broadcast_provider|format_mobile_network }}) ({{ allowed_broadcast_provider|format_mobile_network }})
{% endif %} {% endif %}
{% elif broadcast_channel == 'government' %}
<span class="navigation-service-type navigation-service-type--government {{margin_class}}">Government
{% endif %} {% endif %}
</span> </span>
{% endmacro %} {% endmacro %}
+2 -4
View File
@@ -1,6 +1,7 @@
{% extends "withnav_template.html" %} {% extends "withnav_template.html" %}
{% from "components/banner.html" import banner_wrapper %} {% from "components/banner.html" import banner_wrapper %}
{% from "components/table.html" import mapping_table, row, settings_row, text_field, optional_text_field, edit_field, field, boolean_field with context %} {% from "components/table.html" import mapping_table, row, settings_row, text_field, optional_text_field, edit_field, field, boolean_field with context %}
{% from "service_navigation.html" import broadcast_service_name_tag %}
{% block service_page_title %} {% block service_page_title %}
Settings Settings
@@ -434,10 +435,7 @@
{% if not current_service.broadcast_channel %} {% if not current_service.broadcast_channel %}
Off Off
{% else %} {% else %}
{% if current_service.live and current_service.broadcast_channel == "government" %}Government{% endif %} {{ broadcast_service_name_tag(current_service.trial_mode, current_service.broadcast_channel, current_service.allowed_broadcast_provider, left_margin=False) }}
{% 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 != "all" %}({{ current_service.allowed_broadcast_provider|format_mobile_network }}){% else %}(all networks){% endif %}{%endif%}
{% if not current_service.live%}Training {% endif%}
{% endif %} {% endif %}
{% endcall %} {% endcall %}
{{ edit_field( {{ edit_field(
+23 -7
View File
@@ -318,14 +318,14 @@ def test_some_broadcast_tour_pages_show_service_name(
'severe', 'severe',
'all', 'all',
'.navigation-service-type.navigation-service-type--live', '.navigation-service-type.navigation-service-type--live',
'service one Live Switch service', 'service one Live (all networks) Switch service',
'Live', 'Live (all networks)',
), ),
( (
False, False,
'test', 'test',
'all', 'all',
'.navigation-service-type.navigation-service-type--live', '.navigation-service-type.navigation-service-type--test',
'service one Test (all networks) Switch service', 'service one Test (all networks) Switch service',
'Test (all networks)', 'Test (all networks)',
), ),
@@ -333,7 +333,7 @@ def test_some_broadcast_tour_pages_show_service_name(
False, False,
'test', 'test',
'vodafone', 'vodafone',
'.navigation-service-type.navigation-service-type--live', '.navigation-service-type.navigation-service-type--test',
'service one Test (Vodafone) Switch service', 'service one Test (Vodafone) Switch service',
'Test (Vodafone)', 'Test (Vodafone)',
), ),
@@ -342,12 +342,28 @@ def test_some_broadcast_tour_pages_show_service_name(
'government', 'government',
'all', 'all',
'.navigation-service-type.navigation-service-type--government', '.navigation-service-type.navigation-service-type--government',
'service one Government Switch service', 'service one Government (all networks) Switch service',
'Government', 'Government (all networks)',
),
(
False,
'government',
'vodafone',
'.navigation-service-type.navigation-service-type--government',
'service one Government (Vodafone) Switch service',
'Government (Vodafone)',
),
(
False,
'severe',
'vodafone',
'.navigation-service-type.navigation-service-type--live',
'service one Live (Vodafone) Switch service',
'Live (Vodafone)',
), ),
) )
) )
def test_broadcast_service_shows_live_or_training( def test_broadcast_service_shows_channel_settings(
client_request, client_request,
service_one, service_one,
mock_get_no_broadcast_messages, mock_get_no_broadcast_messages,
+77 -40
View File
@@ -163,9 +163,9 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service(
SERVICE_ONE_ID, SERVICE_ONE_ID,
users=[api_user_active['id']], users=[api_user_active['id']],
permissions=['broadcast'], permissions=['broadcast'],
restricted=True,
organisation_id=ORGANISATION_ID, organisation_id=ORGANISATION_ID,
contact_link='contact_us@gov.uk', contact_link='contact_us@gov.uk',
broadcast_channel="severe",
) )
mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
@@ -186,7 +186,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service(
'Label Value Action', 'Label Value Action',
'Notes None Change the notes for the service', 'Notes None Change the notes for the service',
'Email authentication Off Change your settings for Email authentication', 'Email authentication Off Change your settings for Email authentication',
'Emergency alerts Training Change your settings for emergency alerts', 'Emergency alerts Off Change your settings for emergency alerts',
] ]
assert len(rows) == len(expected_rows) assert len(rows) == len(expected_rows)
@@ -204,7 +204,10 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service(
(True, "live", "test", "ee", "Test (EE)"), (True, "live", "test", "ee", "Test (EE)"),
(True, "live", "test", "three", "Test (Three)"), (True, "live", "test", "three", "Test (Three)"),
(True, "live", "test", "all", "Test (all networks)"), (True, "live", "test", "all", "Test (all networks)"),
(True, "live", "severe", "all", "Live"), (True, "live", "severe", "all", "Live (all networks)"),
(True, "live", "severe", "three", "Live (Three)"),
(True, "live", "government", "all", "Government (all networks)"),
(True, "live", "government", "three", "Government (Three)"),
] ]
) )
def test_platform_admin_sees_correct_description_of_broadcast_service_setting( def test_platform_admin_sees_correct_description_of_broadcast_service_setting(
@@ -239,8 +242,8 @@ def test_platform_admin_sees_correct_description_of_broadcast_service_setting(
assert response.status_code == 200 assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
broadcast_setting_row = page.find(string=re.compile("Emergency alerts")).find_parent('tr') broadcast_setting_row = page.find(string=re.compile("Emergency alerts")).find_parent('tr')
broadcast_setting_description = broadcast_setting_row.select('td')[1].text.strip() broadcast_setting_description = broadcast_setting_row.select('td')[1].text
assert broadcast_setting_description == expected_text assert normalize_spaces(broadcast_setting_description) == expected_text
def test_no_go_live_link_for_service_without_organisation( def test_no_go_live_link_for_service_without_organisation(
@@ -5536,17 +5539,17 @@ def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast
( (
'live-test', 'live-test',
'.service_set_broadcast_network', '.service_set_broadcast_network',
{}, {'broadcast_channel': 'test'},
), ),
( (
'live-severe', 'live-severe',
'.service_confirm_broadcast_account_type', '.service_set_broadcast_network',
{'account_type': 'live-severe'}, {'broadcast_channel': 'severe'},
), ),
( (
'live-government', 'live-government',
'.service_confirm_broadcast_account_type', '.service_set_broadcast_network',
{'account_type': 'live-government'}, {'broadcast_channel': 'government'},
), ),
] ]
) )
@@ -5587,13 +5590,17 @@ def test_get_service_set_broadcast_channel_redirects(
"live", "live",
"severe", "severe",
"all", "all",
[], [
("All networks", "live-severe"),
],
), ),
( (
"live", "live",
"government", "government",
"all", "all",
[], [
("All networks", "live-government"),
],
), ),
( (
"live", "live",
@@ -5639,6 +5646,24 @@ def test_get_service_set_broadcast_channel_redirects(
("Vodafone", "live-test-vodafone"), ("Vodafone", "live-test-vodafone"),
], ],
), ),
(
"live",
"severe",
"vodafone",
[
("A single network", ""),
("Vodafone", "live-severe-vodafone"),
],
),
(
"live",
"government",
"vodafone",
[
("A single network", ""),
("Vodafone", "live-government-vodafone"),
],
),
] ]
) )
def test_get_service_set_broadcast_network_has_radio_selected( def test_get_service_set_broadcast_network_has_radio_selected(
@@ -5663,6 +5688,7 @@ def test_get_service_set_broadcast_network_has_radio_selected(
page = client_request.get( page = client_request.get(
'main.service_set_broadcast_network', 'main.service_set_broadcast_network',
service_id=SERVICE_ONE_ID, service_id=SERVICE_ONE_ID,
broadcast_channel=broadcast_channel,
) )
assert [ assert [
@@ -5675,40 +5701,38 @@ def test_get_service_set_broadcast_network_has_radio_selected(
@pytest.mark.parametrize( @pytest.mark.parametrize(
'data, expected_result', 'broadcast_channel, provider, choice_type',
( (
( ('severe', '', 'network_variant'),
{'network_variant': 'live-test'}, ('government', '', 'network_variant'),
'live-test' ('test', '', 'network_variant'),
), ('test', 'o2', 'network'),
( ('test', 'ee', 'network'),
{'network_variant': '', 'network': 'live-test-ee'}, ('test', 'three', 'network'),
'live-test-ee' ('test', 'vodafone', 'network'),
), ('government', 'vodafone', 'network'),
( ('severe', 'vodafone', 'network'),
{'network_variant': '', 'network': 'live-test-o2'},
'live-test-o2'
),
(
{'network_variant': '', 'network': 'live-test-three'},
'live-test-three'
),
(
{'network_variant': '', 'network': 'live-test-vodafone'},
'live-test-vodafone'
),
), ),
) )
def test_post_service_set_broadcast_network( def test_post_service_set_broadcast_network(
client_request, client_request,
platform_admin_user, platform_admin_user,
data, broadcast_channel,
expected_result, provider,
choice_type,
): ):
if choice_type == 'network_variant':
expected_result = f'live-{broadcast_channel}'
data = {'network_variant': expected_result}
else:
expected_result = f'live-{broadcast_channel}-{provider}'
data = {'network_variant': '', 'network': expected_result}
client_request.login(platform_admin_user) client_request.login(platform_admin_user)
client_request.post( client_request.post(
'main.service_set_broadcast_network', 'main.service_set_broadcast_network',
service_id=SERVICE_ONE_ID, service_id=SERVICE_ONE_ID,
broadcast_channel=broadcast_channel,
_data=data, _data=data,
_expected_status=302, _expected_status=302,
_expected_redirect=url_for( _expected_redirect=url_for(
@@ -5737,6 +5761,7 @@ def test_post_service_set_broadcast_network_makes_you_choose(
page = client_request.post( page = client_request.post(
'main.service_set_broadcast_network', 'main.service_set_broadcast_network',
service_id=SERVICE_ONE_ID, service_id=SERVICE_ONE_ID,
broadcast_channel='all',
_data=data, _data=data,
_expected_status=200, _expected_status=200,
) )
@@ -5783,12 +5808,22 @@ def test_post_service_set_broadcast_network_makes_you_choose(
'this service.', 'this service.',
]), ]),
('live-severe', [ ('live-severe', [
'Live', 'Live (all networks)',
'Members of the public will receive alerts sent from this '
'service.',
]),
('live-severe-vodafone', [
'Live (Vodafone)',
'Members of the public will receive alerts sent from this ' 'Members of the public will receive alerts sent from this '
'service.', 'service.',
]), ]),
('live-government', [ ('live-government', [
'Government', 'Government (all networks)',
'Members of the public will receive alerts sent from this '
'service, even if theyve opted out.'
]),
('live-government-vodafone', [
'Government (Vodafone)',
'Members of the public will receive alerts sent from this ' 'Members of the public will receive alerts sent from this '
'service, even if theyve opted out.' 'service, even if theyve opted out.'
]), ]),
@@ -5856,14 +5891,15 @@ def test_post_service_set_broadcast_account_type_posts_data_to_api_and_redirects
) )
@pytest.mark.parametrize('endpoint, expected_error', ( @pytest.mark.parametrize('endpoint, extra_args, expected_error', (
('main.service_set_broadcast_channel', 'Error: Select mode or channel'), ('main.service_set_broadcast_channel', {}, 'Error: Select mode or channel'),
('main.service_set_broadcast_network', 'Error: Select a mobile network'), ('main.service_set_broadcast_network', {'broadcast_channel': 'all'}, 'Error: Select a mobile network'),
)) ))
def test_post_service_set_broadcast_account_type_shows_errors_if_no_radio_selected( def test_post_service_set_broadcast_account_type_shows_errors_if_no_radio_selected(
platform_admin_client, platform_admin_client,
mocker, mocker,
endpoint, endpoint,
extra_args,
expected_error, expected_error,
): ):
set_service_broadcast_settings_mock = mocker.patch('app.service_api_client.set_service_broadcast_settings') set_service_broadcast_settings_mock = mocker.patch('app.service_api_client.set_service_broadcast_settings')
@@ -5873,6 +5909,7 @@ def test_post_service_set_broadcast_account_type_shows_errors_if_no_radio_select
url_for( url_for(
endpoint, endpoint,
service_id=SERVICE_ONE_ID, service_id=SERVICE_ONE_ID,
**extra_args
) )
) )
assert response.status_code == 200 assert response.status_code == 200