From 3cd8605e111a05d23349ff42b3170d0c59cfded2 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 31 Jan 2017 11:32:53 +0000 Subject: [PATCH 1/2] fix api statistics to account for letters --- app/service/statistics.py | 4 +-- tests/app/service/test_statistics.py | 52 +++++++++++++++++++--------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/app/service/statistics.py b/app/service/statistics.py index 3f06fcacb..626335784 100644 --- a/app/service/statistics.py +++ b/app/service/statistics.py @@ -1,7 +1,7 @@ import itertools from datetime import datetime, timedelta -from app.models import EMAIL_TYPE, SMS_TYPE +from app.models import TEMPLATE_TYPES def format_statistics(statistics): @@ -33,7 +33,7 @@ def create_zeroed_stats_dicts(): return { template_type: { status: 0 for status in ('requested', 'delivered', 'failed') - } for template_type in (EMAIL_TYPE, SMS_TYPE) + } for template_type in TEMPLATE_TYPES } diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index fcf854da8..c5adad55d 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -16,24 +16,25 @@ WeeklyStatsRow = collections.namedtuple('row', ('notification_type', 'status', ' # email_counts and sms_counts are 3-tuple of requested, delivered, failed -@pytest.mark.idparametrize('stats, email_counts, sms_counts', { - 'empty': ([], [0, 0, 0], [0, 0, 0]), +@pytest.mark.idparametrize('stats, email_counts, sms_counts, letter_counts', { + 'empty': ([], [0, 0, 0], [0, 0, 0], [0, 0, 0]), 'always_increment_requested': ([ StatsRow('email', 'delivered', 1), StatsRow('email', 'failed', 1) - ], [2, 1, 1], [0, 0, 0]), - 'dont_mix_email_and_sms': ([ + ], [2, 1, 1], [0, 0, 0], [0, 0, 0]), + 'dont_mix_template_types': ([ StatsRow('email', 'delivered', 1), - StatsRow('sms', 'delivered', 1) - ], [1, 1, 0], [1, 1, 0]), + StatsRow('sms', 'delivered', 1), + StatsRow('letter', 'delivered', 1) + ], [1, 1, 0], [1, 1, 0], [1, 1, 0]), 'convert_fail_statuses_to_failed': ([ StatsRow('email', 'failed', 1), StatsRow('email', 'technical-failure', 1), StatsRow('email', 'temporary-failure', 1), StatsRow('email', 'permanent-failure', 1), - ], [4, 0, 4], [0, 0, 0]), + ], [4, 0, 4], [0, 0, 0], [0, 0, 0]), }) -def test_format_statistics(stats, email_counts, sms_counts): +def test_format_statistics(stats, email_counts, sms_counts, letter_counts): ret = format_statistics(stats) @@ -49,6 +50,12 @@ def test_format_statistics(stats, email_counts, sms_counts): in zip(['requested', 'delivered', 'failed'], sms_counts) } + assert ret['letter'] == { + status: count + for status, count + in zip(['requested', 'delivered', 'failed'], letter_counts) + } + @pytest.mark.parametrize('start,end,dates', [ (datetime(2016, 7, 25), datetime(2016, 7, 25), [datetime(2016, 7, 25)]), @@ -66,6 +73,7 @@ def test_create_zeroed_stats_dicts(): assert create_zeroed_stats_dicts() == { 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 0, 'delivered': 0, 'failed': 0}, } @@ -79,43 +87,51 @@ def _stats(requested, delivered, failed): (datetime(2016, 7, 28), [], { datetime(2016, 7, 25): { 'sms': _stats(0, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) } }), # with a random created time, still create the dict for midnight (datetime(2016, 7, 28, 12, 13, 14), [], { datetime(2016, 7, 25, 0, 0, 0): { 'sms': _stats(0, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) } }), # with no stats but a service (datetime(2016, 7, 14), [], { datetime(2016, 7, 11): { 'sms': _stats(0, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) }, datetime(2016, 7, 18): { 'sms': _stats(0, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) }, datetime(2016, 7, 25): { 'sms': _stats(0, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) } }), # two stats for same week dont re-zero each other (datetime(2016, 7, 21), [ WeeklyStatsRow('email', 'created', datetime(2016, 7, 18), 1), WeeklyStatsRow('sms', 'created', datetime(2016, 7, 18), 1), + WeeklyStatsRow('letter', 'created', datetime(2016, 7, 18), 1), ], { datetime(2016, 7, 18): { 'sms': _stats(1, 0, 0), - 'email': _stats(1, 0, 0) + 'email': _stats(1, 0, 0), + 'letter': _stats(1, 0, 0) }, datetime(2016, 7, 25): { 'sms': _stats(0, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) } }), # two stats for same type are added together @@ -126,11 +142,13 @@ def _stats(requested, delivered, failed): ], { datetime(2016, 7, 18): { 'sms': _stats(2, 1, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) }, datetime(2016, 7, 25): { 'sms': _stats(1, 0, 0), - 'email': _stats(0, 0, 0) + 'email': _stats(0, 0, 0), + 'letter': _stats(0, 0, 0) } }) ]) From 0d089a383eaa4618f4adb56e81e919e817274b3b Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 31 Jan 2017 12:06:25 +0000 Subject: [PATCH 2/2] fix tests in service/test_rest.py --- tests/app/service/test_rest.py | 51 +++++++++++++++------------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index cc4c5e38c..5835d5159 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1179,16 +1179,8 @@ 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): with notify_api.test_request_context(), notify_api.test_client() as client: @@ -1205,7 +1197,7 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s service = json.loads(resp.get_data(as_text=True))['data'] assert service['id'] == str(sample_service.id) assert 'statistics' in service.keys() - assert set(service['statistics'].keys()) == set(['sms', 'email']) + assert set(service['statistics'].keys()) == {'sms', 'email', 'letter'} assert service['statistics']['sms'] == stats @@ -1222,16 +1214,9 @@ def test_get_weekly_notification_stats(notify_api, notify_db, notify_db_session) data = json.loads(resp.get_data(as_text=True))['data'] assert data == { '1999-12-27': { - 'sms': { - 'requested': 1, - 'delivered': 0, - 'failed': 0 - }, - 'email': { - 'requested': 0, - 'delivered': 0, - 'failed': 0 - } + 'sms': {'requested': 1, 'delivered': 0, 'failed': 0}, + 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } } @@ -1255,7 +1240,8 @@ def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_sessio 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} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 3}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1278,7 +1264,8 @@ def test_get_services_with_detailed_flag_excluding_from_test_key(notify_api, not assert len(data) == 1 assert data[0]['statistics'] == { 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 2} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1300,12 +1287,14 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): 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} + 'sms': {'delivered': 1, 'failed': 0, 'requested': 3}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } 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} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 1}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1325,12 +1314,14 @@ def test_get_detailed_services_includes_services_with_no_notifications(notify_db 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} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 1}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } 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} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1348,7 +1339,8 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not assert len(data) == 1 assert data[0]['statistics'] == { 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 2} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1367,7 +1359,8 @@ def test_get_detailed_services_for_date_range(notify_db, notify_db_session): assert len(data) == 1 assert data[0]['statistics'] == { 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 2} + 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}, + 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} }