diff --git a/app/__init__.py b/app/__init__.py index e0928a4b9..464157f60 100644 --- a/app/__init__.py +++ b/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 diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index eff4679dd..031c10939 100644 --- a/app/dao/services_dao.py +++ b/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 + ) diff --git a/app/service/rest.py b/app/service/rest.py index 1c0096000..5a80cdc76 100644 --- a/app/service/rest.py +++ b/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__) -register_errors(service) +service_blueprint = Blueprint('service', __name__) +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('/', methods=['GET']) +@service_blueprint.route('/', 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('/', methods=['POST']) +@service_blueprint.route('/', 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('//api-key', methods=['POST']) +@service_blueprint.route('//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('//api-key/revoke/', methods=['POST']) +@service_blueprint.route('//api-key/revoke/', 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('//api-keys', methods=['GET']) -@service.route('//api-keys/', methods=['GET']) +@service_blueprint.route('//api-keys', methods=['GET']) +@service_blueprint.route('//api-keys/', 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('//users', methods=['GET']) +@service_blueprint.route('//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('//users/', methods=['POST']) +@service_blueprint.route('//users/', 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('//users/', methods=['DELETE']) +@service_blueprint.route('//users/', 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('//fragment/aggregate_statistics') +@service_blueprint.route('//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('//history', methods=['GET']) +@service_blueprint.route('//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('//notifications', methods=['GET']) +@service_blueprint.route('//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('//notifications/weekly', methods=['GET']) +@service_blueprint.route('//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 jsonify(data=data) + return detailed_service_schema.dump(service).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 diff --git a/app/service/statistics.py b/app/service/statistics.py index abcb25740..3f06fcacb 100644 --- a/app/service/statistics.py +++ b/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') diff --git a/requirements_for_test.txt b/requirements_for_test.txt index f5db40330..1ea6f7a8d 100644 --- a/requirements_for_test.txt +++ b/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 diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index df611adca..778bbe66e 100644 --- a/tests/app/notifications/test_rest.py +++ b/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]) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1d3612b01..ba6587a61 100644 --- a/tests/app/service/test_rest.py +++ b/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} + } diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index a4d8648e0..fcf854da8 100644 --- a/tests/app/service/test_statistics.py +++ b/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}, }