diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index af072d91b..f8b966e84 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -97,7 +97,7 @@ def delete_invitations(): @statsd(namespace="tasks") def switch_current_sms_provider_on_slow_delivery(): """ - Switch providers if there are at least two slow delivery notifications (more than four minutes) + Switch providers if at least 10% of notifications took more than four minutes to be delivered in the last ten minutes. Search from the time we last switched to the current provider. """ current_provider = get_current_provider('sms') diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 534f6d015..d693bde25 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -484,7 +484,7 @@ def is_delivery_slow_for_provider( ).filter( Notification.created_at >= created_at, Notification.sent_at.isnot(None), - Notification.status.in_([NOTIFICATION_DELIVERED, NOTIFICATION_SENDING]), + Notification.status.in_([NOTIFICATION_DELIVERED, NOTIFICATION_PENDING, NOTIFICATION_SENDING]), Notification.sent_by == provider, Notification.key_type != KEY_TYPE_TEST ).group_by("slow").all() diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 90dc73035..0776a45fe 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -45,6 +45,7 @@ from app.models import ( NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_SENDING, + NOTIFICATION_PENDING, NOTIFICATION_SENT, NOTIFICATION_DELIVERED, KEY_TYPE_NORMAL, @@ -1267,15 +1268,16 @@ def test_is_delivery_slow_for_provider( @pytest.mark.parametrize("options,expected_result", [ - ({"status": NOTIFICATION_TEMPORARY_FAILURE, "sent_by": "mmg"}, False), - ({"status": NOTIFICATION_DELIVERED, "sent_by": "firetext"}, False), ({"status": NOTIFICATION_DELIVERED, "sent_by": "mmg"}, True), + ({"status": NOTIFICATION_PENDING, "sent_by": "mmg"}, True), + ({"status": NOTIFICATION_SENDING, "sent_by": "mmg"}, True), + + ({"status": NOTIFICATION_TEMPORARY_FAILURE, "sent_by": "mmg"}, False), ({"status": NOTIFICATION_DELIVERED, "sent_by": "mmg", "sent_at": None}, False), ({"status": NOTIFICATION_DELIVERED, "sent_by": "mmg", "key_type": KEY_TYPE_TEST}, False), ({"status": NOTIFICATION_SENDING, "sent_by": "firetext"}, False), - ({"status": NOTIFICATION_SENDING, "sent_by": "mmg"}, True), - ({"status": NOTIFICATION_SENDING, "sent_by": "mmg", "sent_at": None}, False), - ({"status": NOTIFICATION_SENDING, "sent_by": "mmg", "key_type": KEY_TYPE_TEST}, False), + ({"status": NOTIFICATION_DELIVERED, "sent_by": "firetext"}, False), + ]) @freeze_time("2018-12-04 12:00:00.000000") def test_delivery_is_delivery_slow_for_provider_filters_out_notifications_it_should_not_count(