From cd84928a1ec51b21043b42d859918a042e769fb5 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Wed, 20 Apr 2022 17:14:17 +0100 Subject: [PATCH] Add costs to each row in yearly usage API This will replace the manual calculations in Admin [^1][^2] for SMS and also in API [^3] for annual letter costs. Doing the calculation here also means we correctly attribute free allowance to the earliest rows in the billing table - Admin doesn't know when a given rate was applied so can't do this without making assumptions about when we change our rates. Since the calculation now depends on annual billing, we need to change all the tests to make sure a suitable row exists. I've also adjusted the test data to match the assumption that there can only be one SMS rate per bst_date. Note about "OVER" clause ======================== Using "rows=" ("ROWS BETWEEN") makes more sense than "range=" as we want the remainder to be incremental within each group in a "GROUP BY" clause, as well as between groups i.e # ROWS BETWEEN (arbitrary numbers to illustrate) date=2021-04-03, units=3, cost=3.29 date=2021-04-03, units=2, cost=4.17 date=2021-04-04, units=2, cost=5.10 vs. # RANGE BETWEEN date=2021-04-03, units=3, cost=4.17 date=2021-04-03, units=2, cost=4.17 date=2021-04-04, units=2, cost=5.10 See [^4] for more details and examples. [^1]: https://github.com/alphagov/notifications-admin/blob/master/app/templates/views/usage.html#L60 [^2]: https://github.com/alphagov/notifications-api/blob/072c3b207940597aacb5bebdbf3757f848d22cd6/app/billing/billing_schemas.py#L37 [^3]: https://github.com/alphagov/notifications-admin/blob/474d7dfda834ebf2f0966f176fb6da556808d8a1/app/templates/views/usage.html#L98 [^4]: https://learnsql.com/blog/difference-between-rows-range-window-functions/ --- app/billing/billing_schemas.py | 3 ++ app/dao/fact_billing_dao.py | 63 +++++++++++++++++++++++++++- tests/app/billing/test_rest.py | 11 +++++ tests/app/dao/test_ft_billing_dao.py | 45 +++++++++++++++++++- 4 files changed, 119 insertions(+), 3 deletions(-) diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index e95d59027..d437ff616 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -23,6 +23,7 @@ def serialize_ft_billing_remove_emails(rows): "notifications_sent": row.notifications_sent, "rate": float(row.rate), "postage": row.postage, + "cost": float(row.cost), } for row in rows if row.notification_type != 'email' @@ -38,7 +39,9 @@ def serialize_ft_billing_yearly_totals(rows): "chargeable_units": row.chargeable_units, "notifications_sent": row.notifications_sent, "rate": float(row.rate), + # TEMPORARY: while we migrate to "cost" in the Admin app "letter_total": float(row.billable_units * row.rate) if row.notification_type == 'letter' else 0, + "cost": float(row.cost), } for row in rows ] diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 078167702..131e53b35 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -223,6 +223,7 @@ def fetch_billing_totals_for_year(service_id, year): # TEMPORARY: while we migrate away from "billing_units" func.sum(query.c.billable_units).label("billable_units"), func.sum(query.c.chargeable_units).label("chargeable_units"), + func.sum(query.c.cost).label("cost"), ).group_by( query.c.rate, query.c.notification_type @@ -280,6 +281,7 @@ def fetch_monthly_billing_for_year(service_id, year): # TEMPORARY: while we migrate away from "billing_units" func.sum(query.c.billable_units).label("billable_units"), func.sum(query.c.chargeable_units).label("chargeable_units"), + func.sum(query.c.cost).label("cost"), ).group_by( query.c.rate, query.c.notification_type, @@ -311,6 +313,7 @@ def query_service_email_usage_for_year(service_id, year): FactBilling.billable_units.label("chargeable_units"), FactBilling.rate, FactBilling.notification_type, + literal(0).label("cost"), ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start, @@ -334,6 +337,7 @@ def query_service_letter_usage_for_year(service_id, year): FactBilling.notifications_sent.label("chargeable_units"), FactBilling.rate, FactBilling.notification_type, + (FactBilling.notifications_sent * FactBilling.rate).label("cost"), ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start, @@ -343,9 +347,61 @@ def query_service_letter_usage_for_year(service_id, year): def query_service_sms_usage_for_year(service_id, year): + """ + Returns rows from the ft_billing table with some calculated values like cost, + incorporating the SMS free allowance e.g. + + ( + bst_date=2022-04-27, + notifications_sent=12, + chargeable_units=12, + rate=0.0165, + [cost=0 <== covered by the free allowance], + [cost=0.198 <== if free allowance exhausted], + [cost=0.099 <== only some free allowance left], + ... + ) + + In order to calculate how much free allowance is left, we need to work out + how much was used for previous bst_dates - cumulative_chargeable_units - + which we then subtract from the free allowance for the year. + + cumulative_chargeable_units is calculated using a "window" clause, which has + access to all the rows identified by the query filter. Note that it's not + affected by any GROUP BY clauses that happen in outer queries. + + https://www.postgresql.org/docs/current/tutorial-window.html + + ASSUMPTION: rates always change at midnight i.e. there can only be one rate + on a given bst_date. This means we don't need to worry about how to assign + free allowance if it happens to run out when a rate changes. + """ year_start, year_end = get_financial_year_dates(year) chargeable_units = FactBilling.billable_units * FactBilling.rate_multiplier + # Subquery for the number of chargeable units in all rows preceding this one, + # which might be none if this is the first row (hence the "coalesce"). + cumulative_chargeable_units = func.coalesce( + func.sum(chargeable_units).over( + # order is "ASC" by default + order_by=[FactBilling.bst_date], + # first row to previous row + rows=(None, -1) + ), + 0 + ) + + # Subquery for how much free allowance we have left before the current row, + # so we can work out the cost for this row after taking it into account. + cumulative_free_remainder = func.greatest( + AnnualBilling.free_sms_fragment_limit - cumulative_chargeable_units, + 0 + ) + + # Subquery for the number of chargeable_units that we will actually charge + # for, after taking any remaining free allowance into account. + charged_units = func.greatest(chargeable_units - cumulative_free_remainder, 0) + return db.session.query( FactBilling.bst_date, FactBilling.postage, # should always be "none" @@ -355,11 +411,16 @@ def query_service_sms_usage_for_year(service_id, year): chargeable_units.label("chargeable_units"), FactBilling.rate, FactBilling.notification_type, + (charged_units * FactBilling.rate).label("cost") + ).join( + AnnualBilling, + AnnualBilling.service_id == service_id ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start, FactBilling.bst_date <= year_end, - FactBilling.notification_type == SMS_TYPE + FactBilling.notification_type == SMS_TYPE, + AnnualBilling.financial_year_start == year, ) diff --git a/tests/app/billing/test_rest.py b/tests/app/billing/test_rest.py index f6445e63e..51c40ef3d 100644 --- a/tests/app/billing/test_rest.py +++ b/tests/app/billing/test_rest.py @@ -147,6 +147,8 @@ def set_up_monthly_data(): billable_unit=1, rate=0.33, postage='second') + + create_annual_billing(service_id=service.id, free_sms_fragment_limit=4, financial_year_start=2016) return service @@ -174,6 +176,7 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(admin_request, notify_db_se assert letter_row["notifications_sent"] == 30 assert letter_row["rate"] == 0.33 assert letter_row["postage"] == "second" + assert letter_row["cost"] == 9.9 assert sms_row["month"] == "April" assert sms_row["notification_type"] == "sms" @@ -182,6 +185,8 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(admin_request, notify_db_se assert sms_row["notifications_sent"] == 30 assert sms_row["rate"] == 0.162 assert sms_row["postage"] == "none" + # free allowance is 4, so (30 - 4) * 0.162 + assert sms_row["cost"] == 4.212 def set_up_yearly_data(): @@ -189,6 +194,7 @@ def set_up_yearly_data(): sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") letter_template = create_template(service=service, template_type="letter") + for month in range(1, 13): mon = str(month).zfill(2) for day in range(1, monthrange(2016, month)[1] + 1): @@ -209,6 +215,8 @@ def set_up_yearly_data(): rate=0.33, postage='second') start_date, end_date = get_month_start_and_end_date_in_utc(datetime(2016, int(mon), 1)) + + create_annual_billing(service_id=service.id, free_sms_fragment_limit=4, financial_year_start=2016) return service @@ -251,6 +259,7 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(admin_request, notify_ assert json_response[0]['notifications_sent'] == 275 assert json_response[0]['rate'] == 0 assert json_response[0]['letter_total'] == 0 + assert json_response[0]['cost'] == 0 assert json_response[1]['notification_type'] == 'letter' assert json_response[1]['billing_units'] == 275 @@ -258,6 +267,7 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(admin_request, notify_ assert json_response[1]['notifications_sent'] == 275 assert json_response[1]['rate'] == 0.33 assert json_response[1]['letter_total'] == 90.75 + assert json_response[1]['cost'] == 90.75 assert json_response[2]['notification_type'] == 'sms' assert json_response[2]['billing_units'] == 825 @@ -265,3 +275,4 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(admin_request, notify_ assert json_response[2]['notifications_sent'] == 550 assert json_response[2]['rate'] == 0.0162 assert json_response[2]['letter_total'] == 0 + assert json_response[2]['cost'] == 13.3002 diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 5d86b335a..f11632fec 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -73,8 +73,8 @@ def set_up_yearly_data_variable_rates(): letter_template = create_template(service=service, template_type="letter") create_ft_billing(bst_date='2018-05-16', template=sms_template, rate=0.162) - create_ft_billing(bst_date='2018-05-17', template=sms_template, rate_multiplier=2, rate=0.162, billable_unit=2) - create_ft_billing(bst_date='2018-05-16', template=sms_template, rate_multiplier=2, rate=0.0150, billable_unit=2) + create_ft_billing(bst_date='2018-05-17', template=sms_template, rate_multiplier=2, rate=0.0150, billable_unit=2) + create_ft_billing(bst_date='2018-05-16', template=sms_template, rate_multiplier=2, rate=0.162, billable_unit=2) create_ft_billing(bst_date='2018-05-16', template=letter_template, rate=0.33, postage='second') create_ft_billing( @@ -426,6 +426,7 @@ def test_get_rate_for_letters_when_page_count_is_zero(notify_db_session): def test_fetch_monthly_billing_for_year(notify_db_session): service = set_up_yearly_data() + create_annual_billing(service_id=service.id, free_sms_fragment_limit=10, financial_year_start=2016) results = fetch_monthly_billing_for_year(service.id, 2016) assert len(results) == 48 @@ -436,6 +437,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert results[0].billable_units == 30 assert results[0].chargeable_units == 0 assert results[0].rate == Decimal('0') + assert results[0].cost == Decimal('0') assert str(results[1].month) == "2016-04-01" assert results[1].notification_type == 'letter' @@ -443,6 +445,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert results[1].billable_units == 30 assert results[1].chargeable_units == 30 assert results[1].rate == Decimal('0.30') + assert results[1].cost == Decimal('9') assert str(results[1].month) == "2016-04-01" assert results[2].notification_type == 'letter' @@ -450,6 +453,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert results[2].billable_units == 30 assert results[2].chargeable_units == 30 assert results[2].rate == Decimal('0.33') + assert results[2].cost == Decimal('9.9') assert str(results[3].month) == "2016-04-01" assert results[3].notification_type == 'sms' @@ -457,6 +461,8 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert results[3].billable_units == 30 assert results[3].chargeable_units == 30 assert results[3].rate == Decimal('0.162') + # free allowance is 10, so (30 - 10) * 0.162 + assert results[3].cost == Decimal('3.24') assert str(results[4].month) == "2016-05-01" assert str(results[47].month) == "2017-03-01" @@ -464,6 +470,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): def test_fetch_monthly_billing_for_year_variable_rates(notify_db_session): service = set_up_yearly_data_variable_rates() + create_annual_billing(service_id=service.id, free_sms_fragment_limit=6, financial_year_start=2018) results = fetch_monthly_billing_for_year(service.id, 2018) # Test data is only for the month of May @@ -475,6 +482,7 @@ def test_fetch_monthly_billing_for_year_variable_rates(notify_db_session): assert results[0].billable_units == 1 assert results[0].chargeable_units == 1 assert results[0].rate == Decimal('0.33') + assert results[0].cost == Decimal('0.33') assert str(results[1].month) == "2018-05-01" assert results[1].notification_type == 'letter' @@ -482,6 +490,7 @@ def test_fetch_monthly_billing_for_year_variable_rates(notify_db_session): assert results[1].billable_units == 2 assert results[1].chargeable_units == 2 assert results[1].rate == Decimal('0.36') + assert results[1].cost == Decimal('0.72') assert str(results[2].month) == "2018-05-01" assert results[2].notification_type == 'sms' @@ -489,6 +498,8 @@ def test_fetch_monthly_billing_for_year_variable_rates(notify_db_session): assert results[2].billable_units == 4 assert results[2].chargeable_units == 4 assert results[2].rate == Decimal('0.015') + # 1 free units on the 17th + assert results[2].cost == Decimal('0.045') assert str(results[3].month) == "2018-05-01" assert results[3].notification_type == 'sms' @@ -496,6 +507,8 @@ def test_fetch_monthly_billing_for_year_variable_rates(notify_db_session): assert results[3].billable_units == 5 assert results[3].chargeable_units == 5 assert results[3].rate == Decimal('0.162') + # 5 free units on the 16th + assert results[3].cost == Decimal('0') @freeze_time('2018-08-01 13:30:00') @@ -504,6 +517,7 @@ def test_fetch_monthly_billing_for_year_adds_data_for_today(notify_db_session): template = create_template(service=service, template_type="sms") create_rate(start_date=datetime.utcnow() - timedelta(days=1), value=0.158, notification_type='sms') + create_annual_billing(service_id=service.id, free_sms_fragment_limit=1000, financial_year_start=2018) for i in range(1, 32): create_ft_billing(bst_date='2018-07-{}'.format(i), template=template) @@ -519,6 +533,7 @@ def test_fetch_monthly_billing_for_year_adds_data_for_today(notify_db_session): def test_fetch_billing_totals_for_year(notify_db_session): service = set_up_yearly_data() + create_annual_billing(service_id=service.id, free_sms_fragment_limit=1000, financial_year_start=2016) results = fetch_billing_totals_for_year(service_id=service.id, year=2016) assert len(results) == 4 @@ -527,28 +542,48 @@ def test_fetch_billing_totals_for_year(notify_db_session): assert results[0].billable_units == 365 assert results[0].chargeable_units == 0 assert results[0].rate == Decimal('0') + assert results[0].cost == Decimal('0') assert results[1].notification_type == 'letter' assert results[1].notifications_sent == 365 assert results[1].billable_units == 365 assert results[1].chargeable_units == 365 assert results[1].rate == Decimal('0.3') + assert results[1].cost == Decimal('109.5') assert results[2].notification_type == 'letter' assert results[2].notifications_sent == 365 assert results[2].billable_units == 365 assert results[2].chargeable_units == 365 assert results[2].rate == Decimal('0.33') + assert results[2].cost == Decimal('120.45') assert results[3].notification_type == 'sms' assert results[3].notifications_sent == 365 assert results[3].billable_units == 365 assert results[3].chargeable_units == 365 assert results[3].rate == Decimal('0.162') + assert results[3].cost == Decimal('0') + + +def test_fetch_billing_totals_for_year_uses_current_annual_billing(notify_db_session): + service = set_up_yearly_data() + create_annual_billing(service_id=service.id, free_sms_fragment_limit=400, financial_year_start=2015) + create_annual_billing(service_id=service.id, free_sms_fragment_limit=0, financial_year_start=2016) + + result = next( + result for result in + fetch_billing_totals_for_year(service_id=service.id, year=2016) + if result.notification_type == 'sms' + ) + + assert result.chargeable_units == 365 + assert result.cost > 0 def test_fetch_billing_totals_for_year_variable_rates(notify_db_session): service = set_up_yearly_data_variable_rates() + create_annual_billing(service_id=service.id, free_sms_fragment_limit=6, financial_year_start=2018) results = fetch_billing_totals_for_year(service_id=service.id, year=2018) assert len(results) == 4 @@ -557,24 +592,30 @@ def test_fetch_billing_totals_for_year_variable_rates(notify_db_session): assert results[0].billable_units == 1 assert results[0].chargeable_units == 1 assert results[0].rate == Decimal('0.33') + assert results[0].cost == Decimal('0.33') assert results[1].notification_type == 'letter' assert results[1].notifications_sent == 2 assert results[1].billable_units == 2 assert results[1].chargeable_units == 2 assert results[1].rate == Decimal('0.36') + assert results[1].cost == Decimal('0.72') assert results[2].notification_type == 'sms' assert results[2].notifications_sent == 1 assert results[2].billable_units == 4 assert results[2].chargeable_units == 4 assert results[2].rate == Decimal('0.015') + # 1 free unit on the 17th + assert results[2].cost == Decimal('0.045') assert results[3].notification_type == 'sms' assert results[3].notifications_sent == 2 assert results[3].billable_units == 5 assert results[3].chargeable_units == 5 assert results[3].rate == Decimal('0.162') + # 5 free units on the 16th + assert results[3].cost == Decimal('0') def test_delete_billing_data(notify_db_session):