Merge pull request #2282 from alphagov/optimise-platform-admin-live-services

Optimise platform admin live services

app/dao/fact_notification_status_dao.py

@@ -8,7 +8,7 @@ from sqlalchemy.sql.expression import literal
 from sqlalchemy.types import DateTime, Integer
 
 from app import db
-from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST
+from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST, Service
 from app.utils import get_london_midnight_in_utc, midnight_n_days_ago
 
 
@@ -197,3 +197,97 @@ def fetch_notification_statuses_for_job(job_id):
     ).group_by(
         FactNotificationStatus.notification_status
     ).all()
+
+
+def fetch_stats_for_all_services_by_date_range(start_date, end_date, include_from_test_key=True):
+    stats = db.session.query(
+        FactNotificationStatus.service_id.label('service_id'),
+        Service.name.label('name'),
+        Service.restricted.label('restricted'),
+        Service.research_mode.label('research_mode'),
+        Service.active.label('active'),
+        Service.created_at.label('created_at'),
+        FactNotificationStatus.notification_type.label('notification_type'),
+        FactNotificationStatus.notification_status.label('status'),
+        func.sum(FactNotificationStatus.notification_count).label('count')
+    ).filter(
+        FactNotificationStatus.bst_date >= start_date,
+        FactNotificationStatus.bst_date <= end_date,
+        FactNotificationStatus.service_id == Service.id,
+    ).group_by(
+        FactNotificationStatus.service_id.label('service_id'),
+        Service.name,
+        Service.restricted,
+        Service.research_mode,
+        Service.active,
+        Service.created_at,
+        FactNotificationStatus.notification_type,
+        FactNotificationStatus.notification_status,
+    ).order_by(
+        FactNotificationStatus.service_id,
+        FactNotificationStatus.notification_type
+    )
+    if not include_from_test_key:
+        stats = stats.filter(FactNotificationStatus.key_type != KEY_TYPE_TEST)
+
+    if start_date <= datetime.utcnow().date() <= end_date:
+        today = get_london_midnight_in_utc(datetime.utcnow())
+        subquery = db.session.query(
+            Notification.notification_type.cast(db.Text).label('notification_type'),
+            Notification.status.label('status'),
+            Notification.service_id.label('service_id'),
+            func.count(Notification.id).label('count')
+        ).filter(
+            Notification.created_at >= today
+        ).group_by(
+            Notification.notification_type,
+            Notification.status,
+            Notification.service_id
+        )
+        if not include_from_test_key:
+            subquery = subquery.filter(FactNotificationStatus.key_type != KEY_TYPE_TEST)
+        subquery = subquery.subquery()
+
+        stats_for_today = db.session.query(
+            Service.id.label('service_id'),
+            Service.name.label('name'),
+            Service.restricted.label('restricted'),
+            Service.research_mode.label('research_mode'),
+            Service.active.label('active'),
+            Service.created_at.label('created_at'),
+            subquery.c.notification_type.label('notification_type'),
+            subquery.c.status.label('status'),
+            subquery.c.count.label('count')
+        ).outerjoin(
+            subquery,
+            subquery.c.service_id == Service.id
+        ).order_by(Service.id)
+
+        all_stats_table = stats.union_all(stats_for_today).subquery()
+        query = db.session.query(
+            all_stats_table.c.service_id,
+            all_stats_table.c.name,
+            all_stats_table.c.restricted,
+            all_stats_table.c.research_mode,
+            all_stats_table.c.active,
+            all_stats_table.c.created_at,
+            all_stats_table.c.notification_type,
+            all_stats_table.c.status,
+            func.cast(func.sum(all_stats_table.c.count), Integer).label('count'),
+        ).group_by(
+            all_stats_table.c.service_id,
+            all_stats_table.c.name,
+            all_stats_table.c.restricted,
+            all_stats_table.c.research_mode,
+            all_stats_table.c.active,
+            all_stats_table.c.created_at,
+            all_stats_table.c.notification_type,
+            all_stats_table.c.status,
+        ).order_by(
+            all_stats_table.c.name,
+            all_stats_table.c.notification_type,
+            all_stats_table.c.status
+        )
+    else:
+        query = stats
+    return query.all()

app/dao/services_dao.py

@@ -335,51 +335,6 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True, only_act
     return query.all()
 
 
-@statsd(namespace='dao')
-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(
-        table.notification_type,
-        table.status,
-        table.service_id,
-        func.count(table.id).label('count')
-    ).filter(
-        table.created_at >= start_date,
-        table.created_at < end_date
-    ).group_by(
-        table.notification_type,
-        table.status,
-        table.service_id
-    )
-    if not include_from_test_key:
-        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()
-
-
 @transactional
 @version_class(Service)
 @version_class(ApiKey)

