Refactor provider switch methods in dao and tests

app/dao/provider_details_dao.py

@@ -1,9 +1,12 @@
 from datetime import datetime
 
-from flask import current_app
 from sqlalchemy import asc
 
 from app.dao.dao_utils import transactional
+from app.provider_details.switch_providers import (
+    provider_is_already_primary_or_inactive,
+    update_provider_priorities
+)
 from app.models import ProviderDetails, ProviderDetailsHistory
 from app import db
 
@@ -42,55 +45,20 @@ def get_current_provider(notification_type):
 def dao_toggle_sms_provider():
     current_provider = get_current_provider('sms')
     alternate_provider = get_alternative_sms_provider(current_provider.identifier)
-
     dao_switch_sms_provider_to_provider_with_identifier(alternate_provider.identifier)
 
 
 @transactional
 def dao_switch_sms_provider_to_provider_with_identifier(identifier):
-    # Do nothing if the provider is already primary or set as inactive
     current_provider = get_current_provider('sms')
-    if current_provider.identifier == identifier:
-        current_app.logger.warning('Provider {} is already activated'.format(current_provider.display_name))
-        return current_provider
-
     new_provider = get_provider_details_by_identifier(identifier)
-    if not new_provider.active:
+
-        current_app.logger.warning('Cancelling switch from {} to {} as {} is inactive'.format(
+    if provider_is_already_primary_or_inactive(current_provider, new_provider, identifier):
-            current_provider.identifier,
-            new_provider.identifier,
-            new_provider.identifier
-        ))
         return current_provider
-
+    else:
-    # Swap priority to change primary provider
+        updated_providers = update_provider_priorities(current_provider, new_provider)
-    if new_provider.priority > current_provider.priority:
+        for provider in updated_providers:
-        new_provider.priority, current_provider.priority = current_provider.priority, new_provider.priority
+            db.session.add(provider)
-        _print_provider_switch_logs(current_provider, new_provider)
-        db.session.add_all([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)
-        db.session.add(current_provider)
-
-
-def _print_provider_switch_logs(current_provider, new_provider):
-    current_app.logger.warning('Switching provider from {} to {}'.format(
-        current_provider.identifier,
-        new_provider.identifier
-    ))
-
-    current_app.logger.warning('Provider {} now updated with priority of {}'.format(
-        current_provider.identifier,
-        current_provider.priority
-    ))
-
-    current_app.logger.warning('Provider {} now updated with priority of {}'.format(
-        new_provider.identifier,
-        new_provider.priority
-    ))
 
 
 def get_provider_details_by_notification_type(notification_type):

app/provider_details/switch_providers.py

@@ -0,0 +1,48 @@
+from flask import current_app
+
+
+def provider_is_already_primary_or_inactive(current_provider, new_provider, identifier):
+    if current_provider.identifier == identifier:
+        current_app.logger.warning('Provider {} is already activated'.format(current_provider.display_name))
+        return True
+
+    elif not new_provider.active:
+        current_app.logger.warning('Cancelling switch from {} to {} as {} is inactive'.format(
+            current_provider.identifier,
+            new_provider.identifier,
+            new_provider.identifier
+        ))
+        return True
+
+    return False
+
+
+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]
+
+
+def _print_provider_switch_logs(current_provider, new_provider):
+    current_app.logger.warning('Switching provider from {} to {}'.format(
+        current_provider.identifier,
+        new_provider.identifier
+    ))
+
+    current_app.logger.warning('Provider {} now updated with priority of {}'.format(
+        current_provider.identifier,
+        current_provider.priority
+    ))
+
+    current_app.logger.warning('Provider {} now updated with priority of {}'.format(
+        new_provider.identifier,
+        new_provider.priority
+    ))

tests/app/conftest.py

@@ -642,18 +642,6 @@ def current_sms_provider():
     ).first()
 
 
-@pytest.fixture(scope='function')
-def set_primary_sms_provider(identifier='mmg'):
-    primary_provider = get_provider_details_by_identifier(identifier)
-    secondary_provider = get_alternative_sms_provider(identifier)
-
-    primary_provider.priority = 10
-    secondary_provider.priority = 20
-
-    dao_update_provider_details(primary_provider)
-    dao_update_provider_details(secondary_provider)
-
-
 @pytest.fixture(scope='function')
 def ses_provider():
     return ProviderDetails.query.filter_by(identifier='ses').one()

tests/app/dao/test_provider_details_dao.py

@@ -10,12 +10,23 @@ from app.dao.provider_details_dao import (
     get_alternative_sms_provider,
     get_current_provider,
     get_provider_details,
+    get_provider_details_by_identifier,
     get_provider_details_by_notification_type,
     dao_switch_sms_provider_to_provider_with_identifier,
     dao_toggle_sms_provider,
     dao_update_provider_details
 )
-from tests.app.conftest import set_primary_sms_provider
+
+
+def set_primary_sms_provider(identifier='mmg'):
+    primary_provider = get_provider_details_by_identifier(identifier)
+    secondary_provider = get_alternative_sms_provider(identifier)
+
+    primary_provider.priority = 10
+    secondary_provider.priority = 20
+
+    dao_update_provider_details(primary_provider)
+    dao_update_provider_details(secondary_provider)
 
 
 def test_can_get_all_providers(restore_provider_details):
@@ -110,8 +121,6 @@ def test_get_alternative_sms_provider_returns_expected_provider(notify_db, provi
 
 
 def test_switch_sms_provider_to_current_provider_does_not_switch(
-    notify_db,
-    notify_db_session,
     restore_provider_details,
     current_sms_provider,
     mocker