Refactor to switch properly when providers have equal priority with test

app/dao/provider_details_dao.py

@@ -42,9 +42,8 @@ def get_current_provider(notification_type):
 
 
 @transactional
-def dao_toggle_sms_provider():
-    current_provider = get_current_provider('sms')
-    alternate_provider = get_alternative_sms_provider(current_provider.identifier)
+def dao_toggle_sms_provider(identifier):
+    alternate_provider = get_alternative_sms_provider(identifier)
     dao_switch_sms_provider_to_provider_with_identifier(alternate_provider.identifier)
 
 
@@ -53,12 +52,14 @@ def dao_switch_sms_provider_to_provider_with_identifier(identifier):
     current_provider = get_current_provider('sms')
     new_provider = get_provider_details_by_identifier(identifier)
 
-    if provider_is_already_primary_or_inactive(current_provider, new_provider, identifier):
-        return current_provider
-    else:
-        updated_providers = update_provider_priorities(current_provider, new_provider)
-        for provider in updated_providers:
-            db.session.add(provider)
+    if current_provider.priority == new_provider.priority:
+        # Since both priorities are equal, set the current provider
+        # to the one that we want to switch from
+        current_provider = get_alternative_sms_provider(identifier)
+
+    if not provider_is_already_primary_or_inactive(current_provider, new_provider, identifier):
+        update_provider_priorities(current_provider, new_provider)
+        db.session.add_all([current_provider, new_provider])
 
 
 def get_provider_details_by_notification_type(notification_type):

app/delivery/send_to_providers.py

@@ -47,7 +47,7 @@ def send_sms_to_provider(notification):
                     sender=service.sms_sender
                 )
             except Exception as e:
-                dao_toggle_sms_provider()
+                dao_toggle_sms_provider(provider.name)
                 raise e
             else:
                 notification.billable_units = template.fragment_count

app/provider_details/switch_providers.py

@@ -21,14 +21,12 @@ def update_provider_priorities(current_provider, new_provider):
     # Swap priority to change primary provider
     if new_provider.priority > current_provider.priority:
         new_provider.priority, current_provider.priority = current_provider.priority, new_provider.priority
-        _print_provider_switch_logs(current_provider, new_provider)
-        return [current_provider, new_provider]
 
     # Incease other provider priority if equal
     elif new_provider.priority == current_provider.priority:
         current_provider.priority += 10
-        _print_provider_switch_logs(current_provider, new_provider)
-        return [current_provider]
+
+    _print_provider_switch_logs(current_provider, new_provider)
 
 
 def _print_provider_switch_logs(current_provider, new_provider):

tests/app/dao/test_provider_details_dao.py

@@ -133,8 +133,6 @@ def test_switch_sms_provider_to_current_provider_does_not_switch(
 
 
 def test_switch_sms_provider_to_inactive_provider_does_not_switch(
-    notify_db,
-    notify_db_session,
     restore_provider_details,
     current_sms_provider,
     mocker
@@ -151,13 +149,26 @@ def test_switch_sms_provider_to_inactive_provider_does_not_switch(
 
 
 def test_toggle_sms_provider_switches_provider(
-    notify_db,
-    notify_db_session,
     restore_provider_details,
     current_sms_provider,
     mocker
 ):
-    dao_toggle_sms_provider()
+    dao_toggle_sms_provider(current_sms_provider.identifier)
     new_provider = get_current_provider('sms')
+    assert new_provider.identifier != current_sms_provider.identifier
+    assert new_provider.priority < current_sms_provider.priority
+
+
+def test_toggle_sms_provider_switches_when_provider_priorities_are_equal(
+    restore_provider_details,
+    current_sms_provider,
+    mocker
+):
+    new_provider = get_alternative_sms_provider(current_sms_provider.identifier)
+    current_sms_provider.priority = new_provider.priority
+    dao_update_provider_details(current_sms_provider)
+
+    dao_toggle_sms_provider(current_sms_provider.identifier)
 
     assert new_provider.identifier != current_sms_provider.identifier
+    assert new_provider.priority < current_sms_provider.priority