Merge pull request #540 from alphagov/dashboard-notifications-api

detailed service api endpoint

app/dao/services_dao.py

@@ -1,15 +1,13 @@
 import uuid
 
-from app import db
+from sqlalchemy import asc, func
-from app.models import Service
-from sqlalchemy import asc
 from sqlalchemy.orm import joinedload
 
+from app import db
 from app.dao.dao_utils import (
     transactional,
     version_class
 )
-
 from app.models import (
     NotificationStatistics,
     TemplateStatistics,
@@ -135,3 +133,16 @@ def delete_service_and_all_associated_db_objects(service):
     db.session.commit()
     list(map(db.session.delete, users))
     db.session.commit()
+
+
+def dao_fetch_stats_for_service(service_id):
+    return db.session.query(
+        Notification.notification_type,
+        Notification.status,
+        func.count(Notification.id).label('count')
+    ).filter(
+        Notification.service_id == service_id
+    ).group_by(
+        Notification.notification_type,
+        Notification.status,
+    ).all()

app/schemas.py

@@ -123,6 +123,23 @@ class ServiceSchema(BaseSchema):
             raise ValidationError('Only alphanumeric characters allowed')
 
 
+class DetailedServiceSchema(BaseSchema):
+    statistics = fields.Dict()
+
+    class Meta:
+        model = models.Service
+        exclude = (
+            'api_keys',
+            'templates',
+            'users',
+            'created_by',
+            'jobs',
+            'template_statistics',
+            'service_provider_stats',
+            'service_notification_stats'
+        )
+
+
 class NotificationModelSchema(BaseSchema):
     class Meta:
         model = models.Notification
@@ -486,6 +503,7 @@ user_schema = UserSchema()
 user_schema_load_json = UserSchema(load_json=True)
 service_schema = ServiceSchema()
 service_schema_load_json = ServiceSchema(load_json=True)
+detailed_service_schema = DetailedServiceSchema()
 template_schema = TemplateSchema()
 template_schema_load_json = TemplateSchema(load_json=True)
 api_key_schema = ApiKeySchema()

app/service/rest.py

@@ -8,20 +8,21 @@ from flask import (
 )
 from sqlalchemy.orm.exc import NoResultFound
 
+from app.models import EMAIL_TYPE, SMS_TYPE
 from app.dao.api_key_dao import (
     save_model_api_key,
     get_model_api_keys,
     get_unsigned_secret,
     expire_api_key)
 from app.dao.services_dao import (
-    dao_fetch_service_by_id_and_user,
     dao_fetch_service_by_id,
     dao_fetch_all_services,
     dao_create_service,
     dao_update_service,
     dao_fetch_all_services_by_user,
     dao_add_user_to_service,
-    dao_remove_user_from_service
+    dao_remove_user_from_service,
+    dao_fetch_stats_for_service
 )
 from app.dao import notifications_dao
 from app.dao.provider_statistics_dao import get_fragment_count
@@ -34,6 +35,7 @@ from app.schemas import (
     permission_schema,
     notification_status_schema,
     notifications_filter_schema,
+    detailed_service_schema
 )
 from app.utils import pagination_links
 from app.errors import (
@@ -58,14 +60,13 @@ def get_services():
 
 @service.route('/<uuid:service_id>', methods=['GET'])
 def get_service_by_id(service_id):
-    user_id = request.args.get('user_id', None)
+    if 'detailed' in request.args:
-    if user_id:
+        return get_detailed_service(service_id)
-        fetched = dao_fetch_service_by_id_and_user(service_id, user_id)
     else:
         fetched = dao_fetch_service_by_id(service_id)
 
-    data = service_schema.dump(fetched).data
+        data = service_schema.dump(fetched).data
-    return jsonify(data=data)
+        return jsonify(data=data)
 
 
 @service.route('', methods=['POST'])
@@ -232,3 +233,30 @@ def get_all_notifications_for_service(service_id):
             **kwargs
         )
     ), 200
