diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index aaefec145..eb53c769c 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -73,7 +73,7 @@ def dao_fetch_all_services_by_user(user_id, only_active=False): @version_class(Service) @version_class(Template, TemplateHistory) @version_class(ApiKey) -def dao_deactive_service(service_id): +def dao_archive_service(service_id): # have to eager load templates and api keys so that we don't flush when we loop through them # to ensure that db.session still contains the models when it comes to creating history objects service = Service.query.options( @@ -291,3 +291,27 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) return query.all() + + +@transactional +@version_class(Service) +@version_class(ApiKey) +def dao_suspend_service(service_id): + # have to eager load api keys so that we don't flush when we loop through them + # to ensure that db.session still contains the models when it comes to creating history objects + service = Service.query.options( + joinedload('api_keys'), + ).filter(Service.id == service_id).one() + + service.active = False + + for api_key in service.api_keys: + if not api_key.expiry_date: + api_key.expiry_date = datetime.utcnow() + + +@transactional +@version_class(Service) +def dao_resume_service(service_id): + service = Service.query.get(service_id) + service.active = True diff --git a/app/service/rest.py b/app/service/rest.py index 905408a3b..416bace74 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -27,9 +27,10 @@ from app.dao.services_dao import ( dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, dao_fetch_todays_stats_for_all_services, - dao_deactive_service, - fetch_stats_by_date_range_for_all_services -) + dao_archive_service, + fetch_stats_by_date_range_for_all_services, + dao_suspend_service, + dao_resume_service) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, dao_add_and_commit_whitelisted_contacts, @@ -204,7 +205,7 @@ def get_service_provider_aggregate_statistics(service_id): # tables. This is so product owner can pass stories as done @service_blueprint.route('//history', methods=['GET']) def get_service_history(service_id): - from app.models import (Service, ApiKey, Template, TemplateHistory, Event) + from app.models import (Service, ApiKey, TemplateHistory, Event) from app.schemas import ( service_history_schema, api_key_history_schema, @@ -329,12 +330,13 @@ def update_whitelist(service_id): current_app.logger.exception(e) dao_rollback() msg = '{} is not a valid email address or phone number'.format(str(e)) - return jsonify(result='error', message=msg), 400 + raise InvalidRequest(msg, 400) else: dao_add_and_commit_whitelisted_contacts(whitelist_objs) 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) @@ -343,7 +345,54 @@ def deactivate_service(service_id): # assume already inactive, don't change service name return '', 204 - dao_deactive_service(service.id) + dao_archive_service(service.id) + + return '', 204 + + +@service_blueprint.route('//archive', methods=['POST']) +def archive_service(service_id): + """ + When a service is archived the service is made inactive, templates are archived and api keys are revoked. + There is no coming back from this operation. + :param service_id: + :return: + """ + service = dao_fetch_service_by_id(service_id) + + if service.active: + dao_archive_service(service.id) + + return '', 204 + + +@service_blueprint.route('//suspend', methods=['POST']) +def suspend_service(service_id): + """ + Suspending a service will mark the service as inactive and revoke API keys. + :param service_id: + :return: + """ + service = dao_fetch_service_by_id(service_id) + + if service.active: + dao_suspend_service(service.id) + + return '', 204 + + +@service_blueprint.route('//resume', methods=['POST']) +def resume_service(service_id): + """ + Resuming a service that has been suspended will mark the service as active. + The service will need to re-create API keys + :param service_id: + :return: + """ + service = dao_fetch_service_by_id(service_id) + + if not service.active: + dao_resume_service(service.id) return '', 204 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index a3f5d1de7..52624e0d1 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -22,8 +22,9 @@ from app.dao.services_dao import ( dao_fetch_weekly_historical_stats_for_service, fetch_todays_total_message_count, dao_fetch_todays_stats_for_all_services, - fetch_stats_by_date_range_for_all_services -) + fetch_stats_by_date_range_for_all_services, + dao_suspend_service, + dao_resume_service) from app.dao.users_dao import save_model_user from app.models import ( NotificationStatistics, @@ -656,3 +657,27 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session assert len(results) == 1 assert results[0] == ('sms', 'created', result_one.service_id, 2) + + +@freeze_time('2001-01-01T23:59:00') +def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(sample_service, sample_api_key): + dao_suspend_service(sample_service.id) + service = Service.query.get(sample_service.id) + assert not service.active + assert service.name == sample_service.name + + api_key = ApiKey.query.get(sample_api_key.id) + assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) + + +@freeze_time('2001-01-01T23:59:00') +def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revoked(sample_service, sample_api_key): + dao_suspend_service(sample_service.id) + service = Service.query.get(sample_service.id) + assert not service.active + + dao_resume_service(service.id) + assert Service.query.get(service.id).active + + api_key = ApiKey.query.get(sample_api_key.id) + assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) diff --git a/tests/app/service/test_archived_service.py b/tests/app/service/test_archived_service.py new file mode 100644 index 000000000..b06e7ba01 --- /dev/null +++ b/tests/app/service/test_archived_service.py @@ -0,0 +1,123 @@ +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_archive_only_allows_post(client): + auth_header = create_authorization_header() + response = client.get('/service/{}/archive'.format(uuid.uuid4()), headers=[auth_header]) + assert response.status_code == 405 + + +def test_archive_service_errors_with_bad_service_id(client): + auth_header = create_authorization_header() + response = client.post('/service/{}/archive'.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/{}/archive'.format(sample_service.id), headers=[auth_header]) + assert response.status_code == 204 + assert sample_service.name == 'Sample service' + + +@pytest.fixture +def archived_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/{}/archive'.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(archived_service): + assert archived_service.name == '_archived_Sample service' + assert archived_service.email_from == '_archived_sample.service' + + +def test_deactivating_service_revokes_api_keys(archived_service): + assert len(archived_service.api_keys) == 2 + for key in archived_service.api_keys: + assert key.expiry_date is not None + assert key.version == 2 + + +def test_deactivating_service_archives_templates(archived_service): + assert len(archived_service.templates) == 2 + for template in archived_service.templates: + assert template.archived is True + assert template.version == 2 + + +def test_deactivating_service_creates_history(archived_service): + ServiceHistory = Service.get_history_model() + history = ServiceHistory.query.filter_by( + id=archived_service.id + ).order_by( + ServiceHistory.version.desc() + ).first() + + assert history.version == 2 + assert history.active is False + + +@pytest.fixture +def archived_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/{}/archive'.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(archived_service_with_deleted_stuff): + assert archived_service_with_deleted_stuff.templates[0].archived is True + assert archived_service_with_deleted_stuff.templates[0].updated_at == datetime(2001, 1, 1, 0, 0, 0) + assert archived_service_with_deleted_stuff.templates[0].version == 2 + + +def test_deactivating_service_doesnt_affect_existing_revoked_api_keys(archived_service_with_deleted_stuff): + assert archived_service_with_deleted_stuff.api_keys[0].expiry_date == datetime(2001, 1, 1, 0, 0, 0) + assert archived_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 diff --git a/tests/app/service/test_deactivate.py b/tests/app/service/test_deactivate.py index 2ed2c01a6..7ef20d8e8 100644 --- a/tests/app/service/test_deactivate.py +++ b/tests/app/service/test_deactivate.py @@ -6,7 +6,7 @@ from freezegun import freeze_time from app import db from app.models import Service -from app.dao.services_dao import dao_deactive_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 @@ -114,7 +114,7 @@ def test_deactivating_service_doesnt_affect_existing_revoked_api_keys(deactivate def test_deactivating_service_rolls_back_everything_on_error(sample_service, sample_api_key, sample_template): - unwrapped_deactive_service = unwrap_function(dao_deactive_service) + unwrapped_deactive_service = unwrap_function(dao_archive_service) unwrapped_deactive_service(sample_service.id) diff --git a/tests/app/service/test_suspend_resume_service.py b/tests/app/service/test_suspend_resume_service.py new file mode 100644 index 000000000..7f38d1be6 --- /dev/null +++ b/tests/app/service/test_suspend_resume_service.py @@ -0,0 +1,78 @@ +from datetime import datetime + +import pytest + +import uuid + +from freezegun import freeze_time + +from app.models import Service, ApiKey +from tests import create_authorization_header + + +@pytest.mark.parametrize("endpoint", ["suspend", "resume"]) +def test_only_allows_post(client, endpoint): + auth_header = create_authorization_header() + response = client.get("/service/{}/{}".format(uuid.uuid4(), endpoint), + headers=[auth_header]) + assert response.status_code == 405 + + +@pytest.mark.parametrize("endpoint", ["suspend", "resume"]) +def test_returns_404_when_service_does_not_exist(client, endpoint): + auth_header = create_authorization_header() + response = client.post("/service/{}/{}".format(uuid.uuid4(), endpoint), + headers=[auth_header]) + assert response.status_code == 404 + + +@pytest.mark.parametrize("action, active", [("suspend", False), ("resume", True)]) +def test_has_not_effect_when_service_is_already_that_state(client, sample_service, action, active, mocker): + mocked = mocker.patch("app.service.rest.dao_{}_service".format(action)) + sample_service.active = active + auth_header = create_authorization_header() + response = client.post("/service/{}/{}".format(sample_service.id, action), + headers=[auth_header]) + assert response.status_code == 204 + mocked.assert_not_called() + assert sample_service.active == active + + +@freeze_time('2001-01-01T23:59:00') +def test_suspending_service_revokes_api_keys(client, sample_service, sample_api_key): + auth_header = create_authorization_header() + response = client.post("/service/{}/suspend".format(sample_service.id), + headers=[auth_header]) + assert response.status_code == 204 + assert sample_api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) + + +def test_resume_service_leaves_api_keys_revokes(client, sample_service, sample_api_key): + with freeze_time('2001-10-22T11:59:00'): + auth_header = create_authorization_header() + client.post("/service/{}/suspend".format(sample_service.id), + headers=[auth_header]) + with freeze_time('2001-10-22T13:59:00'): + auth_header = create_authorization_header() + response = client.post("/service/{}/resume".format(sample_service.id), + headers=[auth_header]) + assert response.status_code == 204 + assert sample_api_key.expiry_date == datetime(2001, 10, 22, 11, 59, 00) + + +@pytest.mark.parametrize("action, original_state", [("suspend", True), ("resume", False)]) +def test_service_history_is_created(client, sample_service, action, original_state): + sample_service.active = original_state + auth_header = create_authorization_header() + response = client.post("/service/{}/{}".format(sample_service.id, action), + headers=[auth_header]) + ServiceHistory = Service.get_history_model() + history = ServiceHistory.query.filter_by( + id=sample_service.id + ).order_by( + ServiceHistory.version.desc() + ).first() + + assert response.status_code == 204 + assert history.version == 2 + assert history.active != original_state