From 9355c4f8d1195285248e8289a08586ee10a1f378 Mon Sep 17 00:00:00 2001
From: Ben Thorner <ben.thorner@digital.cabinet-office.gov.uk>
Date: Fri, 25 Mar 2022 16:30:54 +0000
Subject: [PATCH] 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.
---
 app/main/views/providers.py            |  1 +
 tests/app/main/views/test_providers.py | 31 +++++++++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/app/main/views/providers.py b/app/main/views/providers.py
index 0868f1a63..fcaf6aa86 100644
--- a/app/main/views/providers.py
+++ b/app/main/views/providers.py
@@ -65,6 +65,7 @@ def edit_sms_provider_ratio():
         provider
         for provider in provider_client.get_all_providers()['provider_details']
         if provider['notification_type'] == 'sms'
+        and provider['active']
     ], key=itemgetter('identifier'), reverse=True)
 
     form = AdminProviderRatioForm(ratio=providers[0]['priority'])
diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py
index af53b51f4..36abc4519 100644
--- a/tests/app/main/views/test_providers.py
+++ b/tests/app/main/views/test_providers.py
@@ -443,16 +443,17 @@ def test_should_show_version_history_for_first_two_sms_providers(
         'app.provider_client.get_all_providers',
         return_value=stub_providers
     )
-    # second_sms_international will be the primary provider because it’s
-    # 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(
         'app.provider_client.get_provider_versions',
         return_value={'data': [
             {
                 'id': id,
                 'priority': priority,
-                'display_name': sms_provider_int_2['display_name'],
-                'identifier': sms_provider_int_2['identifier'],
+                'display_name': sms_provider_2['display_name'],
+                'identifier': sms_provider_2['identifier'],
                 'updated_at': updated_at,
                 'created_by': {
                     'email_address': 'test@foo.bar',
@@ -489,7 +490,7 @@ def test_should_show_version_history_for_first_two_sms_providers(
         radio['value']
         for radio in page.select('input[checked]')
     ] == [
-        str(sms_provider_int_2['priority'])
+        str(sms_provider_2['priority'])
     ]
 
     assert [
@@ -507,18 +508,18 @@ def test_should_show_version_history_for_first_two_sms_providers(
     ] == [
         (
             'Test User 2:00pm '
-            'Second International SMS Provider 100% '
-            'Second Domestic SMS Provider 0%'
+            'Second Domestic SMS Provider 100% '
+            'First Domestic SMS Provider 0%'
         ),
         (
             'Test User 5:00am '
-            'Second International SMS Provider 80% '
-            'Second Domestic SMS Provider 20%'
+            'Second Domestic SMS Provider 80% '
+            'First Domestic SMS Provider 20%'
         ),
         (
             'Test User 3:00am '
-            'Second International SMS Provider 10% '
-            'Second Domestic SMS Provider 90%'
+            'Second Domestic SMS Provider 10% '
+            'First Domestic SMS Provider 90%'
         ),
     ]
 
@@ -527,15 +528,15 @@ def test_should_show_version_history_for_first_two_sms_providers(
     (
         '10',
         [
-            call(sms_provider_int_2['id'], 10),
-            call(sms_provider_2['id'], 90),
+            call(sms_provider_2['id'], 10),
+            call(sms_provider_1['id'], 90),
         ],
     ),
     (
         '80',
         [
-            call(sms_provider_int_2['id'], 80),
-            call(sms_provider_2['id'], 20),
+            call(sms_provider_2['id'], 80),
+            call(sms_provider_1['id'], 20),
         ],
     ),
 ])