+
+
+def get_detailed_service(service_id):
+    service = dao_fetch_service_by_id(service_id)
+    statistics = dao_fetch_stats_for_service(service_id)
+    service.statistics = format_statistics(statistics)
+    data = detailed_service_schema.dump(service).data
+    return jsonify(data=data)
+
+
+def format_statistics(statistics):
+    # statistics come in a named tuple with uniqueness from 'notification_type', 'status' - however missing
+    # statuses/notification types won't be represented and the status types need to be simplified/summed up
+    # so we can return emails/sms * created, sent, and failed
+    counts = {
+        template_type: {
+            status: 0 for status in ('requested', 'delivered', 'failed')
+        } for template_type in (EMAIL_TYPE, SMS_TYPE)
+    }
+    for row in statistics:
+        counts[row.notification_type]['requested'] += row.count
+        if row.status == 'delivered':
+            counts[row.notification_type]['delivered'] += row.count
+        elif row.status in ('failed', 'technical-failure', 'temporary-failure', 'permanent-failure'):
+            counts[row.notification_type]['failed'] += row.count
+
+    return counts

tests/app/conftest.py

@@ -3,7 +3,6 @@ import pytest
 import uuid
 from datetime import (datetime, date)
 
-import pytest
 from flask import current_app
 
 from app import db

tests/app/dao/test_services_dao.py

@@ -1,5 +1,10 @@
 import uuid
 import pytest
