From b4d4133b1f91f2cdc3439fa58edf832bedca1c1c Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 17 Mar 2022 16:35:46 +0000 Subject: [PATCH] Fix SMS priority adjustment if only 1 provider Fixes: > reduced_provider = providers[identifier] E KeyError: 'firetext' Note that the mock return value in the other test was wrong [^1]. [^1]: https://github.com/alphagov/notifications-api/blob/bff97f0bbe835e4cb67ac60cfdac6d1cd6e51ced/app/dao/provider_details_dao.py#L73 --- app/dao/provider_details_dao.py | 3 ++- tests/app/dao/test_provider_details_dao.py | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index a3e3cf24c..69665ef4d 100644 --- a/app/dao/provider_details_dao.py +++ b/app/dao/provider_details_dao.py @@ -84,7 +84,8 @@ def dao_reduce_sms_provider_priority(identifier, *, time_threshold): amount_to_reduce_by = 10 providers_list = _get_sms_providers_for_update(time_threshold) - if not providers_list: + if len(providers_list) < 2: + current_app.logger.info("Not adjusting providers, number of active providers is less than 2.") return providers = {provider.identifier: provider for provider in providers_list} diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index a77f1a52b..eb7ed5d98 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -234,7 +234,7 @@ def test_reduce_sms_provider_priority_does_nothing_if_providers_have_recently_ch mocker, restore_provider_details, ): - mock_get_providers = mocker.patch('app.dao.provider_details_dao._get_sms_providers_for_update', return_value=None) + mock_get_providers = mocker.patch('app.dao.provider_details_dao._get_sms_providers_for_update', return_value=[]) mock_adjust = mocker.patch('app.dao.provider_details_dao._adjust_provider_priority') dao_reduce_sms_provider_priority('firetext', time_threshold=timedelta(minutes=5)) @@ -243,6 +243,20 @@ def test_reduce_sms_provider_priority_does_nothing_if_providers_have_recently_ch assert mock_adjust.called is False +def test_reduce_sms_provider_priority_does_nothing_if_there_is_only_one_active_provider( + mocker, + restore_provider_details, +): + firetext = get_provider_details_by_identifier('firetext') + firetext.active = False + + mock_adjust = mocker.patch('app.dao.provider_details_dao._adjust_provider_priority') + + dao_reduce_sms_provider_priority('firetext', time_threshold=timedelta(minutes=5)) + + assert mock_adjust.called is False + + @pytest.mark.parametrize('existing_mmg, existing_firetext, new_mmg, new_firetext', [ (50, 50, 60, 40), # not just 50/50 - 60/40 specifically (65, 35, 60, 40), # doesn't overshoot if there's less than 10 difference