Do nothing on db conflict

For notification and notification_history we do an upsert. Here, as the
inbound_sms table is never updated, only inserted to once (signified by
lack of updated_at field), an upsert would be unnecessary.

Therefore, if for some reason the delete statement failed as part of
moving data into the inbound_sms_history table, we can simply just
ignore any db conflicts raised by a rerun of
`delete_inbound_sms_older_than_retention`.

app/dao/inbound_sms_dao.py

@@ -63,7 +63,7 @@ def dao_count_inbound_sms_for_service(service_id, limit_days):
     ).count()
 
 
-def _insert_update_inbound_sms_history(subquery, query_limit=10000):
+def _insert_inbound_sms_history(subquery, query_limit=10000):
     offset = 0
     inbound_sms_query = db.session.query(
         *[x.name for x in InboundSmsHistory.__table__.c]
@@ -76,15 +76,8 @@ def _insert_update_inbound_sms_history(subquery, query_limit=10000):
             inbound_sms_query.limit(query_limit).offset(offset)
         )
 
-        statement = statement.on_conflict_do_update(
+        statement = statement.on_conflict_do_nothing(
-            constraint="inbound_sms_history_pkey",
+            constraint="inbound_sms_history_pkey"
-            set_={
-                "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)
 
@@ -107,7 +100,7 @@ def _delete_inbound_sms(datetime_to_delete_from, query_filter):
     # set to nonzero just to enter the loop
     number_deleted = 1
     while number_deleted > 0:
-        _insert_update_inbound_sms_history(subquery, query_limit=query_limit)
+        _insert_inbound_sms_history(subquery, query_limit=query_limit)
 
         number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch')
         deleted += number_deleted

tests/app/dao/test_inbound_sms_dao.py

@@ -13,6 +13,7 @@ from app.dao.inbound_sms_dao import (
 )
 
 from app.models import InboundSmsHistory
+from app import db
 
 from tests.conftest import set_config
 from tests.app.db import create_inbound_sms, create_service, create_service_data_retention
@@ -163,6 +164,45 @@ def test_insert_into_inbound_sms_history_when_deleting_inbound_sms(sample_servic
     assert history[0].created_at == datetime(2019, 12, 12, 20, 20)
 
 
+@freeze_time("2019-12-20 12:00:00")
+def test_delete_inbound_sms_older_than_retention_does_nothing_when_database_conflict_raised(sample_service):
+    inbound_sms = create_inbound_sms(
+        sample_service, created_at=datetime(2019, 12, 12, 20, 20),
+        notify_number='07700900100',
+        provider_date=datetime(2019, 12, 12, 20, 19),
+        provider_reference='from daisy pie',
+        provider='unicorn'
+    )
+    inbound_sms_id = inbound_sms.id
+
+    # Insert data directly in to inbound_sms_history to mimic if we had run `delete_inbound_sms_older_than_retention`
+    # before but for some reason the delete statement had failed
+    conflict_creating_row = InboundSmsHistory(
+        id=inbound_sms.id,
+        service_id=inbound_sms.service.id,
+        created_at=inbound_sms.created_at,
+        notify_number=inbound_sms.notify_number,
+        provider_date=inbound_sms.provider_date,
+        provider_reference=inbound_sms.provider_reference,
+        provider=inbound_sms.provider,
+    )
+    db.session.add(conflict_creating_row)
+    db.session.commit()
+    assert conflict_creating_row.id
+
+    delete_inbound_sms_older_than_retention()
+
+    history = InboundSmsHistory.query.all()
+    assert len(history) == 1
+
+    assert history[0].id == inbound_sms_id
+    assert history[0].notify_number == '07700900100'
+    assert history[0].provider_date == datetime(2019, 12, 12, 20, 19)
+    assert history[0].provider_reference == 'from daisy pie'
+    assert history[0].provider == 'unicorn'
+    assert history[0].created_at == datetime(2019, 12, 12, 20, 20)
+
+
 def test_get_inbound_sms_by_id_returns(sample_service):
     inbound_sms = create_inbound_sms(service=sample_service)
     inbound_from_db = dao_get_inbound_sms_by_id(inbound_sms.service.id, inbound_sms.id)