app/service/rest.py

@@ -23,8 +23,8 @@ from app.dao.api_key_dao import (
 from app.dao.fact_notification_status_dao import (
     fetch_notification_status_for_service_by_month,
     fetch_notification_status_for_service_for_day,
-    fetch_notification_status_for_service_for_today_and_7_previous_days
-)
+    fetch_notification_status_for_service_for_today_and_7_previous_days,
+    fetch_stats_for_all_services_by_date_range)
 from app.dao.inbound_numbers_dao import dao_allocate_number_for_service
 from app.dao.organisation_dao import dao_get_organisation_by_service_id
 from app.dao.service_data_retention_dao import (
@@ -56,7 +56,6 @@ from app.dao.services_dao import (
     dao_remove_user_from_service,
     dao_suspend_service,
     dao_update_service,
-    fetch_stats_by_date_range_for_all_services
 )
 from app.dao.service_whitelist_dao import (
     dao_fetch_service_whitelist,
@@ -472,10 +471,10 @@ def get_detailed_services(start_date, end_date, only_active=False, include_from_
                                                         only_active=only_active)
     else:
 
-        stats = fetch_stats_by_date_range_for_all_services(start_date=start_date,
+        stats = fetch_stats_for_all_services_by_date_range(start_date=start_date,
                                                            end_date=end_date,
                                                            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):
         rows = list(rows)

tests/app/dao/test_fact_notification_status_dao.py

@@ -11,7 +11,7 @@ from app.dao.fact_notification_status_dao import (
     fetch_notification_status_for_service_for_today_and_7_previous_days,
     fetch_notification_status_totals_for_all_services,
     fetch_notification_statuses_for_job,
-)
+    fetch_stats_for_all_services_by_date_range)
 from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
 from freezegun import freeze_time
 from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job
@@ -289,6 +289,7 @@ def set_up_data():
     create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))
     create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered')
     create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered')
+    return service_1, service_2
 
 
 def test_fetch_notification_statuses_for_job(sample_template):
@@ -304,3 +305,36 @@ def test_fetch_notification_statuses_for_job(sample_template):
         'created': 5,
         'delivered': 2
     }
+
+
+@freeze_time('2018-10-31 14:00')
+def test_fetch_stats_for_all_services_by_date_range(notify_db_session):
+    service_1, service_2 = set_up_data()
+    results = fetch_stats_for_all_services_by_date_range(start_date=date(2018, 10, 29),
+                                                         end_date=date(2018, 10, 31))
+    assert len(results) == 5
+
+    assert results[0].service_id == service_1.id
+    assert results[0].notification_type == 'email'
+    assert results[0].status == 'delivered'
+    assert results[0].count == 4
+
+    assert results[1].service_id == service_1.id
+    assert results[1].notification_type == 'sms'
+    assert results[1].status == 'created'
+    assert results[1].count == 2
+
+    assert results[2].service_id == service_1.id
+    assert results[2].notification_type == 'sms'
+    assert results[2].status == 'delivered'
+    assert results[2].count == 11
+
+    assert results[3].service_id == service_2.id
+    assert results[3].notification_type == 'letter'
+    assert results[3].status == 'delivered'
+    assert results[3].count == 10
+
+    assert results[4].service_id == service_2.id
+    assert not results[4].notification_type
+    assert not results[4].status
+    assert not results[4].count

tests/app/dao/test_services_dao.py

@@ -27,7 +27,6 @@ from app.dao.services_dao import (
     dao_fetch_todays_stats_for_service,
     fetch_todays_total_message_count,
     dao_fetch_todays_stats_for_all_services,
-    fetch_stats_by_date_range_for_all_services,
     dao_suspend_service,
     dao_resume_service,
     dao_fetch_active_users_for_service,
@@ -775,25 +774,6 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key(notif
     assert stats[0].count == 2
 
 
-def test_fetch_stats_by_date_range_for_all_services(notify_db_session):
-    template = create_template(service=create_service())
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=4))
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=3))
-    result_one = create_notification(template=template, created_at=datetime.now() - timedelta(days=2))
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=1))
-    create_notification(template=template, created_at=datetime.now())
-
-    start_date = (datetime.utcnow() - timedelta(days=2)).date()
-    end_date = (datetime.utcnow() - timedelta(days=1)).date()
-
-    results = fetch_stats_by_date_range_for_all_services(start_date, end_date)
-
-    assert len(results) == 1
-    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')
 def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(notify_db_session):
     service = create_service()
