Merge pull request #1011 from alphagov/imdad-feat-delete-old-inbound-msgs

Delete inbound messages older than seven days

app/celery/scheduled_tasks.py

@@ -9,6 +9,7 @@ from sqlalchemy.exc import SQLAlchemyError
 from app.aws import s3
 from app import notify_celery
 from app import performance_platform_client
+from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago
 from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago
 from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than_limited_by
 from app.dao.notifications_dao import (
@@ -226,3 +227,21 @@ def timeout_job_statistics():
     if updated:
         current_app.logger.info(
             "Timeout period reached for {} job statistics, failure count has been updated.".format(updated))
+
+
+@notify_celery.task(name="delete-inbound-sms")
+@statsd(namespace="tasks")
+def delete_inbound_sms_older_than_seven_days():
+    try:
+        start = datetime.utcnow()
+        deleted = delete_inbound_sms_created_more_than_a_week_ago()
+        current_app.logger.info(
+            "Delete inbound sms job started {} finished {} deleted {} inbound sms notifications".format(
+                start,
+                datetime.utcnow(),
+                deleted
+            )
+        )
+    except SQLAlchemyError as e:
+        current_app.logger.exception("Failed to delete inbound sms notifications")
+        raise

app/config.py

@@ -2,6 +2,7 @@ from datetime import timedelta
 from celery.schedules import crontab
 from kombu import Exchange, Queue
 import os
+
 from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST
 
 if os.environ.get('VCAP_SERVICES'):
@@ -168,6 +169,11 @@ class Config(object):
             'schedule': crontab(minute=40, hour=0),
             'options': {'queue': QueueNames.PERIODIC}
         },
+        'delete-inbound-sms': {
+            'task': 'delete-inbound-sms',
+            'schedule': crontab(minute=0, hour=1),
+            'options': {'queue': QueueNames.PERIODIC}
+        },
         'send-daily-performance-platform-stats': {
             'task': 'send-daily-performance-platform-stats',
             'schedule': crontab(minute=0, hour=2),

app/dao/inbound_sms_dao.py

@@ -1,6 +1,13 @@
+from datetime import (
+    timedelta,
+    datetime
+)
+
+
 from app import db
 from app.dao.dao_utils import transactional
 from app.models import InboundSms
+from app.statsd_decorators import statsd
 
 
 @transactional
@@ -28,3 +35,15 @@ def dao_count_inbound_sms_for_service(service_id):
     return InboundSms.query.filter(
         InboundSms.service_id == service_id
     ).count()
+
+
+@statsd(namespace="dao")
+@transactional
+def delete_inbound_sms_created_more_than_a_week_ago():
+    seven_days_ago = datetime.utcnow() - timedelta(days=7)
+
+    deleted = db.session.query(InboundSms).filter(
+        InboundSms.created_at < seven_days_ago
+    ).delete(synchronize_session='fetch')
+
+    return deleted

app/dao/notifications_dao.py

@@ -2,7 +2,8 @@ import functools
 from datetime import (
     datetime,
     timedelta,
-    date)
+    date
+)
 
 from flask import current_app
 
@@ -26,6 +27,7 @@ from app.models import (
     NotificationHistory,
     NotificationStatistics,
     Template,
+    ScheduledNotification,
     NOTIFICATION_CREATED,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_SENDING,
@@ -35,7 +37,8 @@ from app.models import (
     NOTIFICATION_PERMANENT_FAILURE,
     KEY_TYPE_NORMAL, KEY_TYPE_TEST,
     LETTER_TYPE,
-    NOTIFICATION_SENT, ScheduledNotification)
+    NOTIFICATION_SENT,
+)
 
 from app.dao.dao_utils import transactional
 from app.statsd_decorators import statsd

tests/app/celery/test_scheduled_tasks.py

@@ -5,19 +5,23 @@ from functools import partial
 
 from flask import current_app
 from freezegun import freeze_time
-from app.celery.scheduled_tasks import s3, timeout_job_statistics, delete_sms_notifications_older_than_seven_days, \
-    delete_letter_notifications_older_than_seven_days, delete_email_notifications_older_than_seven_days, \
-    send_scheduled_notifications
 from app.celery import scheduled_tasks
 from app.celery.scheduled_tasks import (
+    delete_email_notifications_older_than_seven_days,
+    delete_inbound_sms_older_than_seven_days,
+    delete_invitations,
+    delete_notifications_created_more_than_a_week_ago_by_type,
+    delete_letter_notifications_older_than_seven_days,
+    delete_sms_notifications_older_than_seven_days,
     delete_verify_codes,
     remove_csv_files,
-    delete_notifications_created_more_than_a_week_ago_by_type,
-    delete_invitations,
-    timeout_notifications,
     run_scheduled_jobs,
+    s3,
     send_daily_performance_platform_stats,
-    switch_current_sms_provider_on_slow_delivery
+    send_scheduled_notifications,
+    switch_current_sms_provider_on_slow_delivery,
+    timeout_job_statistics,
+    timeout_notifications
 )
 from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient
 from app.dao.jobs_dao import dao_get_job_by_id
@@ -71,7 +75,8 @@ def prepare_current_provider(restore_provider_details):
 
 def test_should_have_decorated_tasks_functions():
     assert delete_verify_codes.__wrapped__.__name__ == 'delete_verify_codes'
