From 106da583ea26242e7e80ac9f00aad7f06a710e22 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 calculation in Admin [^1] for SMS and also in API [^2] for letters. 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 with the data currently returned from the API. Since the calculation now depends on annual billing, we need to change all the tests to make sure a suitable row exists. 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 [^3] 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://learnsql.com/blog/difference-between-rows-range-window-functions/ --- app/billing/billing_schemas.py | 2 ++ app/dao/fact_billing_dao.py | 33 +++++++++++++++++++++++++++- tests/app/billing/test_rest.py | 6 +++++ tests/app/dao/test_ft_billing_dao.py | 27 +++++++++++++++++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index 637552e72..d03fa68e4 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -34,7 +34,9 @@ def serialize_ft_billing_yearly_totals(rows): "billing_units": row.billable_units, "chargeable_units": row.chargeable_units, "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 c8761c123..a35782c4d 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -206,6 +206,7 @@ def fetch_billing_totals_for_year(service_id, year): func.sum(query.c.chargeable_units).label("chargeable_units"), query.c.rate.label("rate"), query.c.notification_type.label("notification_type"), + func.sum(query.c.cost).label("cost"), ).group_by( query.c.rate, query.c.notification_type @@ -287,6 +288,7 @@ def query_service_email_usage_for_year(service_id, year): FactBilling.notifications_sent.label("chargeable_units"), FactBilling.rate, FactBilling.notification_type, + literal(0).label("cost"), ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start, @@ -303,6 +305,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, @@ -315,16 +318,44 @@ def query_service_sms_usage_for_year(service_id, year): 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_by=[ + FactBilling.bst_date, # order is "ASC" by default + FactBilling.rate # ensures test stability for rows on the same day + ], + rows=(None, -1) # ROWS BETWEEN UNBOUNDED PRECEDING AND 1 ROW PRECEDING + ), + literal(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 + ) + return db.session.query( FactBilling.notifications_sent, chargeable_units.label("chargeable_units"), FactBilling.rate, FactBilling.notification_type, + ( + func.greatest(chargeable_units - cumulative_free_remainder, literal(0)) * + FactBilling.rate + ).label("cost") + ).outerjoin( + 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 8b6132f11..7da0a235b 100644 --- a/tests/app/billing/test_rest.py +++ b/tests/app/billing/test_rest.py @@ -185,6 +185,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): @@ -205,6 +206,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 @@ -246,15 +249,18 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(admin_request, notify_ assert json_response[0]['chargeable_units'] == 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 assert json_response[1]['chargeable_units'] == 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 assert json_response[2]['chargeable_units'] == 825 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 7be27ba39..ae4284e8c 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -502,6 +502,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 @@ -510,28 +511,48 @@ def test_fetch_billing_totals_for_year(notify_db_session): assert results[0].billable_units == 365 assert results[0].chargeable_units == 365 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 @@ -540,24 +561,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 == 1 assert results[1].billable_units == 1 assert results[1].chargeable_units == 1 assert results[1].rate == Decimal('0.36') + assert results[1].cost == Decimal('0.36') 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') + # 4 units sent on the 16th, 0 on the 17th + assert results[2].cost == Decimal('0') 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') + # 1 free unit on the 16th, 1 on the 17th (+ 3 paid) + assert results[3].cost == Decimal('0.486') # (5 - 2) * 0.162 def test_delete_billing_data(notify_db_session):