Integrate inbound history insert into delete inbound sms function

app/dao/inbound_sms_dao.py

@@ -63,23 +63,7 @@ def dao_count_inbound_sms_for_service(service_id, limit_days):
     ).count()
 
 
-def _delete_inbound_sms(datetime_to_delete_from, query_filter):
+def _insert_update_notification_history(subquery, query_limit=10000):
-    query_limit = 10000
-
-    subquery = db.session.query(
-        InboundSms.id
-    ).filter(
-        InboundSms.created_at < datetime_to_delete_from,
-        *query_filter
-    ).limit(
-        query_limit
-    ).subquery()
-
-    deleted = 0
-    # set to nonzero just to enter the loop
-    number_deleted = 1
-    while number_deleted > 0:
-
     offset = 0
     inbound_sms_query = db.session.query(
         *[x.name for x in InboundSmsHistory.__table__.c]
@@ -106,6 +90,26 @@ def _delete_inbound_sms(datetime_to_delete_from, query_filter):
         db.session.connection().execute(statement)
 
         offset += query_limit
+
+
+def _delete_inbound_sms(datetime_to_delete_from, query_filter):
+    query_limit = 10000
+
+    subquery = db.session.query(
+        InboundSms.id
+    ).filter(
+        InboundSms.created_at < datetime_to_delete_from,
+        *query_filter
+    ).limit(
+        query_limit
+    ).subquery()
+
+    deleted = 0
+    # set to nonzero just to enter the loop
+    number_deleted = 1
+    while number_deleted > 0:
+        _insert_update_notification_history(subquery, query_limit=query_limit)
+
         number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch')
         deleted += number_deleted
 
@@ -123,6 +127,7 @@ def delete_inbound_sms_older_than_retention():
     ).filter(
         ServiceDataRetention.notification_type == SMS_TYPE
     ).all()
+
     deleted = 0
 
     for f in flexible_data_retention:
@@ -138,42 +143,12 @@ def delete_inbound_sms_older_than_retention():
     deleted += _delete_inbound_sms(seven_days_ago, query_filter=[
         InboundSms.service_id.notin_(x.service_id for x in flexible_data_retention),
     ])
+
     current_app.logger.info('Deleted {} inbound sms'.format(deleted))
+
     return deleted
 
 
-def insert_update_inbound_sms_history(date_to_delete_from, service_id, query_limit=10000):
-    offset = 0
-    inbound_sms_query = db.session.query(
-        *[x.name for x in InboundSmsHistory.__table__.c]
-    ).filter(
-        InboundSms.service_id == service_id,
-        InboundSms.created_at < date_to_delete_from,
-    )
-    inbound_sms_count = inbound_sms_query.count()
-
-    while offset < inbound_sms_count:
-        statement = insert(InboundSmsHistory).from_select(
-            InboundSmsHistory.__table__.c,
-            inbound_sms_query.limit(query_limit).offset(offset)
-        )
-
-        statement = statement.on_conflict_do_update(
-            constraint="inbound_sms_history_pkey",
-            set_={
-                "created_at": statement.excluded.created_at,
-                "service_id": statement.excluded.service_id,
-                "notify_number": statement.excluded.notify_number,
-                "provider_date": statement.excluded.provider_date,
-                "provider_reference": statement.excluded.provider_reference,
-                "provider": statement.excluded.provider
-            }
-        )
-        db.session.connection().execute(statement)
-
-        offset += query_limit
-
-
 def dao_get_inbound_sms_by_id(service_id, inbound_id):
     return InboundSms.query.filter_by(
         id=inbound_id,

tests/app/dao/test_inbound_sms_dao.py

@@ -10,14 +10,10 @@ from app.dao.inbound_sms_dao import (
     dao_get_inbound_sms_by_id,
     dao_get_paginated_inbound_sms_for_service_for_public_api,
     dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service,
-    db,
-    insert_update_inbound_sms_history
 )
 
 from app.models import InboundSmsHistory
 
-from app.utils import midnight_n_days_ago
-
 from tests.conftest import set_config
 from tests.app.db import create_inbound_sms, create_service, create_service_data_retention
 
@@ -281,65 +277,3 @@ def test_most_recent_inbound_sms_only_returns_values_within_7_days(sample_servic
 
     assert len(res.items) == 1
     assert res.items[0].content == 'new'
-
-
-def test_insert_update_inbound_sms_history_limits_by_query_by_date(sample_service):
-    # becoming history
-    inbound_1 = create_inbound_sms(
-        sample_service, user_number='1', content='old', created_at=datetime(2017, 4, 2, 22, 59, 59)
-    )
-    # not becoming history yet
-    create_inbound_sms(sample_service, user_number='2', content='new', created_at=datetime(2017, 4, 2, 23, 0, 0))
-
-    with freeze_time('Monday 10th April 2017 12:00:00'):
-        insert_update_inbound_sms_history(
-            date_to_delete_from=midnight_n_days_ago(7),
-            service_id=sample_service.id)
-    history = InboundSmsHistory.query.all()
-    assert len(history) == 1
-
-    assert history[0].id == inbound_1.id
-
-
-def test_insert_update_inbound_sms_history_above_query_limit(sample_service, mocker):
-    # becoming history
-    create_inbound_sms(
-        sample_service, user_number='1', content='old', created_at=datetime(2017, 4, 2, 22, 59, 59)
-    )
-    create_inbound_sms(
-        sample_service, user_number='1', content='old', created_at=datetime(2017, 4, 2, 20, 59, 59)
-    )
-    # not becoming history yet
-    create_inbound_sms(sample_service, user_number='2', content='new', created_at=datetime(2017, 4, 2, 23, 0, 0))
-
-    db_connection_spy = mocker.spy(db.session, 'connection')
-    db_commit_spy = mocker.spy(db.session, 'commit')
-
-    with freeze_time('Monday 10th April 2017 12:00:00'):
-        insert_update_inbound_sms_history(
-            date_to_delete_from=midnight_n_days_ago(7),
-            service_id=sample_service.id,
-            query_limit=1
-        )
-    history = InboundSmsHistory.query.all()
-    assert len(history) == 2
-
-    assert db_connection_spy.call_count == 2
-    assert db_commit_spy.call_count == 2
-
-
-def test_insert_update_inbound_sms_history_only_updates_given_service(sample_service):
-    inbound_1 = create_inbound_sms(
-        sample_service, user_number='1', content='old', created_at=datetime(2017, 4, 2, 22, 59, 59)
-    )
-    other_service = create_service(service_name='another service')
-    create_inbound_sms(other_service, user_number='2', content='new', created_at=datetime(2017, 4, 2, 22, 0, 0))
-
-    with freeze_time('Monday 10th April 2017 12:00:00'):
-        insert_update_inbound_sms_history(
-            date_to_delete_from=midnight_n_days_ago(7),
-            service_id=sample_service.id)
-    history = InboundSmsHistory.query.all()
-    assert len(history) == 1
-
-    assert history[0].id == inbound_1.id