@@ -807,64 +787,6 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(noti
     assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00)
 
 
-@pytest.mark.parametrize("start_delta, end_delta, expected",
-                         [("5", "1", "4"),  # a date range less than 7 days ago returns test and normal notifications
-                          ("9", "8", "1"),  # a date range older than 9 days does not return test notifications.
-                          ("8", "4", "2")])  # a date range that starts more than 7 days ago
-@freeze_time('2017-10-23T00:00:00')
-def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(notify_db_session,
-                                                                               start_delta,
-                                                                               end_delta,
-                                                                               expected):
-    template = create_template(service=create_service())
-    result_one = create_notification(template=template, created_at=datetime.now(), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=2), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=3), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='normal')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=8), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=8), key_type='normal')
-
-    start_date = (datetime.utcnow() - timedelta(days=int(start_delta))).date()
-    end_date = (datetime.utcnow() - timedelta(days=int(end_delta))).date()
-
-    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] == (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",
-                         [("5", "1", "4"),  # a date range less than 7 days ago returns test and normal notifications
-                          ("9", "8", "1"),  # a date range older than 9 days does not return test notifications.
-                          ("8", "4", "2")])  # a date range that starts more than 7 days ago
-@freeze_time('2017-10-23T23:00:00')
-def test_fetch_stats_by_date_range_during_bst_hour_for_all_services_returns_test_notifications(
-        notify_db_session, start_delta, end_delta, expected
-):
-    template = create_template(service=create_service())
-    result_one = create_notification(template=template, created_at=datetime.now(), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=2), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=3), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='normal')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='test')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=8), key_type='normal')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=9), key_type='normal')
-    create_notification(template=template, created_at=datetime.now() - timedelta(days=9), key_type='test')
-
-    start_date = (datetime.utcnow() - timedelta(days=int(start_delta))).date()
-    end_date = (datetime.utcnow() - timedelta(days=int(end_delta))).date()
-
-    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] == (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')
 def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revoked(notify_db_session):
     service = create_service()

tests/app/service/test_rest.py

@@ -1642,31 +1642,37 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not
     }
 
 
-@pytest.mark.parametrize(
-    'set_time',
-    ['2017-03-28T12:00:00', '2017-01-28T12:00:00', '2017-01-02T12:00:00', '2017-10-31T12:00:00']
-)
-def test_get_detailed_services_for_date_range(notify_db, notify_db_session, set_time):
+@pytest.mark.parametrize("start_date_delta, end_date_delta",
+                         [(2, 1),
+                          (3, 2),
+                          (1, 0)
+                          ])
+@freeze_time('2017-03-28T12:00:00')
+def test_get_detailed_services_for_date_range(sample_template, start_date_delta, end_date_delta):
     from app.service.rest import get_detailed_services
 
-    with freeze_time(set_time):
-        create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow() - timedelta(days=3))
-        create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow() - timedelta(days=2))
-        create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow() - timedelta(days=1))
-        create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow())
+    create_ft_notification_status(bst_date=(datetime.utcnow() - timedelta(days=3)).date(),
+                                  service=sample_template.service,
+                                  notification_type='sms')
+    create_ft_notification_status(bst_date=(datetime.utcnow() - timedelta(days=2)).date(),
+                                  service=sample_template.service,
+                                  notification_type='sms')
+    create_ft_notification_status(bst_date=(datetime.utcnow() - timedelta(days=1)).date(),
+                                  service=sample_template.service,
+                                  notification_type='sms')
 
-        start_date = (datetime.utcnow() - timedelta(days=2)).date()
-        end_date = (datetime.utcnow() - timedelta(days=1)).date()
+    create_notification(template=sample_template, created_at=datetime.utcnow(), status='delivered')
+
+    start_date = (datetime.utcnow() - timedelta(days=start_date_delta)).date()
+    end_date = (datetime.utcnow() - timedelta(days=end_date_delta)).date()
 
     data = get_detailed_services(only_active=False, include_from_test_key=True,
                                  start_date=start_date, end_date=end_date)
 
     assert len(data) == 1
-    assert data[0]['statistics'] == {
-        EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0},
-        SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 2},
-        LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}
-    }
+    assert data[0]['statistics'][EMAIL_TYPE] == {'delivered': 0, 'failed': 0, 'requested': 0}
+    assert data[0]['statistics'][SMS_TYPE] == {'delivered': 2, 'failed': 0, 'requested': 2}
+    assert data[0]['statistics'][LETTER_TYPE] == {'delivered': 0, 'failed': 0, 'requested': 0}
 
 
 def test_search_for_notification_by_to_field(client, sample_template, sample_email_template):