diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 588e14054..d03d08e43 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -42,7 +42,7 @@ from app.dao.notifications_dao import ( update_notification_status_by_reference, dao_get_notification_history_by_reference, ) -from app.dao.provider_details_dao import get_current_provider +from app.dao.provider_details_dao import get_provider_details_by_notification_type from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id @@ -352,7 +352,7 @@ def save_letter( @statsd(namespace="tasks") def update_letter_notifications_to_sent_to_dvla(self, notification_references): # This task will be called by the FTP app to update notifications as sent to DVLA - provider = get_current_provider(LETTER_TYPE) + provider = get_provider_details_by_notification_type(LETTER_TYPE)[0] updated_count, _ = dao_update_notifications_by_reference( notification_references, diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index 58bafc71d..96ccc5d6a 100644 --- a/app/dao/provider_details_dao.py +++ b/app/dao/provider_details_dao.py @@ -29,15 +29,6 @@ def get_alternative_sms_provider(identifier): raise ValueError('Unrecognised sms provider {}'.format(identifier)) -def get_current_provider(notification_type): - return ProviderDetails.query.filter_by( - notification_type=notification_type, - active=True - ).order_by( - asc(ProviderDetails.priority) - ).first() - - def dao_get_provider_versions(provider_id): return ProviderDetailsHistory.query.filter_by( id=provider_id @@ -61,11 +52,6 @@ def dao_reduce_sms_provider_priority(identifier): providers[identifier].priority = max(0, providers[identifier].priority - 10) providers[other].priority = min(100, providers[other].priority + 10) - -def dao_toggle_sms_provider(*args, **kwargs): - raise NotImplementedError - - def get_provider_details_by_notification_type(notification_type, supports_international=False): filters = [ProviderDetails.notification_type == notification_type] diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 9450fc48a..6d3ecaf0f 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -22,10 +22,6 @@ from app.celery.scheduled_tasks import ( from app.config import QueueNames, TaskNames from app.dao.jobs_dao import dao_get_job_by_id from app.dao.notifications_dao import dao_get_scheduled_notifications -from app.dao.provider_details_dao import ( - dao_update_provider_details, - get_current_provider -) from app.models import ( JOB_STATUS_IN_PROGRESS, JOB_STATUS_ERROR, @@ -56,14 +52,6 @@ def _create_slow_delivery_notification(template, provider='mmg'): ) -@pytest.fixture(scope='function') -def prepare_current_provider(restore_provider_details): - initial_provider = get_current_provider('sms') - dao_update_provider_details(initial_provider) - initial_provider.updated_at = datetime.utcnow() - timedelta(minutes=30) - db.session.commit() - - def test_should_call_delete_codes_on_delete_verify_codes_task(notify_db_session, mocker): mocker.patch('app.celery.scheduled_tasks.delete_codes_older_created_more_than_a_day_ago') delete_verify_codes() @@ -115,28 +103,10 @@ def test_should_update_all_scheduled_jobs_and_put_on_queue(sample_template, mock def test_switch_providers_on_slow_delivery_switches_once_then_does_not_switch_if_already_switched( notify_api, mocker, - prepare_current_provider, sample_user, sample_template ): - mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) - starting_provider = get_current_provider('sms') - - _create_slow_delivery_notification(sample_template) - _create_slow_delivery_notification(sample_template) - - switch_current_sms_provider_on_slow_delivery() - - new_provider = get_current_provider('sms') - _create_slow_delivery_notification(sample_template, new_provider.identifier) - _create_slow_delivery_notification(sample_template, new_provider.identifier) - switch_current_sms_provider_on_slow_delivery() - - final_provider = get_current_provider('sms') - - assert new_provider.identifier != starting_provider.identifier - assert new_provider.priority < starting_provider.priority - assert final_provider.identifier == new_provider.identifier + raise NotImplementedError # TODO @freeze_time("2017-05-01 14:00:00") diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index d3d645632..9dfb6daf9 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -2,13 +2,11 @@ import pytest from datetime import datetime from freezegun import freeze_time -from sqlalchemy import asc, desc from app.models import ProviderDetails, ProviderDetailsHistory from app import clients from app.dao.provider_details_dao import ( get_alternative_sms_provider, - get_current_provider, get_provider_details_by_identifier, get_provider_details_by_notification_type, dao_update_provider_details, @@ -25,7 +23,7 @@ from tests.app.db import ( def set_primary_sms_provider(identifier): primary_provider = get_provider_details_by_identifier(identifier) - secondary_provider = get_alternative_sms_provider(identifier) + secondary_provider = get_provider_details_by_identifier(get_alternative_sms_provider(identifier)) primary_provider.priority = 10 secondary_provider.priority = 20 @@ -103,21 +101,12 @@ def test_update_adds_history(restore_provider_details): def test_update_sms_provider_to_inactive_sets_inactive(restore_provider_details): - set_primary_sms_provider('mmg') - primary_provider = get_current_provider('sms') - primary_provider.active = False + mmg = get_provider_details_by_identifier('mmg') - dao_update_provider_details(primary_provider) + mmg.active = False + dao_update_provider_details(mmg) - assert not primary_provider.active - - -def test_get_current_sms_provider_returns_correct_provider(restore_provider_details): - set_primary_sms_provider('mmg') - - provider = get_current_provider('sms') - - assert provider.identifier == 'mmg' + assert not mmg.active @pytest.mark.parametrize('identifier, expected', [ @@ -146,7 +135,7 @@ def test_get_alternative_sms_provider_fails_if_unrecognised(): ({'mmg': -100, 'firetext': 50}, {'mmg': 0, 'firetext': 60}), ({'mmg': 50, 'firetext': -100}, {'mmg': 40, 'firetext': -90}), ]) -def test_change_sms_provider_priority_switches_provider( +def test_reduce_sms_provider_priority_switches_provider( restore_provider_details, starting_priorities, expected_priorities @@ -164,110 +153,53 @@ def test_change_sms_provider_priority_switches_provider( assert mmg.priority == expected_priorities['mmg'] -def test_toggle_sms_provider_switches_when_provider_priorities_are_equal( - mocker, - restore_provider_details, - sample_user -): - mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) - current_provider = get_current_provider('sms') - new_provider = get_alternative_sms_provider(current_provider.identifier) - - current_provider.priority = new_provider.priority - dao_update_provider_details(current_provider) - - dao_toggle_sms_provider(current_provider.identifier) - - old_starting_provider = get_provider_details_by_identifier(current_provider.identifier) - - assert new_provider.identifier != old_starting_provider.identifier - assert new_provider.priority < old_starting_provider.priority - - -def test_toggle_sms_provider_updates_provider_history( +def test_reduce_sms_provider_priority_adds_rows_to_history_table( mocker, restore_provider_details, current_sms_provider, sample_user ): - mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) - provider_history_rows = ProviderDetailsHistory.query.filter( - ProviderDetailsHistory.id == current_sms_provider.id - ).order_by( - desc(ProviderDetailsHistory.version) - ).all() + raise NotImplementedError + # mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) + # provider_history_rows = ProviderDetailsHistory.query.filter( + # ProviderDetailsHistory.id == current_sms_provider.id + # ).order_by( + # desc(ProviderDetailsHistory.version) + # ).all() - dao_toggle_sms_provider(current_sms_provider.identifier) + # dao_toggle_sms_provider(current_sms_provider.identifier) - updated_provider_history_rows = ProviderDetailsHistory.query.filter( - ProviderDetailsHistory.id == current_sms_provider.id - ).order_by( - desc(ProviderDetailsHistory.version) - ).all() + # updated_provider_history_rows = ProviderDetailsHistory.query.filter( + # ProviderDetailsHistory.id == current_sms_provider.id + # ).order_by( + # desc(ProviderDetailsHistory.version) + # ).all() - assert len(updated_provider_history_rows) - len(provider_history_rows) == 1 - assert updated_provider_history_rows[0].version - provider_history_rows[0].version == 1 + # assert len(updated_provider_history_rows) - len(provider_history_rows) == 1 + # assert updated_provider_history_rows[0].version - provider_history_rows[0].version == 1 -def test_toggle_sms_provider_switches_provider_stores_notify_user_id( +def test_reduce_sms_provider_priority_records_notify_user( restore_provider_details, sample_user, mocker ): - mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) + raise NotImplementedError + # mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) - current_provider = get_current_provider('sms') - dao_toggle_sms_provider(current_provider.identifier) - new_provider = get_current_provider('sms') + # current_provider = get_current_provider('sms') + # dao_toggle_sms_provider(current_provider.identifier) + # new_provider = get_current_provider('sms') - assert current_provider.identifier != new_provider.identifier - assert new_provider.created_by.id == sample_user.id - assert new_provider.created_by_id == sample_user.id - - -def test_toggle_sms_provider_switches_provider_stores_notify_user_id_in_history( - restore_provider_details, - sample_user, - mocker -): - mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) - - old_provider = get_current_provider('sms') - dao_toggle_sms_provider(old_provider.identifier) - new_provider = get_current_provider('sms') - - old_provider_from_history = ProviderDetailsHistory.query.filter_by( - identifier=old_provider.identifier, - version=old_provider.version - ).order_by( - asc(ProviderDetailsHistory.priority) - ).first() - new_provider_from_history = ProviderDetailsHistory.query.filter_by( - identifier=new_provider.identifier, - version=new_provider.version - ).order_by( - asc(ProviderDetailsHistory.priority) - ).first() - - assert old_provider.version == old_provider_from_history.version - assert new_provider.version == new_provider_from_history.version - assert new_provider_from_history.created_by_id == sample_user.id - assert old_provider_from_history.created_by_id == sample_user.id + # assert current_provider.identifier != new_provider.identifier + # assert new_provider.created_by.id == sample_user.id + # assert new_provider.created_by_id == sample_user.id def test_can_get_all_provider_history(restore_provider_details, current_sms_provider): assert len(dao_get_provider_versions(current_sms_provider.id)) == 1 -def test_get_current_sms_provider_returns_active_only(restore_provider_details): - current_provider = get_current_provider('sms') - current_provider.active = False - dao_update_provider_details(current_provider) - new_current_provider = get_current_provider('sms') - - assert current_provider.identifier != new_current_provider.identifier - - @freeze_time('2018-06-28 12:00') def test_dao_get_provider_stats(notify_db_session): service_1 = create_service(service_name='1')