From 907a1e91ac003647d57ac5ea76e9a5b03101c19d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 31 Jan 2017 11:33:36 +0000 Subject: [PATCH] Removed the need for 2 return statements. Update tests to fix copy/paste errors, use freezetime to ensure there is never a random failing test. --- app/service/rest.py | 25 +++++----------- tests/app/dao/test_services_dao.py | 6 ++-- .../service/test_suspend_resume_service.py | 30 ++++++++++++------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 138b74f39..416bace74 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -205,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, @@ -330,7 +330,7 @@ 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 @@ -360,11 +360,8 @@ def archive_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) + if service.active: + dao_archive_service(service.id) return '', 204 @@ -378,11 +375,8 @@ def suspend_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_suspend_service(service.id) + if service.active: + dao_suspend_service(service.id) return '', 204 @@ -397,11 +391,8 @@ def resume_service(service_id): """ service = dao_fetch_service_by_id(service_id) - if service.active: - # assume already inactive, don't change service name - return '', 204 - - dao_resume_service(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 5fb2c01d9..52624e0d1 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -659,6 +659,7 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session 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) @@ -666,9 +667,10 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(samp assert service.name == sample_service.name api_key = ApiKey.query.get(sample_api_key.id) - assert api_key.expiry_date.date() == datetime.utcnow().date() + 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) @@ -678,4 +680,4 @@ def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revok assert Service.query.get(service.id).active api_key = ApiKey.query.get(sample_api_key.id) - assert api_key.expiry_date.date() == datetime.utcnow().date() + assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) diff --git a/tests/app/service/test_suspend_resume_service.py b/tests/app/service/test_suspend_resume_service.py index 12f1b9239..8f33a22d2 100644 --- a/tests/app/service/test_suspend_resume_service.py +++ b/tests/app/service/test_suspend_resume_service.py @@ -4,7 +4,9 @@ import pytest import uuid -from app.models import Service +from freezegun import freeze_time + +from app.models import Service, ApiKey from tests import create_authorization_header @@ -33,25 +35,31 @@ def test_has_not_effect_when_service_is_already_that_state(client, sample_servic headers=[auth_header]) assert response.status_code == 204 mocked.assert_not_called() - assert mocked.called == 0 + 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/{}/archive".format(sample_service.id), + response = client.post("/service/{}/suspend".format(sample_service.id), headers=[auth_header]) assert response.status_code == 204 - assert sample_api_key.expiry_date.date() == datetime.utcnow().date() + 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): - sample_api_key.expiry_date = datetime.utcnow() - sample_service.active = False - auth_header = create_authorization_header() - response = client.post("/service/{}/archive".format(sample_service.id), - headers=[auth_header]) - assert response.status_code == 204 - assert sample_api_key.expiry_date.date() == datetime.utcnow().date() + with freeze_time('2001-10-22T11:59:00'): + sample_api_key.expiry_date = datetime.utcnow() + sample_service.active = False + auth_header = create_authorization_header() + client.post("/service/{}/resume".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)])