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

@@ -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('/<uuid:service_id>/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:
+    if service.active:
-        # assume already inactive, don't change service name
+        dao_archive_service(service.id)
-        return '', 204
-
-    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:
+    if service.active:
-        # assume already inactive, don't change service name
+        dao_suspend_service(service.id)
-        return '', 204
-
-    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:
+    if not service.active:
-        # assume already inactive, don't change service name
+        dao_resume_service(service.id)
-        return '', 204
-
-    dao_resume_service(service.id)
 
     return '', 204
 

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)

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()
+    with freeze_time('2001-10-22T11:59:00'):
-    sample_service.active = False
+        sample_api_key.expiry_date = datetime.utcnow()
-    auth_header = create_authorization_header()
+        sample_service.active = False
-    response = client.post("/service/{}/archive".format(sample_service.id),
+        auth_header = create_authorization_header()
-                           headers=[auth_header])
+        client.post("/service/{}/resume".format(sample_service.id),
-    assert response.status_code == 204
+                    headers=[auth_header])
-    assert sample_api_key.expiry_date.date() == datetime.utcnow().date()
+    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)])