diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 227444c7a..6daf54b55 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -15,12 +15,14 @@ from app.dao.provider_details_dao import ( ) from app.celery.research_mode_tasks import send_sms_response, send_email_response from app.dao.templates_dao import dao_get_template_by_id -from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE +from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE, NOTIFICATION_TECHNICAL_FAILURE def send_sms_to_provider(notification): service = notification.service - if not notification.service.active: + if not service.active: + notification.status = NOTIFICATION_TECHNICAL_FAILURE + dao_update_notification(notification) current_app.logger.warn( "Send sms for notification id {} to provider is not allowed: service {} is inactive".format(notification.id, service.id)) @@ -67,7 +69,9 @@ def send_sms_to_provider(notification): def send_email_to_provider(notification): service = notification.service - if not notification.service.active: + if not service.active: + notification.status = NOTIFICATION_TECHNICAL_FAILURE + dao_update_notification(notification) current_app.logger.warn( "Send email for notification id {} to provider is not allowed: service {} is inactive".format( notification.id, diff --git a/app/service/rest.py b/app/service/rest.py index 416bace74..8c21c1202 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -336,20 +336,6 @@ def update_whitelist(service_id): return '', 204 -# Renaming this endpoint to archive -@service_blueprint.route('//deactivate', methods=['POST']) -def deactivate_service(service_id): - service = dao_fetch_service_by_id(service_id) - - if not service.active: - # assume already inactive, don't change service name - return '', 204 - - dao_archive_service(service.id) - - return '', 204 - - @service_blueprint.route('//archive', methods=['POST']) def archive_service(service_id): """ diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index e12e4e9e4..90cd4718a 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -121,13 +121,14 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist ["app.aws_ses_client.send_email", "app.mmg_client.send_sms", "app.firetext_client.send_sms"]) -def test_should_not_send_message_when_service_is_inactive(sample_service, sample_notification, mocker, - client_send): +def test_should_not_send_message_when_service_is_inactive_notiifcation_is_in_tech_failure( + sample_service, sample_notification, mocker, client_send): sample_service.active = False - send_to_providers.send_email_to_provider(sample_notification) send_mock = mocker.patch(client_send, return_value='reference') + send_to_providers.send_email_to_provider(sample_notification) send_mock.assert_not_called() + assert Notification.query.get(sample_notification.id).status == 'technical-failure' def test_send_sms_should_use_template_version_from_notification_not_latest( diff --git a/tests/app/service/test_deactivate.py b/tests/app/service/test_deactivate.py deleted file mode 100644 index 7ef20d8e8..000000000 --- a/tests/app/service/test_deactivate.py +++ /dev/null @@ -1,123 +0,0 @@ -import uuid -from datetime import datetime - -import pytest -from freezegun import freeze_time - -from app import db -from app.models import Service -from app.dao.services_dao import dao_archive_service -from app.dao.api_key_dao import expire_api_key -from app.dao.templates_dao import dao_update_template - -from tests import create_authorization_header, unwrap_function -from tests.app.conftest import ( - sample_template as create_template, - sample_api_key as create_api_key -) - - -def test_deactivate_only_allows_post(client): - auth_header = create_authorization_header() - response = client.get('/service/{}/deactivate'.format(uuid.uuid4()), headers=[auth_header]) - assert response.status_code == 405 - - -def test_deactivate_service_errors_with_bad_service_id(client): - auth_header = create_authorization_header() - response = client.post('/service/{}/deactivate'.format(uuid.uuid4()), headers=[auth_header]) - assert response.status_code == 404 - - -def test_deactivating_inactive_service_does_nothing(client, sample_service): - auth_header = create_authorization_header() - sample_service.active = False - response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) - assert response.status_code == 204 - assert sample_service.name == 'Sample service' - - -@pytest.fixture -def deactivated_service(client, notify_db, notify_db_session, sample_service): - create_template(notify_db, notify_db_session, template_name='a') - create_template(notify_db, notify_db_session, template_name='b') - create_api_key(notify_db, notify_db_session) - create_api_key(notify_db, notify_db_session) - - auth_header = create_authorization_header() - response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) - assert response.status_code == 204 - assert response.data == b'' - return sample_service - - -def test_deactivating_service_changes_name_and_email(deactivated_service): - assert deactivated_service.name == '_archived_Sample service' - assert deactivated_service.email_from == '_archived_sample.service' - - -def test_deactivating_service_revokes_api_keys(deactivated_service): - assert len(deactivated_service.api_keys) == 2 - for key in deactivated_service.api_keys: - assert key.expiry_date is not None - assert key.version == 2 - - -def test_deactivating_service_archives_templates(deactivated_service): - assert len(deactivated_service.templates) == 2 - for template in deactivated_service.templates: - assert template.archived is True - assert template.version == 2 - - -def test_deactivating_service_creates_history(deactivated_service): - ServiceHistory = Service.get_history_model() - history = ServiceHistory.query.filter_by( - id=deactivated_service.id - ).order_by( - ServiceHistory.version.desc() - ).first() - - assert history.version == 2 - assert history.active is False - - -@pytest.fixture -def deactivated_service_with_deleted_stuff(client, notify_db, notify_db_session, sample_service): - with freeze_time('2001-01-01'): - template = create_template(notify_db, notify_db_session, template_name='a') - api_key = create_api_key(notify_db, notify_db_session) - - expire_api_key(sample_service.id, api_key.id) - - template.archived = True - dao_update_template(template) - - with freeze_time('2002-02-02'): - auth_header = create_authorization_header() - response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) - - assert response.status_code == 204 - assert response.data == b'' - return sample_service - - -def test_deactivating_service_doesnt_affect_existing_archived_templates(deactivated_service_with_deleted_stuff): - assert deactivated_service_with_deleted_stuff.templates[0].archived is True - assert deactivated_service_with_deleted_stuff.templates[0].updated_at == datetime(2001, 1, 1, 0, 0, 0) - assert deactivated_service_with_deleted_stuff.templates[0].version == 2 - - -def test_deactivating_service_doesnt_affect_existing_revoked_api_keys(deactivated_service_with_deleted_stuff): - assert deactivated_service_with_deleted_stuff.api_keys[0].expiry_date == datetime(2001, 1, 1, 0, 0, 0) - assert deactivated_service_with_deleted_stuff.api_keys[0].version == 2 - - -def test_deactivating_service_rolls_back_everything_on_error(sample_service, sample_api_key, sample_template): - unwrapped_deactive_service = unwrap_function(dao_archive_service) - - unwrapped_deactive_service(sample_service.id) - - assert sample_service in db.session.dirty - assert sample_api_key in db.session.dirty - assert sample_template in db.session.dirty