Merge branch 'master' into remove-contested-writes

app/__init__.py

@@ -58,7 +58,7 @@ def create_app(app_name=None):
     encryption.init_app(application)
     clients.init_app(sms_clients=[firetext_client, mmg_client, loadtest_client], email_clients=[aws_ses_client])
 
-    from app.service.rest import service as service_blueprint
+    from app.service.rest import service_blueprint
     from app.user.rest import user as user_blueprint
     from app.template.rest import template as template_blueprint
     from app.status.healthcheck import status as status_blueprint

app/dao/services_dao.py

@@ -180,3 +180,25 @@ def dao_fetch_weekly_historical_stats_for_service(service_id):
     ).order_by(
         asc(monday_of_notification_week), NotificationHistory.status
     ).all()
+
+
+@statsd(namespace='dao')
+def dao_fetch_todays_stats_for_all_services():
+    return db.session.query(
+        Notification.notification_type,
+        Notification.status,
+        Notification.service_id,
+        func.count(Notification.id).label('count')
+    ).select_from(
+        Service
+    ).join(
+        Notification
+    ).filter(
+        func.date(Notification.created_at) == date.today()
+    ).group_by(
+        Notification.notification_type,
+        Notification.status,
+        Notification.service_id
+    ).order_by(
+        Notification.service_id
+    )

app/service/rest.py