+
+from sqlalchemy.orm.exc import FlushError, NoResultFound
+from sqlalchemy.exc import IntegrityError
+
+from app import db
 from app.dao.services_dao import (
     dao_create_service,
     dao_add_user_to_service,
@@ -7,9 +12,9 @@ from app.dao.services_dao import (
     dao_fetch_all_services,
     dao_fetch_service_by_id,
     dao_fetch_all_services_by_user,
-    dao_fetch_service_by_id_and_user,
     dao_update_service,
-    delete_service_and_all_associated_db_objects
+    delete_service_and_all_associated_db_objects,
+    dao_fetch_stats_for_service
 )
 from app.dao.users_dao import save_model_user
 from app.models import (
@@ -21,13 +26,16 @@ from app.models import (
     Template,
     Job,
     Notification,
+    NotificationHistory,
     Permission,
     User,
     InvitedUser,
     Service
 )
-from sqlalchemy.orm.exc import FlushError, NoResultFound
+
-from sqlalchemy.exc import IntegrityError
+from tests.app.conftest import (
+    sample_notification as create_notification
+)
 
 
 def test_create_service(sample_user):
@@ -212,27 +220,6 @@ def test_get_service_by_id_returns_service(service_factory):
     assert dao_fetch_service_by_id(service.id).name == 'testing'
 
 
-def test_can_get_service_by_id_and_user(service_factory, sample_user):
-    service = service_factory.get('service 1', sample_user, email_from='service.1')
-    assert dao_fetch_service_by_id_and_user(service.id, sample_user.id).name == 'service 1'
-
-
-def test_cannot_get_service_by_id_and_owned_by_different_user(service_factory, sample_user):
-    service1 = service_factory.get('service 1', sample_user, email_from='service.1')
-    new_user = User(
-        name='Test User',
-        email_address='new_user@digital.cabinet-office.gov.uk',
-        password='password',
-        mobile_number='+447700900986'
-    )
-    save_model_user(new_user)
-    service2 = service_factory.get('service 2', new_user, email_from='service.2')
-    assert dao_fetch_service_by_id_and_user(service1.id, sample_user.id).name == 'service 1'
-    with pytest.raises(NoResultFound) as e:
-        dao_fetch_service_by_id_and_user(service2.id, sample_user.id)
-    assert 'No row was found for one()' in str(e)
-
-
 def test_create_service_creates_a_history_record_with_current_data(sample_user):
     assert Service.query.count() == 0
     assert Service.get_history_model().query.count() == 0
@@ -384,3 +371,52 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sam
 
     other_user_service_two_permissions = Permission.query.filter_by(service=service_two, user=other_user).all()
     assert len(other_user_service_two_permissions) == 8
+
+
+def test_fetch_stats_filters_on_service(sample_notification):
+    service_two = Service(name="service_two",
+                          created_by=sample_notification.service.created_by,
+                          email_from="hello",
+                          active=False,
+                          restricted=False,
+                          message_limit=1000)
+    dao_create_service(service_two, sample_notification.service.created_by)
+
+    stats = dao_fetch_stats_for_service(service_two.id)
+    assert len(stats) == 0
+
+
+def test_fetch_stats_ignores_historical_notification_data(sample_notification):
+    service_id = sample_notification.service.id
+
+    db.session.delete(sample_notification)
+
+    assert Notification.query.count() == 0
+    assert NotificationHistory.query.count() == 1
+
+    stats = dao_fetch_stats_for_service(service_id)
+    assert len(stats) == 0
+
+
+def test_fetch_stats_counts_correctly(notify_db, notify_db_session, sample_template, sample_email_template):
+    # two created email, one failed email, and one created sms
+    create_notification(notify_db, notify_db_session, template=sample_email_template, status='created')
+    create_notification(notify_db, notify_db_session, template=sample_email_template, status='created')
+    create_notification(notify_db, notify_db_session, template=sample_email_template, status='technical-failure')
+    create_notification(notify_db, notify_db_session, template=sample_template, status='created')
+
+    stats = dao_fetch_stats_for_service(sample_template.service.id)
+    stats = sorted(stats, key=lambda x: (x.notification_type, x.status))
+    assert len(stats) == 3
+
+    assert stats[0].notification_type == 'email'
+    assert stats[0].status == 'created'
+    assert stats[0].count == 2
+
+    assert stats[1].notification_type == 'email'
+    assert stats[1].status == 'technical-failure'
+    assert stats[1].count == 1
+
+    assert stats[2].notification_type == 'sms'
+    assert stats[2].status == 'created'
+    assert stats[2].count == 1

tests/app/service/test_rest.py

@@ -1,5 +1,8 @@
 import json
+import collections
 import uuid
+
+import pytest
 from flask import url_for
 
 from app.dao.users_dao import save_model_user
@@ -14,6 +17,9 @@ from tests.app.conftest import (
 )
 
 
+Row = collections.namedtuple('row', ('notification_type', 'status', 'count'))
+
+
 def test_get_service_list(notify_api, service_factory):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
@@ -1086,3 +1092,58 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl
             assert resp.status_code == 400
             assert result['result'] == 'error'
             assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']}
+
+
+def test_get_detailed_service(notify_api, sample_service, sample_notification):
+    with notify_api.test_request_context(), notify_api.test_client() as client:
+        resp = client.get(
+            '/service/{}?detailed=False'.format(sample_service.id),
+            headers=[create_authorization_header()]
+        )
+
+    assert resp.status_code == 200
+    service = json.loads(resp.get_data(as_text=True))['data']
+    assert service['id'] == str(sample_service.id)
+    assert 'statistics' in service.keys()
+    assert set(service['statistics'].keys()) == set(['sms', 'email'])
+    assert service['statistics']['sms'] == {
+        'requested': 1,
+        'delivered': 0,
+        'failed': 0
+    }
+
+
+# email_counts and sms_counts are 3-tuple of requested, delivered, failed
+@pytest.mark.idparametrize('stats, email_counts, sms_counts', {
+    'empty': ([], [0, 0, 0], [0, 0, 0]),
+    'always_increment_requested': ([
+        Row('email', 'delivered', 1),
+        Row('email', 'failed', 1)
+    ], [2, 1, 1], [0, 0, 0]),
+    'dont_mix_email_and_sms': ([
+        Row('email', 'delivered', 1),
+        Row('sms', 'delivered', 1)
+    ], [1, 1, 0], [1, 1, 0]),
+    'convert_fail_statuses_to_failed': ([
+        Row('email', 'failed', 1),
+        Row('email', 'technical-failure', 1),
+        Row('email', 'temporary-failure', 1),
+        Row('email', 'permanent-failure', 1),
+    ], [4, 0, 4], [0, 0, 0]),
+})
+def test_format_statistics(stats, email_counts, sms_counts):
+    from app.service.rest import format_statistics
+
+    ret = format_statistics(stats)
+
+    assert ret['email'] == {
+        status: count
+        for status, count
+        in zip(['requested', 'delivered', 'failed'], email_counts)
+    }
+
+    assert ret['sms'] == {
+        status: count
+        for status, count
+        in zip(['requested', 'delivered', 'failed'], sms_counts)
+    }

tests/conftest.py

@@ -68,3 +68,12 @@ def os_environ(request):
 def sqs_client_conn(request):
     boto3.setup_default_session(region_name='eu-west-1')
     return boto3.resource('sqs')
+
+
+def pytest_generate_tests(metafunc):
+    # Copied from https://gist.github.com/pfctdayelise/5719730
+    idparametrize = getattr(metafunc.function, 'idparametrize', None)
+    if idparametrize:
+        argnames, testdata = idparametrize.args
+        ids, argvalues = zip(*sorted(testdata.items()))
+        metafunc.parametrize(argnames, argvalues, ids=ids)