diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 0b6334479..49d962318 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -55,11 +55,10 @@ def dao_get_paginated_inbound_sms_for_service_for_public_api( ).items -def dao_count_inbound_sms_for_service(service_id): - start_date = midnight_n_days_ago(6) +def dao_count_inbound_sms_for_service(service_id, limit_days): return InboundSms.query.filter( InboundSms.service_id == service_id, - InboundSms.created_at >= start_date + InboundSms.created_at >= midnight_n_days_ago(limit_days) ).count() @@ -126,10 +125,11 @@ def dao_get_inbound_sms_by_id(service_id, inbound_id): def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service( service_id, - page + page, + limit_days ): """ - This query starts from inbound_sms and joins on to itself to find the most recent row for each user_number + This query starts from inbound_sms and joins on to itself to find the most recent row for each user_number. Equivalent sql: @@ -146,7 +146,6 @@ def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service( ORDER BY t1.created_at DESC; LIMIT 50 OFFSET :page """ - start_date = midnight_n_days_ago(6) t2 = aliased(InboundSms) q = db.session.query( InboundSms @@ -160,7 +159,7 @@ def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service( ).filter( t2.id == None, # noqa InboundSms.service_id == service_id, - InboundSms.created_at >= start_date + InboundSms.created_at >= midnight_n_days_ago(limit_days) ).order_by( InboundSms.created_at.desc() ) diff --git a/app/inbound_sms/rest.py b/app/inbound_sms/rest.py index 884ae24ce..38c6b55bd 100644 --- a/app/inbound_sms/rest.py +++ b/app/inbound_sms/rest.py @@ -12,6 +12,7 @@ from app.dao.inbound_sms_dao import ( dao_get_inbound_sms_by_id, dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service ) +from app.dao.service_data_retention_dao import fetch_service_data_retention_by_notification_type from app.errors import register_errors from app.schema_validation import validate @@ -55,9 +56,12 @@ def get_inbound_sms_for_service(service_id): def get_most_recent_inbound_sms_for_service(service_id): # used on the service inbox page page = request.args.get('page', 1) - # get most recent message for each user for service - results = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(service_id, int(page)) + inbound_data_retention = fetch_service_data_retention_by_notification_type(service_id, 'sms') + limit_days = inbound_data_retention.days_of_retention if inbound_data_retention else 7 + + # get most recent message for each user for service + results = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(service_id, int(page), limit_days) return jsonify( data=[row.serialize() for row in results.items], has_next=results.has_next @@ -66,7 +70,8 @@ def get_most_recent_inbound_sms_for_service(service_id): @inbound_sms.route('/summary') def get_inbound_sms_summary_for_service(service_id): - count = dao_count_inbound_sms_for_service(service_id) + # this is for the dashboard, so always limit to 7 days, even if they have a longer data retention + count = dao_count_inbound_sms_for_service(service_id, limit_days=7) most_recent = dao_get_inbound_sms_for_service(service_id, limit=1) return jsonify( diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index 24eb1761c..9c61a8934 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -76,15 +76,17 @@ def test_count_inbound_sms_for_service(notify_db_session): create_inbound_sms(service_one) create_inbound_sms(service_two) - assert dao_count_inbound_sms_for_service(service_one.id) == 2 + assert dao_count_inbound_sms_for_service(service_one.id, limit_days=1) == 2 -def test_count_inbound_sms_for_service_filters_messages_older_than_seven_days(sample_service, notify_db_session): - create_inbound_sms(sample_service, user_number='447700900111', content='111 2', created_at=datetime(2017, 1, 2)) - create_inbound_sms(sample_service, user_number='447700900111', content='111 2', created_at=datetime(2017, 1, 3)) +def test_count_inbound_sms_for_service_filters_messages_older_than_n_days(sample_service): + # test between evening sunday 2nd of june and morning of monday 3rd + create_inbound_sms(sample_service, created_at=datetime(2019, 6, 2, 22, 59)) + create_inbound_sms(sample_service, created_at=datetime(2019, 6, 2, 22, 59)) + create_inbound_sms(sample_service, created_at=datetime(2019, 6, 2, 23, 1)) - with freeze_time('2017-01-09'): - assert dao_count_inbound_sms_for_service(sample_service.id) == 1 + with freeze_time('Monday 10th June 2019 12:00'): + assert dao_count_inbound_sms_for_service(sample_service.id, limit_days=7) == 1 @freeze_time("2017-06-08 12:00:00") @@ -222,7 +224,7 @@ def test_most_recent_inbound_sms_only_returns_most_recent_for_each_number(notify with set_config(notify_api, 'PAGE_SIZE', 3): with freeze_time('2017-01-02'): - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, limit_days=7, page=1) # noqa assert len(res.items) == 2 assert res.has_next is False @@ -244,7 +246,7 @@ def test_most_recent_inbound_sms_paginates_properly(notify_api, sample_service): with set_config(notify_api, 'PAGE_SIZE', 2): with freeze_time('2017-01-02'): # first page has most recent 444 and 333 - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, limit_days=7, page=1) # noqa assert len(res.items) == 2 assert res.has_next is True assert res.per_page == 2 @@ -252,30 +254,21 @@ def test_most_recent_inbound_sms_paginates_properly(notify_api, sample_service): assert res.items[1].content == '333 2' # second page has no 444 or 333 - just most recent 222 and 111 - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=2) + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, limit_days=7, page=2) # noqa assert len(res.items) == 2 assert res.has_next is False assert res.items[0].content == '222 2' assert res.items[1].content == '111 2' -def test_most_recent_inbound_sms_only_returns_values_within_7_days(notify_api, sample_service): - create_inbound_sms(sample_service, user_number='447700900111', content='111 1', created_at=datetime(2017, 4, 1)) - create_inbound_sms(sample_service, user_number='447700900111', content='111 2', created_at=datetime(2017, 4, 1)) - create_inbound_sms(sample_service, user_number='447700900222', content='222 1', created_at=datetime(2017, 4, 1)) - create_inbound_sms(sample_service, user_number='447700900222', content='222 2', created_at=datetime(2017, 4, 1)) - create_inbound_sms(sample_service, user_number='447700900333', content='333 1', created_at=datetime(2017, 4, 2)) - create_inbound_sms(sample_service, user_number='447700900333', content='333 2', created_at=datetime(2017, 4, 3)) - create_inbound_sms(sample_service, user_number='447700900444', content='444 1', created_at=datetime(2017, 4, 4)) - create_inbound_sms(sample_service, user_number='447700900444', content='444 2', created_at=datetime(2017, 4, 5)) +def test_most_recent_inbound_sms_only_returns_values_within_7_days(sample_service): + # just out of bounds + create_inbound_sms(sample_service, user_number='1', content='old', created_at=datetime(2017, 4, 2, 22, 59, 59)) + # just in bounds + create_inbound_sms(sample_service, user_number='2', content='new', created_at=datetime(2017, 4, 2, 23, 0, 0)) - # 7 days ago BST midnight - create_inbound_sms(sample_service, user_number='447700900666', content='666 1', created_at='2017-04-02T23:00:00') + with freeze_time('Monday 10th April 2017 12:00:00'): + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, limit_days=7, page=1) # noqa - with freeze_time('2017-04-09T12:00:00'): - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) - - assert len(res.items) == 3 - assert res.items[0].content == '444 2' - assert res.items[1].content == '333 2' - assert res.items[2].content == '666 1' + assert len(res.items) == 1 + assert res.items[0].content == 'new' diff --git a/tests/app/inbound_sms/test_rest.py b/tests/app/inbound_sms/test_rest.py index eb04d397a..cfa150d52 100644 --- a/tests/app/inbound_sms/test_rest.py +++ b/tests/app/inbound_sms/test_rest.py @@ -1,10 +1,10 @@ -from datetime import datetime +from datetime import datetime, timedelta import pytest from freezegun import freeze_time from tests.app.db import ( - create_inbound_sms, create_service, create_service_with_inbound_number, + create_inbound_sms, create_service, create_service_with_inbound_number, create_service_data_retention ) @@ -297,3 +297,40 @@ def test_get_most_recent_inbound_sms_for_service( assert len(response['data']) == expected_rows assert response['has_next'] == has_next_link + + +@freeze_time('Monday 10th April 2017 12:00') +def test_get_most_recent_inbound_sms_for_service_respects_data_retention( + admin_request, + sample_service +): + create_service_data_retention(sample_service.id, 'sms', 5) + for i in range(10): + created = datetime.utcnow() - timedelta(days=i) + create_inbound_sms(sample_service, user_number='44770090000{}'.format(i), created_at=created) + + response = admin_request.get('inbound_sms.get_most_recent_inbound_sms_for_service', service_id=sample_service.id) + + assert len(response['data']) == 6 + assert [x['created_at'] for x in response['data']] == [ + '2017-04-10T12:00:00.000000Z', + '2017-04-09T12:00:00.000000Z', + '2017-04-08T12:00:00.000000Z', + '2017-04-07T12:00:00.000000Z', + '2017-04-06T12:00:00.000000Z', + '2017-04-05T12:00:00.000000Z', + ] + + +@freeze_time('Monday 10th April 2017 12:00') +def test_get_most_recent_inbound_sms_for_service_respects_data_retention_if_older_than_a_week( + admin_request, + sample_service +): + create_service_data_retention(sample_service.id, 'sms', 14) + create_inbound_sms(sample_service, created_at=datetime(2017, 4, 1, 12, 0)) + + response = admin_request.get('inbound_sms.get_most_recent_inbound_sms_for_service', service_id=sample_service.id) + + assert len(response['data']) == 1 + assert response['data'][0]['created_at'] == '2017-04-01T12:00:00.000000Z'