@@ -1,3 +1,5 @@
+import itertools
+
 from flask import (
     jsonify,
     request,
@@ -21,7 +23,8 @@ from app.dao.services_dao import (
     dao_remove_user_from_service,
     dao_fetch_stats_for_service,
     dao_fetch_todays_stats_for_service,
-    dao_fetch_weekly_historical_stats_for_service
+    dao_fetch_weekly_historical_stats_for_service,
+    dao_fetch_todays_stats_for_all_services
 )
 from app.dao import notifications_dao
 from app.dao.provider_statistics_dao import get_fragment_count
@@ -42,25 +45,28 @@ from app.errors import (
 )
 from app.service import statistics
 
-service = Blueprint('service', __name__)
+service_blueprint = Blueprint('service', __name__)
-register_errors(service)
+register_errors(service_blueprint)
 
 
-@service.route('', methods=['GET'])
+@service_blueprint.route('', methods=['GET'])
 def get_services():
     user_id = request.args.get('user_id', None)
     if user_id:
         services = dao_fetch_all_services_by_user(user_id)
+    elif request.args.get('detailed') == 'True':
+        return jsonify(data=get_detailed_services())
     else:
         services = dao_fetch_all_services()
     data = service_schema.dump(services, many=True).data
     return jsonify(data=data)
 
 
-@service.route('/<uuid:service_id>', methods=['GET'])
+@service_blueprint.route('/<uuid:service_id>', methods=['GET'])
 def get_service_by_id(service_id):
     if request.args.get('detailed') == 'True':
-        return get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True')
+        data = get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True')
+        return jsonify(data=data)
     else:
         fetched = dao_fetch_service_by_id(service_id)
 
@@ -68,7 +74,7 @@ def get_service_by_id(service_id):
         return jsonify(data=data)
 
 
-@service.route('', methods=['POST'])
+@service_blueprint.route('', methods=['POST'])
 def create_service():
     data = request.get_json()
     if not data.get('user_id', None):
@@ -82,7 +88,7 @@ def create_service():
     return jsonify(data=service_schema.dump(valid_service).data), 201
 
 
-@service.route('/<uuid:service_id>', methods=['POST'])
+@service_blueprint.route('/<uuid:service_id>', methods=['POST'])
 def update_service(service_id):
     fetched_service = dao_fetch_service_by_id(service_id)
     current_data = dict(service_schema.dump(fetched_service).data.items())
@@ -92,7 +98,7 @@ def update_service(service_id):
     return jsonify(data=service_schema.dump(fetched_service).data), 200
 
 
-@service.route('/<uuid:service_id>/api-key', methods=['POST'])
+@service_blueprint.route('/<uuid:service_id>/api-key', methods=['POST'])
 def create_api_key(service_id=None):
     fetched_service = dao_fetch_service_by_id(service_id=service_id)
     valid_api_key = api_key_schema.load(request.get_json()).data
@@ -102,14 +108,14 @@ def create_api_key(service_id=None):
     return jsonify(data=unsigned_api_key), 201
 
 
-@service.route('/<uuid:service_id>/api-key/revoke/<uuid:api_key_id>', methods=['POST'])
+@service_blueprint.route('/<uuid:service_id>/api-key/revoke/<uuid:api_key_id>', methods=['POST'])
 def revoke_api_key(service_id, api_key_id):
     expire_api_key(service_id=service_id, api_key_id=api_key_id)
     return jsonify(), 202
 
 
-@service.route('/<uuid:service_id>/api-keys', methods=['GET'])
+@service_blueprint.route('/<uuid:service_id>/api-keys', methods=['GET'])
-@service.route('/<uuid:service_id>/api-keys/<uuid:key_id>', methods=['GET'])
+@service_blueprint.route('/<uuid:service_id>/api-keys/<uuid:key_id>', methods=['GET'])
 def get_api_keys(service_id, key_id=None):
     dao_fetch_service_by_id(service_id=service_id)
 
@@ -125,14 +131,14 @@ def get_api_keys(service_id, key_id=None):
     return jsonify(apiKeys=api_key_schema.dump(api_keys, many=True).data), 200
 
 
-@service.route('/<uuid:service_id>/users', methods=['GET'])
+@service_blueprint.route('/<uuid:service_id>/users', methods=['GET'])
 def get_users_for_service(service_id):
     fetched = dao_fetch_service_by_id(service_id)
     result = user_schema.dump(fetched.users, many=True)
     return jsonify(data=result.data)
 
 
-@service.route('/<uuid:service_id>/users/<user_id>', methods=['POST'])
+@service_blueprint.route('/<uuid:service_id>/users/<user_id>', methods=['POST'])
 def add_user_to_service(service_id, user_id):
     service = dao_fetch_service_by_id(service_id)
     user = get_model_users(user_id=user_id)
@@ -147,7 +153,7 @@ def add_user_to_service(service_id, user_id):
     return jsonify(data=data), 201
 
 
-@service.route('/<uuid:service_id>/users/<user_id>', methods=['DELETE'])
+@service_blueprint.route('/<uuid:service_id>/users/<user_id>', methods=['DELETE'])
 def remove_user_from_service(service_id, user_id):
     service = dao_fetch_service_by_id(service_id)
     user = get_model_users(user_id=user_id)
@@ -163,7 +169,7 @@ def remove_user_from_service(service_id, user_id):
     return jsonify({}), 204
 
 
-@service.route('/<uuid:service_id>/fragment/aggregate_statistics')
+@service_blueprint.route('/<uuid:service_id>/fragment/aggregate_statistics')
 def get_service_provider_aggregate_statistics(service_id):
     return jsonify(data=get_fragment_count(service_id))
 
@@ -171,7 +177,7 @@ def get_service_provider_aggregate_statistics(service_id):
 # This is placeholder get method until more thought
 # goes into how we want to fetch and view various items in history
 # tables. This is so product owner can pass stories as done
-@service.route('/<uuid:service_id>/history', methods=['GET'])
+@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.schemas import (
@@ -201,7 +207,7 @@ def get_service_history(service_id):
     return jsonify(data=data)
 
 
-@service.route('/<uuid:service_id>/notifications', methods=['GET'])
+@service_blueprint.route('/<uuid:service_id>/notifications', methods=['GET'])
 def get_all_notifications_for_service(service_id):
     data = notifications_filter_schema.load(request.args).data
     page = data['page'] if 'page' in data else 1
@@ -228,7 +234,7 @@ def get_all_notifications_for_service(service_id):
     ), 200
 
 
-@service.route('/<uuid:service_id>/notifications/weekly', methods=['GET'])
+@service_blueprint.route('/<uuid:service_id>/notifications/weekly', methods=['GET'])
 def get_weekly_notification_stats(service_id):
     service = dao_fetch_service_by_id(service_id)
     stats = dao_fetch_weekly_historical_stats_for_service(service_id)
@@ -243,5 +249,19 @@ def get_detailed_service(service_id, today_only=False):
 
     service.statistics = statistics.format_statistics(stats)
 
-    data = detailed_service_schema.dump(service).data
+    return detailed_service_schema.dump(service).data
-    return jsonify(data=data)
+
+
+def get_detailed_services():
+    services = {service.id: service for service in dao_fetch_all_services()}
+    stats = dao_fetch_todays_stats_for_all_services()
+
+    for service_id, rows in itertools.groupby(stats, lambda x: x.service_id):
+        services[service_id].statistics = statistics.format_statistics(rows)
+
+    # if service has not sent anything, query will not have set statistics correctly
+    for service in services.values():
+        if not hasattr(service, 'statistics'):
+            service.statistics = statistics.create_zeroed_stats_dicts()
+
+    return detailed_service_schema.dump(services.values(), many=True).data

app/service/statistics.py

@@ -8,7 +8,7 @@ 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 = _create_zeroed_stats_dicts()
+    counts = create_zeroed_stats_dicts()
     for row in statistics:
         _update_statuses_from_row(counts[row.notification_type], row)
 
@@ -20,7 +20,7 @@ def format_weekly_notification_stats(statistics, service_created_at):
     # turn a datetime into midnight that day http://stackoverflow.com/a/1937636
     preceeding_monday_midnight = datetime.combine(preceeding_monday.date(), datetime.min.time())
     week_dict = {
-        week: _create_zeroed_stats_dicts()
+        week: create_zeroed_stats_dicts()
         for week in _weeks_for_range(preceeding_monday_midnight, datetime.utcnow())
     }
     for row in statistics:
@@ -29,7 +29,7 @@ def format_weekly_notification_stats(statistics, service_created_at):
     return week_dict
 
 
-def _create_zeroed_stats_dicts():
+def create_zeroed_stats_dicts():
     return {
         template_type: {
             status: 0 for status in ('requested', 'delivered', 'failed')

requirements_for_test.txt

@@ -6,6 +6,6 @@ pytest-cov==2.2.0
 coveralls==1.1
 mock==1.0.1
 moto==0.4.19
-flex==5.6.0
+flex==5.7.0
 freezegun==0.3.6
 requests-mock==0.7.0

tests/app/notifications/test_rest.py

@@ -207,7 +207,7 @@ def test_normal_api_key_returns_notifications_created_from_jobs_and_from_api(
 
         notifications = json.loads(response.get_data(as_text=True))['notifications']
         assert len(notifications) == 2
-        assert set(x['id'] for x in notifications) == set([str(sample_notification.id), str(api_notification.id)])
+        assert set(x['id'] for x in notifications) == {str(sample_notification.id), str(api_notification.id)}
 
 
 @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST])

tests/app/service/test_rest.py

@@ -1,3 +1,4 @@
+from datetime import datetime
 import json
 import uuid
 
@@ -263,7 +264,7 @@ def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(no
             json_resp = json.loads(resp.get_data(as_text=True))
             assert resp.status_code == 404
             assert json_resp['result'] == 'error'
-            assert 'No result found' == json_resp['message']
+            assert json_resp['message'] == 'No result found'
 
 
 def test_should_not_create_service_if_missing_data(notify_api, sample_user):
@@ -288,8 +289,6 @@ def test_should_not_create_service_if_missing_data(notify_api, sample_user):
 
 
 def test_should_not_create_service_with_duplicate_name(notify_api,
-                                                       notify_db,
-                                                       notify_db_session,
                                                        sample_user,
                                                        sample_service):
     with notify_api.test_request_context():
@@ -314,8 +313,6 @@ def test_should_not_create_service_with_duplicate_name(notify_api,
 
 
 def test_create_service_should_throw_duplicate_key_constraint_for_existing_email_from(notify_api,
-                                                                                      notify_db,
-                                                                                      notify_db_session,
                                                                                       service_factory,
                                                                                       sample_user):
     first_service = service_factory.get('First service', email_from='first.service')
@@ -432,7 +429,7 @@ def test_update_service_research_mode_throws_validation_error(notify_api, sample
                 headers=[('Content-Type', 'application/json'), auth_header]
             )
             result = json.loads(resp.get_data(as_text=True))
-            result['message']['research_mode'][0] == "Not a valid boolean."
+            assert result['message']['research_mode'][0] == "Not a valid boolean."
             assert resp.status_code == 400
 
 
@@ -505,7 +502,7 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api,
             )
 
 
-def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session):
+def test_update_service_should_404_if_id_is_invalid(notify_api):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             data = {
@@ -524,7 +521,7 @@ def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notif
             assert resp.status_code == 404
 
 
-def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_service):
+def test_get_users_by_service(notify_api, sample_service):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             user_on_service = sample_service.users[0]
@@ -544,8 +541,6 @@ def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_s
 
 
 def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_service(notify_api,
-                                                                                      notify_db,
-                                                                                      notify_db_session,
                                                                                       sample_service):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
@@ -1110,8 +1105,7 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl
         'delivered': 0,
         'failed': 0
     })
