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/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/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 6140231e49..eacca3db3b 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,82 @@ 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 + + +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 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}