Merge pull request #2687 from alphagov/inbound-sms-history

Add and write into Inbound sms history

app/dao/inbound_sms_dao.py

@@ -2,10 +2,11 @@ from flask import current_app
 from notifications_utils.statsd_decorators import statsd
 from sqlalchemy import desc, and_
 from sqlalchemy.orm import aliased
+from sqlalchemy.dialects.postgresql import insert
 
 from app import db
 from app.dao.dao_utils import transactional
-from app.models import InboundSms, Service, ServiceDataRetention, SMS_TYPE
+from app.models import InboundSms, InboundSmsHistory, Service, ServiceDataRetention, SMS_TYPE
 from app.utils import midnight_n_days_ago
 
 
@@ -62,6 +63,27 @@ def dao_count_inbound_sms_for_service(service_id, limit_days):
     ).count()
 
 
+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]
+    ).filter(InboundSms.id.in_(subquery))
+    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_nothing(
+            constraint="inbound_sms_history_pkey"
+        )
+        db.session.connection().execute(statement)
+
+        offset += query_limit
+
+
 def _delete_inbound_sms(datetime_to_delete_from, query_filter):
     query_limit = 10000
 
@@ -78,6 +100,8 @@ 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_inbound_sms_history(subquery, query_limit=query_limit)
+
         number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch')
         deleted += number_deleted
 
@@ -97,6 +121,7 @@ def delete_inbound_sms_older_than_retention():
     ).all()
 
     deleted = 0
+
     for f in flexible_data_retention:
         n_days_ago = midnight_n_days_ago(f.days_of_retention)
 

app/models.py

@@ -1898,6 +1898,18 @@ class InboundSms(db.Model):
         }
 
 
+class InboundSmsHistory(db.Model, HistoryModel):
+    __tablename__ = 'inbound_sms_history'
+    id = db.Column(UUID(as_uuid=True), primary_key=True)
+    created_at = db.Column(db.DateTime, index=True, unique=False, nullable=False)
+    service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False)
+    service = db.relationship('Service')
+    notify_number = db.Column(db.String, nullable=False)
+    provider_date = db.Column(db.DateTime)
+    provider_reference = db.Column(db.String)
+    provider = db.Column(db.String, nullable=False)
+
+
 class LetterRate(db.Model):
     __tablename__ = 'letter_rates'
 

migrations/versions/0311_add_inbound_sms_history.py

@@ -0,0 +1,33 @@
+"""
+
+Revision ID: 0311_add_inbound_sms_history
+Revises: 0310_returned_letters_table
+Create Date: 2019-12-20 15:38:53.358509
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+revision = '0311_add_inbound_sms_history'
+down_revision = '0310_returned_letters_table'
+
+
+def upgrade():
+    op.create_table('inbound_sms_history',
+    sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False),
+    sa.Column('created_at', sa.DateTime(), nullable=False),
+    sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=True),
+    sa.Column('notify_number', sa.String(), nullable=False),
+    sa.Column('provider_date', sa.DateTime(), nullable=True),
+    sa.Column('provider_reference', sa.String(), nullable=True),
+    sa.Column('provider', sa.String(), nullable=False),
+    sa.ForeignKeyConstraint(['service_id'], ['services.id'], ),
+    sa.PrimaryKeyConstraint('id')
+    )
+    op.create_index(op.f('ix_inbound_sms_history_service_id'), 'inbound_sms_history', ['service_id'], unique=False)
+
+
+def downgrade():
+    op.drop_index(op.f('ix_inbound_sms_history_service_id'), table_name='inbound_sms_history')
+    op.drop_table('inbound_sms_history')

tests/app/dao/test_inbound_sms_dao.py

@@ -9,8 +9,12 @@ from app.dao.inbound_sms_dao import (
     delete_inbound_sms_older_than_retention,
     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
+    dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service,
 )
+
+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
 
@@ -99,7 +103,6 @@ def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session
 
     create_service_data_retention(long_retention_service, notification_type='sms', days_of_retention=30)
     create_service_data_retention(short_retention_service, notification_type='sms', days_of_retention=3)
-    # email retention doesn't affect anything
     create_service_data_retention(short_retention_service, notification_type='email', days_of_retention=4)
 
     dates = [
@@ -115,6 +118,9 @@ def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session
 
     deleted_count = delete_inbound_sms_older_than_retention()
 
+    history = InboundSmsHistory.query.all()
+    assert len(history) == 7
+
     # four deleted for the 3-day service, two for the default seven days one, one for the 30 day
     assert deleted_count == 7
     assert {
@@ -128,6 +134,75 @@ def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session
     } == set(dates[:4])
 
 
+@freeze_time("2019-12-20 12:00:00")
+def test_insert_into_inbound_sms_history_when_deleting_inbound_sms(sample_service):
+    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'
+    )
+    create_inbound_sms(sample_service, created_at=datetime(2019, 12, 19, 20, 19))
+
+    delete_inbound_sms_older_than_retention()
+    history = InboundSmsHistory.query.all()
+    assert len(history) == 1
+
+    for key_name in [
+        'provider', 'provider_date', 'service_id', 'created_at', 'provider_reference', 'notify_number', 'id'
+    ]:
+        assert key_name in vars(history[0])
+
+    for key_name in ['content', 'user_number']:
+        assert key_name not in vars(history[0])
+
+    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)
+
+
+@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)