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
This commit is contained in:
Leo Hemsted
2018-07-10 14:39:51 +01:00
parent 7fda32b7cf
commit d2d267e5d8
2 changed files with 40 additions and 21 deletions

View File

@@ -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('/<template_id>')
@@ -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)

View File

@@ -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