From db0d45966f929be1277cd82401d426cb6c64f97a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 15 Nov 2019 10:23:48 +0000 Subject: [PATCH] Fix query to populate ft_billing table. The group by for the query was wrong which would result in 2 rows with different totals but the same unique key, so the second row would update the first row. Meaning we had incorrect numbers for the billing data. Because some of the data had null for the sent_by column, the select would turn the Null --> dvla, but that same function was not used in the group by. So any time we had missing sent_by data we would end up with 2 rows where one would overwrite the other. --- app/dao/fact_billing_dao.py | 25 ++++++++++--------------- tests/app/dao/test_ft_billing_dao.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index f94f3c4c7..020900c95 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -336,25 +336,20 @@ def _query_for_billing_data(table, notification_type, start_date, end_date, serv EMAIL_TYPE: NOTIFICATION_STATUS_TYPES_BILLABLE, LETTER_TYPE: NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS } + sent_by_func = func.coalesce(table.sent_by, case( + [(table.notification_type == 'letter', 'dvla'), + (table.notification_type == 'sms', 'unknown'), + (table.notification_type == 'email', 'ses')]), ) + letter_page_count = case([(table.notification_type == 'letter', table.billable_units), ]) + query = db.session.query( table.template_id, table.service_id, table.notification_type, - func.coalesce(table.sent_by, - case( - [ - (table.notification_type == 'letter', 'dvla'), - (table.notification_type == 'sms', 'unknown'), - (table.notification_type == 'email', 'ses') - ]), - ).label('sent_by'), + sent_by_func.label('sent_by'), func.coalesce(table.rate_multiplier, 1).cast(Integer).label('rate_multiplier'), func.coalesce(table.international, False).label('international'), - case( - [ - (table.notification_type == 'letter', table.billable_units), - ] - ).label('letter_page_count'), + letter_page_count.label('letter_page_count'), func.sum(table.billable_units).label('billable_units'), func.count().label('notifications_sent'), Service.crown, @@ -370,8 +365,8 @@ def _query_for_billing_data(table, notification_type, start_date, end_date, serv table.template_id, table.service_id, table.notification_type, - 'sent_by', - 'letter_page_count', + sent_by_func, + letter_page_count, table.rate_multiplier, table.international, Service.crown, diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 800da2fa5..7c9b94439 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -221,6 +221,34 @@ def test_fetch_billing_data_for_day_groups_by_postage(notify_db_session): assert len(results) == 3 +def test_fetch_billing_data_for_day_groups_by_sent_by(notify_db_session): + service = create_service() + letter_template = create_template(service=service, template_type='letter') + email_template = create_template(service=service, template_type='email') + create_notification(template=letter_template, status='delivered', postage='second', sent_by='dvla') + create_notification(template=letter_template, status='delivered', postage='second', sent_by='dvla') + create_notification(template=letter_template, status='delivered', postage='second', sent_by=None) + create_notification(template=email_template, status='delivered') + + today = convert_utc_to_bst(datetime.utcnow()) + results = fetch_billing_data_for_day(today) + assert len(results) == 2 + + +def test_fetch_billing_data_for_day_groups_by_page_count(notify_db_session): + service = create_service() + letter_template = create_template(service=service, template_type='letter') + email_template = create_template(service=service, template_type='email') + create_notification(template=letter_template, status='delivered', postage='second', billable_units=1) + create_notification(template=letter_template, status='delivered', postage='second', billable_units=1) + create_notification(template=letter_template, status='delivered', postage='second', billable_units=2) + create_notification(template=email_template, status='delivered') + + today = convert_utc_to_bst(datetime.utcnow()) + results = fetch_billing_data_for_day(today) + assert len(results) == 3 + + def test_fetch_billing_data_for_day_sets_postage_for_emails_and_sms_to_none(notify_db_session): service = create_service() sms_template = create_template(service=service, template_type='sms')