Merge pull request #1343 from alphagov/improve-get-services

Improve get services

app/dao/services_dao.py

@@ -369,9 +369,9 @@ def dao_fetch_monthly_historical_stats_for_service(service_id, year):
 
 
 @statsd(namespace='dao')
-def dao_fetch_todays_stats_for_all_services(include_from_test_key=True):
+def dao_fetch_todays_stats_for_all_services(include_from_test_key=True, only_active=True):
 
-    query = db.session.query(
+    subquery = db.session.query(
         Notification.notification_type,
         Notification.status,
         Notification.service_id,
@@ -382,26 +382,43 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True):
         Notification.notification_type,
         Notification.status,
         Notification.service_id
-    ).order_by(
-        Notification.service_id
     )
 
     if not include_from_test_key:
-        query = query.filter(Notification.key_type != KEY_TYPE_TEST)
+        subquery = subquery.filter(Notification.key_type != KEY_TYPE_TEST)
+
+    subquery = subquery.subquery()
+
+    query = db.session.query(
+        Service.id.label('service_id'),
+        Service.name,
+        Service.restricted,
+        Service.research_mode,
+        Service.active,
+        Service.created_at,
+        subquery.c.notification_type,
+        subquery.c.status,
+        subquery.c.count
+    ).outerjoin(
+        subquery,
+        subquery.c.service_id == Service.id
+    ).order_by(Service.id)
+
+    if only_active:
+        query = query.filter(Service.active)
 
     return query.all()
 
 
 @statsd(namespace='dao')
-def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True):
+def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True, only_active=True):
     start_date = get_london_midnight_in_utc(start_date)
     end_date = get_london_midnight_in_utc(end_date + timedelta(days=1))
     table = NotificationHistory
 
     if start_date >= datetime.utcnow() - timedelta(days=7):
         table = Notification
-
+    subquery = db.session.query(
-    query = db.session.query(
         table.notification_type,
         table.status,
         table.service_id,
@@ -413,12 +430,27 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro
         table.notification_type,
         table.status,
         table.service_id
-    ).order_by(
-        table.service_id
     )
-
     if not include_from_test_key:
-        query = query.filter(table.key_type != KEY_TYPE_TEST)
+        subquery = subquery.filter(table.key_type != KEY_TYPE_TEST)
+    subquery = subquery.subquery()
+
+    query = db.session.query(
+        Service.id.label('service_id'),
+        Service.name,
+        Service.restricted,
+        Service.research_mode,
+        Service.active,
+        Service.created_at,
+        subquery.c.notification_type,
+        subquery.c.status,
+        subquery.c.count
+    ).outerjoin(
+        subquery,
+        subquery.c.service_id == Service.id
+    ).order_by(Service.id)
+    if only_active:
+        query = query.filter(Service.active)
 
     return query.all()
 

app/service/rest.py

@@ -409,23 +409,33 @@ def get_detailed_service(service_id, today_only=False):
 
 
 def get_detailed_services(start_date, end_date, only_active=False, include_from_test_key=True):
-    services = {service.id: service for service in dao_fetch_all_services(only_active)}
     if start_date == datetime.utcnow().date():
-        stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key)
+        stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key,
+                                                        only_active=only_active)
     else:
 
         stats = fetch_stats_by_date_range_for_all_services(start_date=start_date,
                                                            end_date=end_date,
-                                                           include_from_test_key=include_from_test_key)
+                                                           include_from_test_key=include_from_test_key,
-
+                                                           only_active=only_active)
+    results = []
     for service_id, rows in itertools.groupby(stats, lambda x: x.service_id):
-        services[service_id].statistics = statistics.format_statistics(rows)
+        rows = list(rows)
-
+        if rows[0].count is None:
-    # if service has not sent anything, query will not have set statistics correctly
+            s = statistics.create_zeroed_stats_dicts()
-    for service in services.values():
+        else:
-        if not hasattr(service, 'statistics'):
+            s = statistics.format_statistics(rows)
-            service.statistics = statistics.create_zeroed_stats_dicts()
+        results.append({
-    return detailed_service_schema.dump(services.values(), many=True).data
+            'id': str(rows[0].service_id),
+            'name': rows[0].name,
+            'notification_type': rows[0].notification_type,
+            'research_mode': rows[0].research_mode,
+            'restricted': rows[0].restricted,
+            'active': rows[0].active,
+            'created_at': rows[0].created_at,
+            'statistics': s
+        })
+    return results
 
 
 @service_blueprint.route('/<uuid:service_id>/whitelist', methods=['GET'])

tests/app/dao/test_services_dao.py

@@ -682,7 +682,7 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_services(notify_db
     assert stats == sorted(stats, key=lambda x: x.service_id)
 
 
-def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db):
+def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db, notify_db_session):
     with freeze_time('2001-01-01T23:59:00'):
         just_before_midnight_yesterday = create_notification(notify_db, None, to_field='1', status='delivered')
 
@@ -711,12 +711,15 @@ def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db, not
     create_notification(notify_db, notify_db_session, service=service2)
 
     stats = dao_fetch_todays_stats_for_all_services()
-
     assert len(stats) == 4
-    assert ('sms', 'created', service1.id, 2) in stats
+    assert (service1.id, service1.name, service1.restricted, service1.research_mode, service1.active,
-    assert ('sms', 'failed', service1.id, 1) in stats
+            service1.created_at, 'sms', 'created', 2) in stats
-    assert ('email', 'created', service1.id, 1) in stats
+    assert (service1.id, service1.name, service1.restricted, service1.research_mode, service1.active,
-    assert ('sms', 'created', service2.id, 1) in stats
+            service1.created_at, 'sms', 'failed', 1) in stats
+    assert (service1.id, service1.name, service1.restricted, service1.research_mode, service1.active,
+            service1.created_at, 'email', 'created', 1) in stats
+    assert (service2.id, service2.name, service2.restricted, service2.research_mode, service2.active,
+            service2.created_at, 'sms', 'created', 1) in stats
 
 
 def test_dao_fetch_todays_stats_for_all_services_includes_all_keys_by_default(notify_db, notify_db_session):
@@ -754,7 +757,9 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session
     results = fetch_stats_by_date_range_for_all_services(start_date, end_date)
 
     assert len(results) == 1
-    assert results[0] == ('sms', 'created', result_one.service_id, 2)
+    assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted,
+                          result_one.service.research_mode, result_one.service.active,
+                          result_one.service.created_at, 'sms', 'created', 2)
 
 
 @freeze_time('2001-01-01T23:59:00')
@@ -793,7 +798,9 @@ def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(n
     results = fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True)
 
     assert len(results) == 1
-    assert results[0] == ('sms', 'created', result_one.service_id, int(expected))
+    assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted,
+                          result_one.service.research_mode, result_one.service.active, result_one.service.created_at,
+                          'sms', 'created', int(expected))
 
 
 @pytest.mark.parametrize("start_delta, end_delta, expected",
@@ -820,7 +827,9 @@ def test_fetch_stats_by_date_range_during_bst_hour_for_all_services_returns_test
     results = fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True)
 
     assert len(results) == 1
-    assert results[0] == ('sms', 'created', result_one.service_id, int(expected))
+    assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted,
+                          result_one.service.research_mode, result_one.service.active, result_one.service.created_at,
+                          'sms', 'created', int(expected))
 
 
 @freeze_time('2001-01-01T23:59:00')