-    ], ids=['seven_days', 'today']
+], ids=['seven_days', 'today'])
-)
 def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_service, today_only, stats):
     with notify_api.test_request_context(), notify_api.test_client() as client:
         with freeze_time('2000-01-01T12:00:00'):
@@ -1156,3 +1150,94 @@ def test_get_weekly_notification_stats(notify_api, notify_db, notify_db_session)
             }
         }
     }
+
+
+def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_session):
+    notifications = [
+        create_sample_notification(notify_db, notify_db_session),
+        create_sample_notification(notify_db, notify_db_session)
+    ]
+    with notify_api.test_request_context(), notify_api.test_client() as client:
+        resp = client.get(
+            '/service?detailed=True',
+            headers=[create_authorization_header()]
+        )
+
+    assert resp.status_code == 200
+    data = json.loads(resp.get_data(as_text=True))['data']
+    assert len(data) == 1
+    assert data[0]['name'] == 'Sample service'
+    assert data[0]['id'] == str(notifications[0].service_id)
+    assert data[0]['statistics'] == {
+        'email': {'delivered': 0, 'failed': 0, 'requested': 0},
+        'sms': {'delivered': 0, 'failed': 0, 'requested': 2}
+    }
+
+
+def test_get_detailed_services_groups_by_service(notify_db, notify_db_session):
+    from app.service.rest import get_detailed_services
+
+    service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1')
+    service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2')
+
+    create_sample_notification(notify_db, notify_db_session, service=service_1, status='created')
+    create_sample_notification(notify_db, notify_db_session, service=service_2, status='created')
+    create_sample_notification(notify_db, notify_db_session, service=service_1, status='delivered')
+    create_sample_notification(notify_db, notify_db_session, service=service_1, status='created')
+
+    data = get_detailed_services()
+    data = sorted(data, key=lambda x: x['name'])
+
+    assert len(data) == 2
+    assert data[0]['id'] == str(service_1.id)
+    assert data[0]['statistics'] == {
+        'email': {'delivered': 0, 'failed': 0, 'requested': 0},
+        'sms': {'delivered': 1, 'failed': 0, 'requested': 3}
+    }
+    assert data[1]['id'] == str(service_2.id)
+    assert data[1]['statistics'] == {
+        'email': {'delivered': 0, 'failed': 0, 'requested': 0},
+        'sms': {'delivered': 0, 'failed': 0, 'requested': 1}
+    }
+
+
+def test_get_detailed_services_includes_services_with_no_notifications(notify_db, notify_db_session):
+    from app.service.rest import get_detailed_services
+
+    service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1')
+    service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2')
+
+    create_sample_notification(notify_db, notify_db_session, service=service_1)
+
+    data = get_detailed_services()
+    data = sorted(data, key=lambda x: x['name'])
+
+    assert len(data) == 2
+    assert data[0]['id'] == str(service_1.id)
+    assert data[0]['statistics'] == {
+        'email': {'delivered': 0, 'failed': 0, 'requested': 0},
+        'sms': {'delivered': 0, 'failed': 0, 'requested': 1}
+    }
+    assert data[1]['id'] == str(service_2.id)
+    assert data[1]['statistics'] == {
+        'email': {'delivered': 0, 'failed': 0, 'requested': 0},
+        'sms': {'delivered': 0, 'failed': 0, 'requested': 0}
+    }
+
+
+def test_get_detailed_services_only_includes_todays_notifications(notify_db, notify_db_session):
+    from app.service.rest import get_detailed_services
+
+    create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 9, 23, 59))
+    create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 0, 0))
+    create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 12, 0))
+
+    with freeze_time('2015-10-10T12:00:00'):
+        data = get_detailed_services()
+        data = sorted(data, key=lambda x: x['id'])
+
+    assert len(data) == 1
+    assert data[0]['statistics'] == {
+        'email': {'delivered': 0, 'failed': 0, 'requested': 0},
+        'sms': {'delivered': 0, 'failed': 0, 'requested': 2}
+    }

tests/app/service/test_statistics.py

@@ -7,7 +7,7 @@ from freezegun import freeze_time
 from app.service.statistics import (
     format_statistics,
     _weeks_for_range,
-    _create_zeroed_stats_dicts,
+    create_zeroed_stats_dicts,
     format_weekly_notification_stats
 )
 
@@ -63,7 +63,7 @@ def test_weeks_for_range(start, end, dates):
 
 
 def test_create_zeroed_stats_dicts():
-    assert _create_zeroed_stats_dicts() == {
+    assert create_zeroed_stats_dicts() == {
         'sms': {'requested': 0, 'delivered': 0, 'failed': 0},
         'email': {'requested': 0, 'delivered': 0, 'failed': 0},
     }