From ebb13a12519d4f6ef185a72635f8b388fb4d5034 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 17:24:44 +0100 Subject: [PATCH 1/5] create initial stats query get statistics for all services, for today only --- app/dao/services_dao.py | 20 ++++++++++++++++++++ app/service/rest.py | 15 +++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index eff4679dd..5015ba363 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -180,3 +180,23 @@ 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, + func.count(Notification.id).label('count') + ).select_from( + Service + ).join( + # don't want to create a relationship in case we accidentally lazily load it, so manually define the join term + Notification + ).filter( + func.date(Notification.created_at) == date.today() + ).group_by( + Notification.notification_type, + Notification.status, + Notification.service_id + ) diff --git a/app/service/rest.py b/app/service/rest.py index 1c0096000..6445a8311 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -52,6 +52,8 @@ def get_services(): if user_id: services = dao_fetch_all_services_by_user(user_id) else: + if request.args.get('detailed') == 'True': + return get_detailed_services() services = dao_fetch_all_services() data = service_schema.dump(services, many=True).data return jsonify(data=data) @@ -245,3 +247,16 @@ def get_detailed_service(service_id, today_only=False): data = 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(service_id) + + for row in stats: + services[row.service_id].statistics + # todo: how do we separate rows of statistics by service? + service.statistics = statistics.format_statistics(stats) + + data = detailed_service_schema.dump(service).data + return jsonify(data=data) From 00d19f63f05e53419d9c9099c3eacb5de0759bf3 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 19 Aug 2016 16:36:20 +0100 Subject: [PATCH 2/5] group results by service using itertools allows us to nicely reuse the existing format_statistics function --- app/dao/services_dao.py | 3 +++ app/service/rest.py | 19 ++++++++++--------- tests/app/notifications/test_rest.py | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5015ba363..c1d2c4672 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -187,6 +187,7 @@ 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 @@ -199,4 +200,6 @@ def dao_fetch_todays_stats_for_all_services(): 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 6445a8311..149e2ca14 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 @@ -51,9 +54,9 @@ 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 get_detailed_services() else: - if request.args.get('detailed') == 'True': - return get_detailed_services() services = dao_fetch_all_services() data = service_schema.dump(services, many=True).data return jsonify(data=data) @@ -251,12 +254,10 @@ def get_detailed_service(service_id, today_only=False): def get_detailed_services(): services = {service.id: service for service in dao_fetch_all_services()} - stats = dao_fetch_todays_stats_for_all_services(service_id) + stats = dao_fetch_todays_stats_for_all_services() - for row in stats: - services[row.service_id].statistics - # todo: how do we separate rows of statistics by service? - service.statistics = statistics.format_statistics(stats) + for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): + services[service_id].statistics = statistics.format_statistics(rows) - data = detailed_service_schema.dump(service).data + data = detailed_service_schema.dump(service, many=True).data return jsonify(data=data) 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]) From 2053ebd933cd5fe7cb3d92ca6ada04f71f908384 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 19 Aug 2016 17:30:25 +0100 Subject: [PATCH 3/5] rename service to service_blueprint it was causing a bug where a local variable service was not being instantiated and we were trying to operate on the blueprint instead it's being used in so few places it makes sense to rename it --- app/__init__.py | 2 +- app/service/rest.py | 36 ++++++++++++++++++------------------ 2 files changed, 19 insertions(+), 19 deletions(-) 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/service/rest.py b/app/service/rest.py index 149e2ca14..14ee99616 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -45,11 +45,11 @@ 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: @@ -62,7 +62,7 @@ def get_services(): 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') @@ -73,7 +73,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): @@ -87,7 +87,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()) @@ -97,7 +97,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 @@ -107,14 +107,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) @@ -130,14 +130,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) @@ -152,7 +152,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) @@ -168,7 +168,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)) @@ -176,7 +176,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 ( @@ -206,7 +206,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 @@ -233,7 +233,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) @@ -259,5 +259,5 @@ def get_detailed_services(): for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) - data = detailed_service_schema.dump(service, many=True).data + data = detailed_service_schema.dump(services, many=True).data return jsonify(data=data) From 29df7edaf90ccc1ba6fb6aa52214ff931dd2cce1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 19 Aug 2016 17:36:31 +0100 Subject: [PATCH 4/5] tests for detailed services --- app/dao/services_dao.py | 1 - app/service/rest.py | 11 ++- requirements_for_test.txt | 2 +- tests/app/service/test_rest.py | 125 +++++++++++++++++++++++++++------ 4 files changed, 111 insertions(+), 28 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index c1d2c4672..031c10939 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -192,7 +192,6 @@ def dao_fetch_todays_stats_for_all_services(): ).select_from( Service ).join( - # don't want to create a relationship in case we accidentally lazily load it, so manually define the join term Notification ).filter( func.date(Notification.created_at) == date.today() diff --git a/app/service/rest.py b/app/service/rest.py index 14ee99616..1ba277957 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -55,7 +55,7 @@ def get_services(): if user_id: services = dao_fetch_all_services_by_user(user_id) elif request.args.get('detailed') == 'True': - return get_detailed_services() + return jsonify(data=get_detailed_services()) else: services = dao_fetch_all_services() data = service_schema.dump(services, many=True).data @@ -65,7 +65,8 @@ def get_services(): @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) @@ -248,8 +249,7 @@ 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(): @@ -259,5 +259,4 @@ def get_detailed_services(): for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) - data = detailed_service_schema.dump(services, many=True).data - return jsonify(data=data) + return detailed_service_schema.dump(services.values(), many=True).data 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/service/test_rest.py b/tests/app/service/test_rest.py index 1d3612b01..4a64e86ac 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 @@ -15,6 +16,7 @@ from tests.app.conftest import ( sample_user as create_sample_user, sample_notification as create_sample_notification ) +from app.service.rest import get_detailed_services def test_get_service_list(notify_api, service_factory): @@ -263,7 +265,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 +290,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 +314,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 +430,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 +503,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 +522,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 +542,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: @@ -1100,16 +1096,16 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl @pytest.mark.parametrize('today_only,stats', [ - ('False', { - 'requested': 2, - 'delivered': 1, - 'failed': 0 - }), - ('True', { - 'requested': 1, - 'delivered': 0, - 'failed': 0 - }) + ('False', { + 'requested': 2, + 'delivered': 1, + 'failed': 0 + }), + ('True', { + 'requested': 1, + 'delivered': 0, + 'failed': 0 + }) ], ids=['seven_days', 'today'] ) def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_service, today_only, stats): @@ -1156,3 +1152,92 @@ 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): + 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['id']) + + 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': 1, 'failed': 0, 'requested': 2} + } + + +def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): + 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_only_includes_todays_notifications(notify_db, notify_db_session): + 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} + } From 556b69a487536c123abbaa26dc15218d1b2129ed Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 23 Aug 2016 17:08:53 +0100 Subject: [PATCH 5/5] still return service if they have never sent any notifications --- app/service/rest.py | 5 ++ app/service/statistics.py | 6 +-- tests/app/service/test_rest.py | 74 ++++++++++++++-------------- tests/app/service/test_statistics.py | 4 +- 4 files changed, 47 insertions(+), 42 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 1ba277957..5a80cdc76 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -259,4 +259,9 @@ def get_detailed_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/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4a64e86ac..ba6587a61 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -16,7 +16,6 @@ from tests.app.conftest import ( sample_user as create_sample_user, sample_notification as create_sample_notification ) -from app.service.rest import get_detailed_services def test_get_service_list(notify_api, service_factory): @@ -1096,18 +1095,17 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl @pytest.mark.parametrize('today_only,stats', [ - ('False', { - 'requested': 2, - 'delivered': 1, - 'failed': 0 - }), - ('True', { - 'requested': 1, - 'delivered': 0, - 'failed': 0 - }) - ], ids=['seven_days', 'today'] -) + ('False', { + 'requested': 2, + 'delivered': 1, + 'failed': 0 + }), + ('True', { + 'requested': 1, + 'delivered': 0, + 'failed': 0 + }) +], 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'): @@ -1177,31 +1175,8 @@ def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_sessio def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): - 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') + from app.service.rest import get_detailed_services - 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['id']) - - 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': 1, 'failed': 0, 'requested': 2} - } - - -def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): 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') @@ -1226,8 +1201,33 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): } +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)) 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}, }