From d4a300ec7a7fd3af08a67c7e035d4aca253d72cd Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 9 Nov 2016 11:45:39 +0000
Subject: [PATCH] add only_active flag to GET /services/

does what it says on the tin
---
 app/dao/services_dao.py        | 11 ++++++++---
 app/service/rest.py            |  2 +-
 tests/app/conftest.py          |  3 ++-
 tests/app/service/test_rest.py | 17 +++++++++++++++++
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
index ceb1d1635..6a7be124e 100644
--- a/app/dao/services_dao.py
+++ b/app/dao/services_dao.py
@@ -28,12 +28,17 @@ from app.models import (
 from app.statsd_decorators import statsd
 
 
-def dao_fetch_all_services():
-    return Service.query.order_by(
+def dao_fetch_all_services(only_active=False):
+    query = Service.query.order_by(
         asc(Service.created_at)
     ).options(
         joinedload('users')
-    ).all()
+    )
+
+    if only_active:
+        query = query.filter(Service.active)
+
+    return query.all()
 
 
 def dao_fetch_service_by_id(service_id):
diff --git a/app/service/rest.py b/app/service/rest.py
index 4e6d94ba0..fbdbb04bf 100644
--- a/app/service/rest.py
+++ b/app/service/rest.py
@@ -67,7 +67,7 @@ def get_services():
     elif request.args.get('detailed') == 'True':
         return jsonify(data=get_detailed_services())
     else:
-        services = dao_fetch_all_services()
+        services = dao_fetch_all_services(only_active=request.args.get('only_active') == 'True')
     data = service_schema.dump(services, many=True).data
     return jsonify(data=data)
 
diff --git a/tests/app/conftest.py b/tests/app/conftest.py
index 3d1af5359..aa4b7b5fd 100644
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -129,6 +129,7 @@ def sample_service(notify_db,
                    notify_db_session,
                    service_name="Sample service",
                    user=None,
+                   active=True,
                    restricted=False,
                    limit=1000,
                    email_from=None):
@@ -139,7 +140,7 @@ def sample_service(notify_db,
     data = {
         'name': service_name,
         'message_limit': limit,
-        'active': True,
+        'active': active,
         'restricted': restricted,
         'email_from': email_from,
         'created_by': user
diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py
index c1983321e..42184d6de 100644
--- a/tests/app/service/test_rest.py
+++ b/tests/app/service/test_rest.py
@@ -38,6 +38,23 @@ def test_get_service_list(notify_api, service_factory):
             assert json_resp['data'][2]['name'] == 'three'
 
 
+def test_get_service_list_with_only_active_flag(client, service_factory):
+    inactive = service_factory.get('one', email_from='one')
+    active = service_factory.get('two', email_from='two')
+
+    inactive.active = False
+
+    auth_header = create_authorization_header()
+    response = client.get(
+        '/service?only_active=True',
+        headers=[auth_header]
+    )
+    assert response.status_code == 200
+    json_resp = json.loads(response.get_data(as_text=True))
+    assert len(json_resp['data']) == 1
+    assert json_resp['data'][0]['id'] == str(active.id)
+
+
 def test_get_service_list_by_user(notify_db, notify_db_session, client, sample_user, service_factory):
     other_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk')
     service_factory.get('one', sample_user, email_from='one')