From d2d267e5d8911f12c25672b8b54db003fee477bb Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 10 Jul 2018 14:39:51 +0100 Subject: [PATCH] rename `limit_days` to `whole_days` on template statistics endpoint and return data for one more day. we're not really limiting to 7 days - we're returning 7 entire days, plus whatever time has elapsed since midnight today. I felt it would be best to rename the variable to `whole_days` to imply that it's not "limit this data set to seven days", it's "give me at least seven days". the endpoint is backwards compatible so we can rename the variable on the front-end later --- app/template_statistics/rest.py | 19 +++++----- tests/app/template_statistics/test_rest.py | 42 +++++++++++++++------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index d5ea53c23..1c0f3b27d 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -29,17 +29,18 @@ register_errors(template_statistics) @template_statistics.route('') def get_template_statistics_for_service_by_day(service_id): + whole_days = request.args.get('whole_days', request.args.get('limit_days', '')) try: - limit_days = int(request.args.get('limit_days', '')) + whole_days = int(whole_days) except ValueError: - error = '{} is not an integer'.format(request.args.get('limit_days')) - message = {'limit_days': [error]} + error = '{} is not an integer'.format(whole_days) + message = {'whole_days': [error]} raise InvalidRequest(message, status_code=400) - if limit_days < 1 or limit_days > 7: - raise InvalidRequest({'limit_days': ['limit_days must be between 1 and 7']}, status_code=400) + if whole_days < 0 or whole_days > 7: + raise InvalidRequest({'whole_days': ['whole_days must be between 0 and 7']}, status_code=400) - return jsonify(data=_get_template_statistics_for_last_n_days(service_id, limit_days)) + return jsonify(data=_get_template_statistics_for_last_n_days(service_id, whole_days)) @template_statistics.route('/') @@ -54,10 +55,12 @@ def get_template_statistics_for_template_id(service_id, template_id): return jsonify(data=data) -def _get_template_statistics_for_last_n_days(service_id, limit_days): +def _get_template_statistics_for_last_n_days(service_id, whole_days): template_stats_by_id = Counter() - for day in last_n_days(limit_days): + # 0 whole_days = last 1 days (ie since midnight today) = today. + # 7 whole days = last 8 days (ie since midnight this day last week) = a week and a bit + for day in last_n_days(whole_days + 1): # "{SERVICE_ID}-template-usage-{YYYY-MM-DD}" key = cache_key_for_service_template_usage_per_day(service_id, day) stats = redis_store.get_all_from_hash(key) diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 2aed2374b..b9559e134 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -32,10 +32,10 @@ def set_up_get_all_from_hash(mock_redis, side_effect): @pytest.mark.parametrize('query_string', [ {}, - {'limit_days': 0}, - {'limit_days': 9}, - {'limit_days': 3.5}, - {'limit_days': 'blurk'}, + {'whole_days': -1}, + {'whole_days': 8}, + {'whole_days': 3.5}, + {'whole_days': 'blurk'}, ]) def test_get_template_statistics_for_service_by_day_with_bad_arg_returns_400(admin_request, query_string): json_resp = admin_request.get( @@ -45,14 +45,14 @@ def test_get_template_statistics_for_service_by_day_with_bad_arg_returns_400(adm _expected_status=400 ) assert json_resp['result'] == 'error' - assert 'limit_days' in json_resp['message'] + assert 'whole_days' in json_resp['message'] def test_get_template_statistics_for_service_by_day_returns_template_info(admin_request, mocker, sample_notification): json_resp = admin_request.get( 'template_statistics.get_template_statistics_for_service_by_day', service_id=sample_notification.service_id, - limit_days=1 + whole_days=1 ) assert len(json_resp['data']) == 1 @@ -64,6 +64,22 @@ def test_get_template_statistics_for_service_by_day_returns_template_info(admin_ assert json_resp['data'][0]['is_precompiled_letter'] is False +@pytest.mark.parametrize('var_name', ['limit_days', 'whole_days']) +def test_get_template_statistics_for_service_by_day_accepts_old_query_string( + admin_request, + mocker, + sample_notification, + var_name +): + json_resp = admin_request.get( + 'template_statistics.get_template_statistics_for_service_by_day', + service_id=sample_notification.service_id, + **{var_name: 1} + ) + + assert len(json_resp['data']) == 1 + + @freeze_time('2018-01-01 12:00:00') def test_get_template_statistics_for_service_by_day_gets_out_of_redis_if_available( admin_request, @@ -78,7 +94,7 @@ def test_get_template_statistics_for_service_by_day_gets_out_of_redis_if_availab json_resp = admin_request.get( 'template_statistics.get_template_statistics_for_service_by_day', service_id=sample_template.service_id, - limit_days=1 + whole_days=0 ) assert len(json_resp['data']) == 1 @@ -112,7 +128,7 @@ def test_get_template_statistics_for_service_by_day_goes_to_db_if_not_in_redis( json_resp = admin_request.get( 'template_statistics.get_template_statistics_for_service_by_day', service_id=sample_template.service_id, - limit_days=2 + whole_days=1 ) assert len(json_resp['data']) == 1 @@ -161,7 +177,7 @@ def test_get_template_statistics_for_service_by_day_combines_templates_correctly json_resp = admin_request.get( 'template_statistics.get_template_statistics_for_service_by_day', service_id=sample_service.id, - limit_days=3 + whole_days=2 ) assert len(json_resp['data']) == 2 @@ -185,14 +201,14 @@ def test_get_template_statistics_for_service_by_day_gets_stats_for_correct_days( # first time it is called redis returns data, second time returns none set_up_get_all_from_hash(mock_redis, [ - {sample_template.id: 1}, + {sample_template.id: 1}, # last weds None, {sample_template.id: 1}, {sample_template.id: 1}, {sample_template.id: 1}, {sample_template.id: 1}, None, - None, + None, # current day ]) mock_dao = mocker.patch( 'app.template_statistics.rest.dao_get_template_usage', @@ -204,7 +220,7 @@ 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=8 + whole_days=7 ) assert len(json_resp['data']) == 1 @@ -239,7 +255,7 @@ 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=8 + whole_days=7 ) assert len(json_resp['data']) == 0