Populate inbound sms history when deleting inbound sms

This commit is contained in:
Pea Tyczynska
2019-12-17 18:04:22 +00:00
parent de9d1a41eb
commit a6b4675ae7
2 changed files with 136 additions and 7 deletions

View File

@@ -2,10 +2,11 @@ from flask import current_app
from notifications_utils.statsd_decorators import statsd from notifications_utils.statsd_decorators import statsd
from sqlalchemy import desc, and_ from sqlalchemy import desc, and_
from sqlalchemy.orm import aliased from sqlalchemy.orm import aliased
from sqlalchemy.dialects.postgresql import insert
from app import db from app import db
from app.dao.dao_utils import transactional 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 from app.utils import midnight_n_days_ago
@@ -78,6 +79,33 @@ def _delete_inbound_sms(datetime_to_delete_from, query_filter):
# set to nonzero just to enter the loop # set to nonzero just to enter the loop
number_deleted = 1 number_deleted = 1
while number_deleted > 0: while number_deleted > 0:
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_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
number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch') number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch')
deleted += number_deleted deleted += number_deleted
@@ -95,8 +123,8 @@ def delete_inbound_sms_older_than_retention():
).filter( ).filter(
ServiceDataRetention.notification_type == SMS_TYPE ServiceDataRetention.notification_type == SMS_TYPE
).all() ).all()
deleted = 0 deleted = 0
for f in flexible_data_retention: for f in flexible_data_retention:
n_days_ago = midnight_n_days_ago(f.days_of_retention) n_days_ago = midnight_n_days_ago(f.days_of_retention)
@@ -110,12 +138,42 @@ def delete_inbound_sms_older_than_retention():
deleted += _delete_inbound_sms(seven_days_ago, query_filter=[ deleted += _delete_inbound_sms(seven_days_ago, query_filter=[
InboundSms.service_id.notin_(x.service_id for x in flexible_data_retention), InboundSms.service_id.notin_(x.service_id for x in flexible_data_retention),
]) ])
current_app.logger.info('Deleted {} inbound sms'.format(deleted)) current_app.logger.info('Deleted {} inbound sms'.format(deleted))
return 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): def dao_get_inbound_sms_by_id(service_id, inbound_id):
return InboundSms.query.filter_by( return InboundSms.query.filter_by(
id=inbound_id, id=inbound_id,

View File

@@ -9,8 +9,15 @@ from app.dao.inbound_sms_dao import (
delete_inbound_sms_older_than_retention, delete_inbound_sms_older_than_retention,
dao_get_inbound_sms_by_id, dao_get_inbound_sms_by_id,
dao_get_paginated_inbound_sms_for_service_for_public_api, 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,
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.conftest import set_config
from tests.app.db import create_inbound_sms, create_service, create_service_data_retention from tests.app.db import create_inbound_sms, create_service, create_service_data_retention
@@ -90,7 +97,7 @@ def test_count_inbound_sms_for_service_filters_messages_older_than_n_days(sample
@freeze_time("2017-06-08 12:00:00") @freeze_time("2017-06-08 12:00:00")
def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session): def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session, mocker):
no_retention_service = create_service(service_name='no retention') no_retention_service = create_service(service_name='no retention')
short_retention_service = create_service(service_name='three days') short_retention_service = create_service(service_name='three days')
long_retention_service = create_service(service_name='thirty days') long_retention_service = create_service(service_name='thirty days')
@@ -99,7 +106,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(long_retention_service, notification_type='sms', days_of_retention=30)
create_service_data_retention(short_retention_service, notification_type='sms', days_of_retention=3) 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) create_service_data_retention(short_retention_service, notification_type='email', days_of_retention=4)
dates = [ dates = [
@@ -115,6 +121,9 @@ def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session
deleted_count = delete_inbound_sms_older_than_retention() 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 # four deleted for the 3-day service, two for the default seven days one, one for the 30 day
assert deleted_count == 7 assert deleted_count == 7
assert { assert {
@@ -272,3 +281,65 @@ def test_most_recent_inbound_sms_only_returns_values_within_7_days(sample_servic
assert len(res.items) == 1 assert len(res.items) == 1
assert res.items[0].content == 'new' 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