-    assert delete_notifications_created_more_than_a_week_ago_by_type.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago_by_type'  # noqa
+    assert delete_notifications_created_more_than_a_week_ago_by_type.__wrapped__.__name__ == \
+        'delete_notifications_created_more_than_a_week_ago_by_type'
     assert timeout_notifications.__wrapped__.__name__ == 'timeout_notifications'
     assert delete_invitations.__wrapped__.__name__ == 'delete_invitations'
     assert run_scheduled_jobs.__wrapped__.__name__ == 'run_scheduled_jobs'
@@ -79,6 +84,8 @@ def test_should_have_decorated_tasks_functions():
     assert send_daily_performance_platform_stats.__wrapped__.__name__ == 'send_daily_performance_platform_stats'
     assert switch_current_sms_provider_on_slow_delivery.__wrapped__.__name__ == \
         'switch_current_sms_provider_on_slow_delivery'
+    assert delete_inbound_sms_older_than_seven_days.__wrapped__.__name__ == \
+        'delete_inbound_sms_older_than_seven_days'
 
 
 def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api, mocker):
@@ -440,3 +447,9 @@ def test_timeout_job_statistics_called_with_notification_timeout(notify_api, moc
     dao_mock = mocker.patch('app.celery.scheduled_tasks.dao_timeout_job_statistics')
     timeout_job_statistics()
     dao_mock.assert_called_once_with(999)
+
+
+def test_should_call_delete_inbound_sms_older_than_seven_days(notify_api, mocker):
+    mocker.patch('app.celery.scheduled_tasks.delete_inbound_sms_created_more_than_a_week_ago')
+    delete_inbound_sms_older_than_seven_days()
+    assert scheduled_tasks.delete_inbound_sms_created_more_than_a_week_ago.call_count == 1

tests/app/dao/test_inbound_sms_dao.py

@@ -1,11 +1,16 @@
-from datetime import datetime
+from datetime import datetime, timedelta
 
 from freezegun import freeze_time
 
-from app.dao.inbound_sms_dao import dao_get_inbound_sms_for_service, dao_count_inbound_sms_for_service
-
+from app.dao.inbound_sms_dao import (
+    dao_get_inbound_sms_for_service,
+    dao_count_inbound_sms_for_service,
+    delete_inbound_sms_created_more_than_a_week_ago
+)
 from tests.app.db import create_inbound_sms, create_service
 
+from app.models import InboundSms
+
 
 def test_get_all_inbound_sms(sample_service):
     inbound = create_inbound_sms(sample_service)
@@ -57,3 +62,27 @@ def test_count_inbound_sms_for_service(notify_db_session):
     create_inbound_sms(service_two)
 
     assert dao_count_inbound_sms_for_service(service_one.id) == 2
+
+
+@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)
+    create_inbound_sms(sample_service, created_at=older_than_seven_days)
+    delete_inbound_sms_created_more_than_a_week_ago()
+
+    assert len(InboundSms.query.all()) == 0
+
+
+@freeze_time("2017-01-01 12:00:00")
+def test_should_not_delete_inbound_sms_before_seven_days(sample_service):
+    yesterday = datetime.utcnow() - timedelta(days=1)
+    just_before_seven_days = datetime.utcnow() - timedelta(days=6, hours=23, minutes=59, seconds=59)
+    older_than_seven_days = datetime.utcnow() - timedelta(days=7, seconds=1)
+
+    create_inbound_sms(sample_service, created_at=yesterday)
+    create_inbound_sms(sample_service, created_at=just_before_seven_days)
+    create_inbound_sms(sample_service, created_at=older_than_seven_days)
+
+    delete_inbound_sms_created_more_than_a_week_ago()
+
+    assert len(InboundSms.query.all()) == 2

tests/app/db.py

@@ -2,6 +2,7 @@ from datetime import datetime
 import uuid
 
 
+from app.dao.inbound_sms_dao import dao_create_inbound_sms
 from app.dao.jobs_dao import dao_create_job
 from app.models import (
     InboundSms,
@@ -12,6 +13,7 @@ from app.models import (
     ScheduledNotification,
     ServicePermission,
     Job,
+    InboundSms,
     EMAIL_TYPE,
     SMS_TYPE,
     KEY_TYPE_NORMAL,
@@ -151,14 +153,15 @@ def create_notification(
     return notification
 
 
-def create_job(template,
+def create_job(
+    template,
     notification_count=1,
     created_at=None,
     job_status='pending',
     scheduled_for=None,
     processing_started=None,
-               original_file_name='some.csv'):
-
+    original_file_name='some.csv'
+):
     data = {
         'id': uuid.uuid4(),
         'service_id': template.service_id,
@@ -193,11 +196,12 @@ def create_inbound_sms(
     user_number='447700900111',
     provider_date=None,
     provider_reference=None,
-    content='Hello'
+    content='Hello',
+    created_at=None
 ):
     inbound = InboundSms(
         service=service,
-        created_at=datetime.utcnow(),
+        created_at=created_at or datetime.utcnow(),
         notify_number=notify_number or service.sms_sender,
         user_number=user_number,
         provider_date=provider_date or datetime.utcnow(),