Fix not deleting dead rows in status table

To address: https://github.com/alphagov/notifications-api/pull/3454#pullrequestreview-880302729

Since we now delete all rows before inserting fresh ones, we no
longer need to worry about conflicts. I've also extended the old
test to check all three kinds of overwrite: new, changed, gone.
This commit is contained in:
Ben Thorner
2022-02-16 12:30:41 +00:00
parent a69d1635a1
commit 574b1d3c63
2 changed files with 33 additions and 27 deletions

View File

@@ -39,6 +39,13 @@ def update_fact_notification_status(process_day, notification_type, service_id):
start_date = get_london_midnight_in_utc(process_day) start_date = get_london_midnight_in_utc(process_day)
end_date = get_london_midnight_in_utc(process_day + timedelta(days=1)) end_date = get_london_midnight_in_utc(process_day + timedelta(days=1))
# delete any existing rows in case some no longer exist e.g. if all messages are sent
FactNotificationStatus.query.filter(
FactNotificationStatus.bst_date == process_day,
FactNotificationStatus.notification_type == notification_type,
FactNotificationStatus.service_id == service_id,
).delete()
# query notifications or notification_history for the day, depending on their data retention # query notifications or notification_history for the day, depending on their data retention
service = Service.query.get(service_id) service = Service.query.get(service_id)
source_table = get_notification_table_to_use( source_table = get_notification_table_to_use(
@@ -71,7 +78,8 @@ def update_fact_notification_status(process_day, notification_type, service_id):
source_table.status source_table.status
) )
stmt = insert(FactNotificationStatus.__table__).from_select( db.session.connection().execute(
insert(FactNotificationStatus.__table__).from_select(
[ [
FactNotificationStatus.bst_date, FactNotificationStatus.bst_date,
FactNotificationStatus.template_id, FactNotificationStatus.template_id,
@@ -84,17 +92,8 @@ def update_fact_notification_status(process_day, notification_type, service_id):
], ],
query query
) )
stmt = stmt.on_conflict_do_update(
constraint="ft_notification_status_pkey",
set_={
FactNotificationStatus.notification_count: stmt.excluded.notification_count,
FactNotificationStatus.updated_at: datetime.utcnow()
}
) )
db.session.connection().execute(stmt)
def fetch_notification_status_for_service_by_month(start_date, end_date, service_id): def fetch_notification_status_for_service_by_month(start_date, end_date, service_id):
return db.session.query( return db.session.query(

View File

@@ -599,9 +599,10 @@ def test_create_nightly_notification_status_for_service_and_day(notify_db_sessio
def test_create_nightly_notification_status_for_service_and_day_overwrites_old_data(notify_db_session): def test_create_nightly_notification_status_for_service_and_day_overwrites_old_data(notify_db_session):
first_service = create_service(service_name='First Service') first_service = create_service(service_name='First Service')
first_template = create_template(service=first_service) first_template = create_template(service=first_service)
create_notification(template=first_template, status='delivered')
process_day = date.today() process_day = date.today()
# first run: one notification, expect one row (just one status)
notification = create_notification(template=first_template, status='sending')
create_nightly_notification_status_for_service_and_day(str(process_day), first_service.id, 'sms') create_nightly_notification_status_for_service_and_day(str(process_day), first_service.id, 'sms')
new_fact_data = FactNotificationStatus.query.order_by( new_fact_data = FactNotificationStatus.query.order_by(
@@ -611,8 +612,11 @@ def test_create_nightly_notification_status_for_service_and_day_overwrites_old_d
assert len(new_fact_data) == 1 assert len(new_fact_data) == 1
assert new_fact_data[0].notification_count == 1 assert new_fact_data[0].notification_count == 1
assert new_fact_data[0].notification_status == 'sending'
create_notification(template=first_template, status='delivered') # second run: status changed, still expect one row (one status)
notification.status = 'delivered'
create_notification(template=first_template, status='created')
create_nightly_notification_status_for_service_and_day(str(process_day), first_service.id, 'sms') create_nightly_notification_status_for_service_and_day(str(process_day), first_service.id, 'sms')
updated_fact_data = FactNotificationStatus.query.order_by( updated_fact_data = FactNotificationStatus.query.order_by(
@@ -620,8 +624,11 @@ def test_create_nightly_notification_status_for_service_and_day_overwrites_old_d
FactNotificationStatus.notification_type FactNotificationStatus.notification_type
).all() ).all()
assert len(updated_fact_data) == 1 assert len(updated_fact_data) == 2
assert updated_fact_data[0].notification_count == 2 assert updated_fact_data[0].notification_count == 1
assert updated_fact_data[0].notification_status == 'created'
assert updated_fact_data[1].notification_count == 1
assert updated_fact_data[1].notification_status == 'delivered'
# the job runs at 12:30am London time. 04/01 is in BST. # the job runs at 12:30am London time. 04/01 is in BST.