Merge pull request #1855 from alphagov/inbound-sms-max-7-days

Only get inbound messages that are a maximum of 7 days old

app/dao/inbound_sms_dao.py

@@ -1,6 +1,7 @@
 from datetime import (
     timedelta,
-    datetime
+    datetime,
+    date
 )
 from flask import current_app
 from notifications_utils.statsd_decorators import statsd
@@ -10,6 +11,7 @@ from sqlalchemy.orm import aliased
 from app import db
 from app.dao.dao_utils import transactional
 from app.models import InboundSms
+from app.utils import get_london_midnight_in_utc
 
 
 @transactional
@@ -18,8 +20,10 @@ def dao_create_inbound_sms(inbound_sms):
 
 
 def dao_get_inbound_sms_for_service(service_id, limit=None, user_number=None):
+    start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6))
     q = InboundSms.query.filter(
-        InboundSms.service_id == service_id
+        InboundSms.service_id == service_id,
+        InboundSms.created_at >= start_date
     ).order_by(
         InboundSms.created_at.desc()
     )
@@ -56,8 +60,10 @@ def dao_get_paginated_inbound_sms_for_service_for_public_api(
 
 
 def dao_count_inbound_sms_for_service(service_id):
+    start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6))
     return InboundSms.query.filter(
-        InboundSms.service_id == service_id
+        InboundSms.service_id == service_id,
+        InboundSms.created_at >= start_date
     ).count()
 
 
@@ -102,6 +108,7 @@ 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 = get_london_midnight_in_utc(date.today() - timedelta(days=6))
     t2 = aliased(InboundSms)
     q = db.session.query(
         InboundSms
@@ -110,11 +117,12 @@ def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(
         and_(
             InboundSms.user_number == t2.user_number,
             InboundSms.service_id == t2.service_id,
-            InboundSms.created_at < t2.created_at
+            InboundSms.created_at < t2.created_at,
         )
     ).filter(
         t2.id == None,  # noqa
-        InboundSms.service_id == service_id
+        InboundSms.service_id == service_id,
+        InboundSms.created_at >= start_date
     ).order_by(
         InboundSms.created_at.desc()
     )

tests/app/dao/test_inbound_sms_dao.py

@@ -37,7 +37,8 @@ def test_get_all_inbound_sms_limits_and_orders(sample_service):
     with freeze_time('2017-01-02'):
         two = create_inbound_sms(sample_service)
 
-    res = dao_get_inbound_sms_for_service(sample_service.id, limit=2)
+        res = dao_get_inbound_sms_for_service(sample_service.id, limit=2)
+
     assert len(res) == 2
     assert res[0] == three
     assert res[0].created_at == datetime(2017, 1, 3)
@@ -57,6 +58,22 @@ def test_get_all_inbound_sms_filters_on_service(notify_db_session):
     assert res[0] == sms_one
 
 
+def test_get_all_inbound_sms_filters_on_time(sample_service, notify_db_session):
+    create_inbound_sms(sample_service, user_number='447700900111', content='111 1', created_at=datetime(2017, 1, 2))
+    sms_two = create_inbound_sms(
+        sample_service,
+        user_number='447700900111',
+        content='111 2',
+        created_at=datetime(2017, 1, 3)
+    )
+
+    with freeze_time('2017-01-09'):
+        res = dao_get_inbound_sms_for_service(sample_service.id)
+
+    assert len(res) == 1
+    assert res[0] == sms_two
+
+
 def test_count_inbound_sms_for_service(notify_db_session):
     service_one = create_service(service_name='one')
     service_two = create_service(service_name='two')
@@ -68,6 +85,14 @@ def test_count_inbound_sms_for_service(notify_db_session):
     assert dao_count_inbound_sms_for_service(service_one.id) == 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))
+
+    with freeze_time('2017-01-09'):
+        assert dao_count_inbound_sms_for_service(sample_service.id) == 1
+
+
 @freeze_time("2017-01-01 12:00:00")
 def test_should_delete_inbound_sms_older_than_seven_days(sample_service):
     older_than_seven_days = datetime.utcnow() - timedelta(days=7, seconds=1)
@@ -187,7 +212,8 @@ def test_most_recent_inbound_sms_only_returns_most_recent_for_each_number(notify
     create_inbound_sms(sample_service, user_number='447700900222', content='222 2', created_at=datetime(2017, 1, 2))
 
     with set_config(notify_api, 'PAGE_SIZE', 3):
-        res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1)
+        with freeze_time('2017-01-02'):
+            res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1)
 
     assert len(res.items) == 2
     assert res.has_next is False
@@ -207,17 +233,40 @@ def test_most_recent_inbound_sms_paginates_properly(notify_api, sample_service):
     create_inbound_sms(sample_service, user_number='447700900444', content='444 2', created_at=datetime(2017, 1, 8))
 
     with set_config(notify_api, 'PAGE_SIZE', 2):
-        # 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)
-        assert len(res.items) == 2
-        assert res.has_next is True
-        assert res.per_page == 2
-        assert res.items[0].content == '444 2'
-        assert res.items[1].content == '333 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)
+            assert len(res.items) == 2
+            assert res.has_next is True
+            assert res.per_page == 2
+            assert res.items[0].content == '444 2'
+            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)
-        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'
+            # 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)
+            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))
+
+    # 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('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'

tests/app/inbound_sms/test_rest.py

@@ -37,13 +37,12 @@ def test_post_to_get_inbound_sms_with_limit(admin_request, sample_service):
     with freeze_time('2017-01-02'):
         two = create_inbound_sms(sample_service)
 
-    data = {'limit': 1}
-
-    sms = admin_request.post(
-        'inbound_sms.post_query_inbound_sms_for_service',
-        service_id=sample_service.id,
-        _data=data
-    )['data']
+        data = {'limit': 1}
+        sms = admin_request.post(
+            'inbound_sms.post_query_inbound_sms_for_service',
+            service_id=sample_service.id,
+            _data=data
+        )['data']
 
     assert len(sms) == 1
     assert sms[0]['id'] == str(two.id)
@@ -220,10 +219,10 @@ def test_get_inbound_sms_summary(admin_request, sample_service):
     with freeze_time('2017-01-03'):
         create_inbound_sms(other_service)
 
-    summary = admin_request.get(
-        'inbound_sms.get_inbound_sms_summary_for_service',
-        service_id=sample_service.id
-    )
+        summary = admin_request.get(
+            'inbound_sms.get_inbound_sms_summary_for_service',
+            service_id=sample_service.id
+        )
 
     assert summary == {
         'count': 2,