From ad7278fd4e33b3cfcb0e04adec5335d8e59168c6 Mon Sep 17 00:00:00 2001 From: Jenny Duckett Date: Fri, 2 Dec 2016 12:30:25 +0000 Subject: [PATCH 1/3] Add tests for dao_fetch_todays_stats_for_all_services --- tests/app/dao/test_services_dao.py | 79 ++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 6140231e49..266ac14683 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -20,7 +20,8 @@ dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, - fetch_todays_total_message_count + fetch_todays_total_message_count, + dao_fetch_todays_stats_for_all_services ) from app.dao.users_dao import save_model_user from app.models import ( @@ -38,12 +39,16 @@ User, InvitedUser, Service, - BRANDING_GOVUK + BRANDING_GOVUK, + KEY_TYPE_NORMAL, + KEY_TYPE_TEAM, + KEY_TYPE_TEST ) from tests.app.conftest import ( sample_notification as create_notification, - sample_notification_history as create_notification_history + sample_notification_history as create_notification_history, + sample_email_template as create_email_template ) @@ -555,3 +560,71 @@ def test_dao_fetch_todays_total_message_count_returns_count_for_today(notify_db, def test_dao_fetch_todays_total_message_count_returns_0_when_no_messages_for_today(notify_db, notify_db_session): assert fetch_todays_total_message_count(uuid.uuid4()) == 0 + + +def test_dao_fetch_todays_stats_for_all_services_includes_all_services(notify_db, notify_db_session, service_factory): + # two services, each with an email and sms notification + service1 = service_factory.get('service 1', email_from='service.1') + service2 = service_factory.get('service 2', email_from='service.2') + create_notification(notify_db, notify_db_session, service=service1) + create_notification(notify_db, notify_db_session, service=service2) + create_notification( + notify_db, notify_db_session, service=service1, + template=create_email_template(notify_db, notify_db_session, service=service1)) + create_notification( + notify_db, notify_db_session, service=service2, + template=create_email_template(notify_db, notify_db_session, service=service2)) + + stats = dao_fetch_todays_stats_for_all_services().all() + + assert len(stats) == 4 + # services are ordered by service id; not explicit on email/sms or status + assert stats == sorted(stats, key=lambda x: x.service_id) + + +def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db): + with freeze_time('2001-01-01T23:59:00'): + just_before_midnight_yesterday = create_notification(notify_db, None, to_field='1', status='delivered') + + with freeze_time('2001-01-02T00:01:00'): + just_after_midnight_today = create_notification(notify_db, None, to_field='2', status='failed') + + with freeze_time('2001-01-02T12:00:00'): + stats = dao_fetch_todays_stats_for_all_services().all() + + stats = {row.status: row.count for row in stats} + assert 'delivered' not in stats + assert stats['failed'] == 1 + + +def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db, notify_db_session, service_factory): + service1 = service_factory.get('service 1', email_from='service.1') + service2 = service_factory.get('service 2', email_from='service.2') + # service1: 2 sms with status "created" and one "failed", and one email + create_notification(notify_db, notify_db_session, service=service1) + create_notification(notify_db, notify_db_session, service=service1) + create_notification(notify_db, notify_db_session, service=service1, status='failed') + create_notification( + notify_db, notify_db_session, service=service1, + template=create_email_template(notify_db, notify_db_session, service=service1)) + # service2: 1 sms "created" + create_notification(notify_db, notify_db_session, service=service2) + + stats = dao_fetch_todays_stats_for_all_services().all() + + assert len(stats) == 4 + assert ('sms', 'created', service1.id, 2) in stats + assert ('sms', 'failed', service1.id, 1) in stats + assert ('email', 'created', service1.id, 1) in stats + assert ('sms', 'created', service2.id, 1) in stats + + +def test_dao_fetch_todays_stats_for_all_services_includes_all_keys_by_default(notify_db, notify_db_session): + create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_NORMAL) + create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEAM) + create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST) + + stats = dao_fetch_todays_stats_for_all_services().all() + + assert len(stats) == 1 + assert stats[0].count == 3 From 7668745d8b3a92e3ddaaa266b972573f82708188 Mon Sep 17 00:00:00 2001 From: Jenny Duckett Date: Fri, 2 Dec 2016 16:43:24 +0000 Subject: [PATCH 2/3] Allow excluding test key use in the all services stats query --- app/dao/services_dao.py | 9 +++++++-- tests/app/dao/test_services_dao.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 08b30b3813..71896a5df7 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -239,8 +239,8 @@ def dao_fetch_weekly_historical_stats_for_service(service_id): @statsd(namespace='dao') -def dao_fetch_todays_stats_for_all_services(): - return db.session.query( +def dao_fetch_todays_stats_for_all_services(include_from_test_key=True): + query = db.session.query( Notification.notification_type, Notification.status, Notification.service_id, @@ -258,3 +258,8 @@ def dao_fetch_todays_stats_for_all_services(): ).order_by( Notification.service_id ) + + if not include_from_test_key: + query = query.filter(Notification.key_type != KEY_TYPE_TEST) + + return query diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 266ac14683..eacca3db3b 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -628,3 +628,14 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_keys_by_default(no assert len(stats) == 1 assert stats[0].count == 3 + + +def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key(notify_db, notify_db_session): + create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_NORMAL) + create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEAM) + create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST) + + stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=False).all() + + assert len(stats) == 1 + assert stats[0].count == 2 From d2649aebc8daa0646e9372094dfc86e72c9ff069 Mon Sep 17 00:00:00 2001 From: Jenny Duckett Date: Fri, 2 Dec 2016 17:40:12 +0000 Subject: [PATCH 3/3] Add include_from_test_key parameter to /service We want to be able to toggle the numbers on the platform admin page between including and excluding notifications sent using test keys, so that we can see both real use of the platform and all load on it. This parameter defaults to True, which is the existing behaviour. --- app/service/rest.py | 7 ++++--- tests/app/service/test_rest.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 75c68fe1fb..1c1e9b77c8 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -62,11 +62,12 @@ def get_services(): only_active = request.args.get('only_active') == 'True' detailed = request.args.get('detailed') == 'True' user_id = request.args.get('user_id', None) + include_from_test_key = request.args.get('include_from_test_key', 'True') != 'False' if user_id: services = dao_fetch_all_services_by_user(user_id, only_active) elif detailed: - return jsonify(data=get_detailed_services(only_active)) + return jsonify(data=get_detailed_services(only_active, include_from_test_key=include_from_test_key)) else: services = dao_fetch_all_services(only_active) data = service_schema.dump(services, many=True).data @@ -268,9 +269,9 @@ def get_detailed_service(service_id, today_only=False): return detailed_service_schema.dump(service).data -def get_detailed_services(only_active=False): +def get_detailed_services(only_active=False, include_from_test_key=True): services = {service.id: service for service in dao_fetch_all_services(only_active)} - stats = dao_fetch_todays_stats_for_all_services() + stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 3b3bdc7448..3641982d19 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -16,7 +16,7 @@ sample_user as create_sample_user, sample_notification as create_sample_notification, sample_notification_with_job) -from app.models import KEY_TYPE_TEST +from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST def test_get_service_list(notify_api, service_factory): @@ -1251,7 +1251,8 @@ 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) + create_sample_notification(notify_db, notify_db_session), + create_sample_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST) ] with notify_api.test_request_context(), notify_api.test_client() as client: resp = client.get( @@ -1264,6 +1265,29 @@ def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_sessio 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': 3} + } + + +def test_get_services_with_detailed_flag_excluding_from_test_key(notify_api, notify_db, notify_db_session): + notifications = [ + create_sample_notification(notify_db, notify_db_session, key_type=KEY_TYPE_NORMAL), + create_sample_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEAM), + create_sample_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST), + create_sample_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST), + create_sample_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST) + ] + with notify_api.test_request_context(), notify_api.test_client() as client: + resp = client.get( + '/service?detailed=True&include_from_test_key=False', + 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]['statistics'] == { 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}