From 7fda32b7cf2efb5fb14405ae8a20b8a9ce4b3a87 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 6 Jul 2018 18:09:41 +0100 Subject: [PATCH] dashboard functions should return data for 8 days, not 7 really, it'll be somewhere btween 7 and 8 depending on what time of day you request it at. But if today is monday, then seven days ago is last tuesday - but we should return data for last monday as well so that users see a full week's worth of data also update/clarify the tests to make sure this is being honored for all the different widgets on the dashboard --- app/dao/services_dao.py | 6 ++-- tests/app/dao/test_services_dao.py | 35 +++++++++------------ tests/app/job/test_rest.py | 36 ++++++++++------------ tests/app/template_statistics/test_rest.py | 30 +++++++++--------- 4 files changed, 49 insertions(+), 58 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 4aff00390..d1afdc3d5 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -37,7 +37,7 @@ from app.models import ( SMS_TYPE, LETTER_TYPE, ) -from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc +from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc, midnight_n_days_ago DEFAULT_SERVICE_PERMISSIONS = [ SMS_TYPE, @@ -247,8 +247,8 @@ def delete_service_and_all_associated_db_objects(service): @statsd(namespace="dao") def dao_fetch_stats_for_service(service_id): - # We want 7 days inclusive - start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) + # We always want between seven and eight days + start_date = midnight_n_days_ago(7) return _stats_for_service_query(service_id).filter( Notification.created_at >= start_date ).all() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index e9a85a2eb..2720db90c 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -620,30 +620,23 @@ def test_fetch_stats_for_today_only_includes_today(notify_db, notify_db_session, assert stats['created'] == 1 -def test_fetch_stats_should_not_gather_notifications_older_than_7_days(notify_db, notify_db_session, sample_template): +@pytest.mark.parametrize('created_at, rows_returned', [ + ('Sunday 8th July 2018 12:00', 0), + ('Sunday 8th July 2018 22:59', 0), + ('Sunday 8th July 2018 23:00', 1), + ('Monday 9th July 2018 09:00', 1), + ('Monday 9th July 2018 15:00', 1), + ('Monday 16th July 2018 12:00', 1), +]) +def test_fetch_stats_should_not_gather_notifications_older_than_7_days(sample_template, created_at, rows_returned): + # It's monday today. Things made last monday should still show + with freeze_time(created_at): + create_notification_db(sample_template,) - # 8 days ago - create_notification(notify_db, None, to_field='1', status='delivered', created_at='2001-04-02T12:00:00') - create_notification(notify_db, None, to_field='1', status='delivered', created_at='2001-04-02T22:59:59') - - # 7 days ago BST midnight - create_notification(notify_db, None, to_field='1', status='failed', created_at='2001-04-02T23:00:00') - # 7 days ago, 2hours ago - create_notification(notify_db, None, to_field='2', status='failed', created_at='2001-04-03T10:00:00') - - # 7 days ago - create_notification(notify_db, None, to_field='2', status='failed', created_at='2001-04-03T12:00:00') - - # right_now - create_notification(notify_db, None, to_field='3', status='created', created_at='2001-04-09T12:00:00') - - with freeze_time('2001-04-09T12:00:00'): + with freeze_time('Monday 16th July 2018 12:00'): stats = dao_fetch_stats_for_service(sample_template.service_id) - stats = {row.status: row.count for row in stats} - assert 'delivered' not in stats - assert stats['failed'] == 3 - assert stats['created'] == 1 + assert len(stats) == rows_returned def test_dao_fetch_todays_total_message_count_returns_count_for_today(notify_db, diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index a4fd20117..a147267f4 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -619,28 +619,24 @@ def test_get_jobs(client, notify_db, notify_db_session, sample_template): assert len(resp_json['data']) == 5 -def test_get_jobs_with_limit_days(client, notify_db, notify_db_session, sample_template): - create_job( - notify_db, - notify_db_session, - service=sample_template.service, - template=sample_template, - ) - create_job( - notify_db, - notify_db_session, - service=sample_template.service, - template=sample_template, - created_at=datetime.now() - timedelta(days=7)) +def test_get_jobs_with_limit_days(admin_request, notify_db, notify_db_session, sample_template): + for time in [ + 'Sunday 1st July 2018 22:59', + 'Sunday 2nd July 2018 23:00', # beginning of monday morning + 'Monday 3rd July 2018 12:00' + ]: + with freeze_time(time): + create_job( + notify_db, + notify_db_session, + service=sample_template.service, + template=sample_template, + ) - service_id = sample_template.service.id + with freeze_time('Monday 9th July 2018 12:00'): + resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id, limit_days=7) - path = '/service/{}/job'.format(service_id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header], query_string={'limit_days': 5}) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert len(resp_json['data']) == 1 + assert len(resp_json['data']) == 2 def test_get_jobs_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_service): diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index abf464773..2aed2374b 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -33,7 +33,7 @@ def set_up_get_all_from_hash(mock_redis, side_effect): @pytest.mark.parametrize('query_string', [ {}, {'limit_days': 0}, - {'limit_days': 8}, + {'limit_days': 9}, {'limit_days': 3.5}, {'limit_days': 'blurk'}, ]) @@ -190,6 +190,7 @@ def test_get_template_statistics_for_service_by_day_gets_stats_for_correct_days( {sample_template.id: 1}, {sample_template.id: 1}, {sample_template.id: 1}, + {sample_template.id: 1}, None, None, ]) @@ -203,25 +204,26 @@ def test_get_template_statistics_for_service_by_day_gets_stats_for_correct_days( json_resp = admin_request.get( 'template_statistics.get_template_statistics_for_service_by_day', service_id=sample_template.service_id, - limit_days=7 + limit_days=8 ) assert len(json_resp['data']) == 1 - assert json_resp['data'][0]['count'] == 10 + assert json_resp['data'][0]['count'] == 11 assert json_resp['data'][0]['template_id'] == str(sample_template.id) - assert mock_redis.get_all_from_hash.call_count == 7 + assert mock_redis.get_all_from_hash.call_count == 8 - assert '2018-03-22' in mock_redis.get_all_from_hash.mock_calls[0][1][0] - assert '2018-03-23' in mock_redis.get_all_from_hash.mock_calls[1][1][0] - assert '2018-03-24' in mock_redis.get_all_from_hash.mock_calls[2][1][0] - assert '2018-03-25' in mock_redis.get_all_from_hash.mock_calls[3][1][0] - assert '2018-03-26' in mock_redis.get_all_from_hash.mock_calls[4][1][0] - assert '2018-03-27' in mock_redis.get_all_from_hash.mock_calls[5][1][0] - assert '2018-03-28' in mock_redis.get_all_from_hash.mock_calls[6][1][0] + assert '2018-03-21' in mock_redis.get_all_from_hash.mock_calls[0][1][0] # last wednesday + assert '2018-03-22' in mock_redis.get_all_from_hash.mock_calls[1][1][0] + assert '2018-03-23' in mock_redis.get_all_from_hash.mock_calls[2][1][0] + assert '2018-03-24' in mock_redis.get_all_from_hash.mock_calls[3][1][0] + assert '2018-03-25' in mock_redis.get_all_from_hash.mock_calls[4][1][0] + assert '2018-03-26' in mock_redis.get_all_from_hash.mock_calls[5][1][0] + assert '2018-03-27' in mock_redis.get_all_from_hash.mock_calls[6][1][0] + assert '2018-03-28' in mock_redis.get_all_from_hash.mock_calls[7][1][0] # current day (wednesday) mock_dao.mock_calls == [ - call(ANY, day=datetime(2018, 3, 23)), + call(ANY, day=datetime(2018, 3, 22)), call(ANY, day=datetime(2018, 3, 27)), call(ANY, day=datetime(2018, 3, 28)) ] @@ -237,11 +239,11 @@ def test_get_template_statistics_for_service_by_day_returns_empty_list_if_no_tem json_resp = admin_request.get( 'template_statistics.get_template_statistics_for_service_by_day', service_id=sample_service.id, - limit_days=7 + limit_days=8 ) assert len(json_resp['data']) == 0 - assert mock_redis.get_all_from_hash.call_count == 7 + assert mock_redis.get_all_from_hash.call_count == 8 # make sure we don't try and set any empty hashes in redis assert mock_redis.set_hash_and_expire.call_count == 0