From a826f6e924cba1577d35123e2b3b2b6977a47fb8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 26 Jul 2018 18:41:06 +0100 Subject: [PATCH] make rebuild-ft-billing-for-day remove unused rows if the billing data was incorrect and needs to be rebuilt, we should remove old rows. Previously we were only upserting new rows, but old no-longer-relevant rows were staying in the database. This commit makes the flask command remove *all* rows for that service and day, before inserting the rows from the original notification data after. This commit doesn't change the existing nightly task, nor does it change the upsert that happens upon viewing the usage page. In normal usage, there should never be a case where the number of billable units for a rate decreases. It should only ever increase, thus, never need to be deleted --- app/commands.py | 29 ++++++++++++++++++++++++---- app/dao/fact_billing_dao.py | 12 ++++++++++++ tests/app/dao/test_ft_billing_dao.py | 29 ++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/app/commands.py b/app/commands.py index 13fb5c4ed..982fbdc97 100644 --- a/app/commands.py +++ b/app/commands.py @@ -19,7 +19,11 @@ from app.celery.service_callback_tasks import send_delivery_status_to_service from app.celery.letters_pdf_tasks import create_letters_pdf from app.config import QueueNames from app.dao.date_util import get_financial_year -from app.dao.fact_billing_dao import fetch_billing_data_for_day, update_fact_billing +from app.dao.fact_billing_dao import ( + fetch_billing_data_for_day, + update_fact_billing, + delete_billing_data_for_service_for_day +) from app.dao.monthly_billing_dao import ( create_or_update_monthly_billing, get_monthly_billing_by_notification_type, @@ -526,17 +530,34 @@ def rebuild_ft_billing_for_day(service_id, day): Rebuild the data in ft_billing for the given service_id and date """ def rebuild_ft_data(process_day, service): + deleted_rows = delete_billing_data_for_service_for_day(process_day, service) + current_app.logger.info('deleted {} existing billing rows for {} on {}'.format( + deleted_rows, + service, + process_day + )) transit_data = fetch_billing_data_for_day(process_day=process_day, service_id=service) + # transit_data = every row that should exist for data in transit_data: + # upsert existing rows update_fact_billing(data, process_day) + current_app.logger.info('added/updated {} billing rows for {} on {}'.format( + len(transit_data), + service, + process_day + )) + if service_id: # confirm the service exists dao_fetch_service_by_id(service_id) rebuild_ft_data(day, service_id) else: - services = get_service_ids_that_need_billing_populated(day, day) - for service_id in services: - rebuild_ft_data(day, service_id) + services = get_service_ids_that_need_billing_populated( + get_london_midnight_in_utc(day), + get_london_midnight_in_utc(day + timedelta(days=1)) + ) + for row in services: + rebuild_ft_data(day, row.service_id) @notify_command(name='compare-ft-billing-to-monthly-billing') diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index db5758d69..0552fd032 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -125,6 +125,18 @@ def fetch_monthly_billing_for_year(service_id, year): return yearly_data +def delete_billing_data_for_service_for_day(process_day, service_id): + """ + Delete all ft_billing data for a given service on a given bst_date + + Returns how many rows were deleted + """ + return FactBilling.query.filter( + FactBilling.bst_date == process_day, + FactBilling.service_id == service_id + ).delete() + + def fetch_billing_data_for_day(process_day, service_id=None): start_date = convert_bst_to_utc(datetime.combine(process_day, time.min)) end_date = convert_bst_to_utc(datetime.combine(process_day + timedelta(days=1), time.min)) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index d286da29f..6a21732f1 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -6,9 +6,12 @@ from freezegun import freeze_time from app import db from app.dao.fact_billing_dao import ( - fetch_monthly_billing_for_year, fetch_billing_data_for_day, get_rates_for_billing, - get_rate, + delete_billing_data_for_service_for_day, + fetch_billing_data_for_day, fetch_billing_totals_for_year, + fetch_monthly_billing_for_year, + get_rate, + get_rates_for_billing, ) from app.models import FactBilling, Notification from app.utils import convert_utc_to_bst @@ -353,3 +356,25 @@ def test_fetch_billing_totals_for_year(notify_db_session): assert results[3].notifications_sent == 365 assert results[3].billable_units == 365 assert results[3].rate == Decimal('0.162') + + +def test_delete_billing_data(notify_db_session): + service_1 = create_service(service_name='1') + service_2 = create_service(service_name='2') + sms_template = create_template(service_1, 'sms') + email_template = create_template(service_1, 'email') + other_service_template = create_template(service_2, 'sms') + + existing_rows_to_delete = [ # noqa + create_ft_billing('2018-01-01', 'sms', sms_template, service_1, billable_unit=1), + create_ft_billing('2018-01-01', 'email', email_template, service_1, billable_unit=2) + ] + other_day = create_ft_billing('2018-01-02', 'sms', sms_template, service_1, billable_unit=3) + other_service = create_ft_billing('2018-01-01', 'sms', other_service_template, service_2, billable_unit=4) + + delete_billing_data_for_service_for_day('2018-01-01', service_1.id) + + current_rows = FactBilling.query.all() + assert sorted(x.billable_units for x in current_rows) == sorted( + [other_day.billable_units, other_service.billable_units] + )