diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index b4a017483..1ee2a99ae 100644 --- a/app/dao/provider_details_dao.py +++ b/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): diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 526cd5b13..e74cc174d 100644 --- a/app/delivery/send_to_providers.py +++ b/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 diff --git a/app/provider_details/switch_providers.py b/app/provider_details/switch_providers.py index ebb2e7d23..6d2a992cb 100644 --- a/app/provider_details/switch_providers.py +++ b/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): diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index db932f7f9..9050248c7 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/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