Only show active providers when changing priority

This fixes a bug where a third inactive provider would (potentially)
appear in place of one of the two active ones, depending on the order
of its identifier compared to the other two.

In future we may look at simplifying this to cope with more than two
active providers. For now, the existing UI will continue to work when
we add a new, inactive SMS provider for Reach.
This commit is contained in:
Ben Thorner
2022-04-05 15:55:04 +01:00
parent f9749c0ae3
commit 9355c4f8d1
2 changed files with 17 additions and 15 deletions
+1
View File
@@ -65,6 +65,7 @@ def edit_sms_provider_ratio():
provider provider
for provider in provider_client.get_all_providers()['provider_details'] for provider in provider_client.get_all_providers()['provider_details']
if provider['notification_type'] == 'sms' if provider['notification_type'] == 'sms'
and provider['active']
], key=itemgetter('identifier'), reverse=True) ], key=itemgetter('identifier'), reverse=True)
form = AdminProviderRatioForm(ratio=providers[0]['priority']) form = AdminProviderRatioForm(ratio=providers[0]['priority'])
+16 -15
View File
@@ -443,16 +443,17 @@ def test_should_show_version_history_for_first_two_sms_providers(
'app.provider_client.get_all_providers', 'app.provider_client.get_all_providers',
return_value=stub_providers return_value=stub_providers
) )
# second_sms_international will be the primary provider because its
# the first in the list when reverse sorting the SMS providers # we get only get version info for the "primary" (first) provider
# in reverse alphabetical order i.e. sms_provider_2
mocker.patch( mocker.patch(
'app.provider_client.get_provider_versions', 'app.provider_client.get_provider_versions',
return_value={'data': [ return_value={'data': [
{ {
'id': id, 'id': id,
'priority': priority, 'priority': priority,
'display_name': sms_provider_int_2['display_name'], 'display_name': sms_provider_2['display_name'],
'identifier': sms_provider_int_2['identifier'], 'identifier': sms_provider_2['identifier'],
'updated_at': updated_at, 'updated_at': updated_at,
'created_by': { 'created_by': {
'email_address': 'test@foo.bar', 'email_address': 'test@foo.bar',
@@ -489,7 +490,7 @@ def test_should_show_version_history_for_first_two_sms_providers(
radio['value'] radio['value']
for radio in page.select('input[checked]') for radio in page.select('input[checked]')
] == [ ] == [
str(sms_provider_int_2['priority']) str(sms_provider_2['priority'])
] ]
assert [ assert [
@@ -507,18 +508,18 @@ def test_should_show_version_history_for_first_two_sms_providers(
] == [ ] == [
( (
'Test User 2:00pm ' 'Test User 2:00pm '
'Second International SMS Provider 100% ' 'Second Domestic SMS Provider 100% '
'Second Domestic SMS Provider 0%' 'First Domestic SMS Provider 0%'
), ),
( (
'Test User 5:00am ' 'Test User 5:00am '
'Second International SMS Provider 80% ' 'Second Domestic SMS Provider 80% '
'Second Domestic SMS Provider 20%' 'First Domestic SMS Provider 20%'
), ),
( (
'Test User 3:00am ' 'Test User 3:00am '
'Second International SMS Provider 10% ' 'Second Domestic SMS Provider 10% '
'Second Domestic SMS Provider 90%' 'First Domestic SMS Provider 90%'
), ),
] ]
@@ -527,15 +528,15 @@ def test_should_show_version_history_for_first_two_sms_providers(
( (
'10', '10',
[ [
call(sms_provider_int_2['id'], 10), call(sms_provider_2['id'], 10),
call(sms_provider_2['id'], 90), call(sms_provider_1['id'], 90),
], ],
), ),
( (
'80', '80',
[ [
call(sms_provider_int_2['id'], 80), call(sms_provider_2['id'], 80),
call(sms_provider_2['id'], 20), call(sms_provider_1['id'], 20),
], ],
), ),
]) ])