From d466265a9dcbfcd7c5e403485ab517f6cf7a4868 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 20 Jan 2017 16:13:13 +0000 Subject: [PATCH] Refactor provider switch methods in dao and tests --- app/dao/provider_details_dao.py | 52 +++++----------------- app/provider_details/switch_providers.py | 48 ++++++++++++++++++++ tests/app/conftest.py | 12 ----- tests/app/dao/test_provider_details_dao.py | 15 +++++-- 4 files changed, 70 insertions(+), 57 deletions(-) create mode 100644 app/provider_details/switch_providers.py diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index 52938057e..b4a017483 100644 --- a/app/dao/provider_details_dao.py +++ b/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( - current_provider.identifier, - new_provider.identifier, - new_provider.identifier - )) + + if provider_is_already_primary_or_inactive(current_provider, new_provider, identifier): return current_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) - 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 - )) + else: + updated_providers = update_provider_priorities(current_provider, new_provider) + for provider in updated_providers: + db.session.add(provider) def get_provider_details_by_notification_type(notification_type): diff --git a/app/provider_details/switch_providers.py b/app/provider_details/switch_providers.py new file mode 100644 index 000000000..ebb2e7d23 --- /dev/null +++ b/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 + )) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 876d4519b..1d62aa996 100644 --- a/tests/app/conftest.py +++ b/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() diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index 09f34a7c2..db932f7f9 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/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