diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index d580b2a39..964beffaf 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -32,9 +32,9 @@ from app.models import ( from app.utils import get_london_midnight_in_utc, get_notification_table_to_use -def fetch_sms_free_allowance_remainder(start_date): +def fetch_sms_free_allowance_remainder_until_date(end_date): # ASSUMPTION: AnnualBilling has been populated for year. - billing_year = get_financial_year_for_datetime(start_date) + billing_year = get_financial_year_for_datetime(end_date) start_of_year = date(billing_year, 4, 1) billable_units = func.coalesce(func.sum(FactBilling.billable_units * FactBilling.rate_multiplier), 0) @@ -50,7 +50,7 @@ def fetch_sms_free_allowance_remainder(start_date): FactBilling, and_( AnnualBilling.service_id == FactBilling.service_id, FactBilling.bst_date >= start_of_year, - FactBilling.bst_date < start_date, + FactBilling.bst_date < end_date, FactBilling.notification_type == SMS_TYPE, ) ).filter( @@ -65,14 +65,18 @@ def fetch_sms_free_allowance_remainder(start_date): def fetch_sms_billing_for_all_services(start_date, end_date): # ASSUMPTION: AnnualBilling has been populated for year. - free_allowance_remainder = fetch_sms_free_allowance_remainder(start_date).subquery() + allowance_left_at_start_date_query = fetch_sms_free_allowance_remainder_until_date(start_date).subquery() sms_billable_units = func.sum(FactBilling.billable_units * FactBilling.rate_multiplier) - sms_remainder = func.coalesce( - free_allowance_remainder.c.sms_remainder, - free_allowance_remainder.c.free_sms_fragment_limit - ) - chargeable_sms = func.greatest(sms_billable_units - sms_remainder, 0) + + # subtract sms_billable_units units accrued since report's start date to get up-to-date + # allowance remainder + sms_allowance_left = func.greatest(allowance_left_at_start_date_query.c.sms_remainder - sms_billable_units, 0) + + # billable units here are for period between start date and end date only, so to see + # how many are chargeable, we need to see how much free allowance was used up in the + # period up until report's start date and then do a subtraction + chargeable_sms = func.greatest(sms_billable_units - allowance_left_at_start_date_query.c.sms_remainder, 0) sms_cost = chargeable_sms * FactBilling.rate query = db.session.query( @@ -80,16 +84,16 @@ def fetch_sms_billing_for_all_services(start_date, end_date): Organisation.id.label('organisation_id'), Service.name.label("service_name"), Service.id.label("service_id"), - free_allowance_remainder.c.free_sms_fragment_limit, + allowance_left_at_start_date_query.c.free_sms_fragment_limit, FactBilling.rate.label('sms_rate'), - sms_remainder.label("sms_remainder"), + sms_allowance_left.label("sms_remainder"), sms_billable_units.label('sms_billable_units'), chargeable_sms.label("chargeable_billable_sms"), sms_cost.label('sms_cost'), ).select_from( Service ).outerjoin( - free_allowance_remainder, Service.id == free_allowance_remainder.c.service_id + allowance_left_at_start_date_query, Service.id == allowance_left_at_start_date_query.c.service_id ).outerjoin( Service.organisation ).join( @@ -103,8 +107,8 @@ def fetch_sms_billing_for_all_services(start_date, end_date): Organisation.id, Service.id, Service.name, - free_allowance_remainder.c.free_sms_fragment_limit, - free_allowance_remainder.c.sms_remainder, + allowance_left_at_start_date_query.c.free_sms_fragment_limit, + allowance_left_at_start_date_query.c.sms_remainder, FactBilling.rate, ).order_by( Organisation.name, @@ -593,22 +597,26 @@ def fetch_email_usage_for_organisation(organisation_id, start_date, end_date): def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date): # ASSUMPTION: AnnualBilling has been populated for year. - free_allowance_remainder = fetch_sms_free_allowance_remainder(start_date).subquery() + allowance_left_at_start_date_query = fetch_sms_free_allowance_remainder_until_date(start_date).subquery() sms_billable_units = func.sum(FactBilling.billable_units * FactBilling.rate_multiplier) - sms_remainder = func.coalesce( - free_allowance_remainder.c.sms_remainder, - free_allowance_remainder.c.free_sms_fragment_limit - ) - chargeable_sms = func.greatest(sms_billable_units - sms_remainder, 0) + + # subtract sms_billable_units units accrued since report's start date to get up-to-date + # allowance remainder + sms_allowance_left = func.greatest(allowance_left_at_start_date_query.c.sms_remainder - sms_billable_units, 0) + + # billable units here are for period between start date and end date only, so to see + # how many are chargeable, we need to see how much free allowance was used up in the + # period up until report's start date and then do a subtraction + chargeable_sms = func.greatest(sms_billable_units - allowance_left_at_start_date_query.c.sms_remainder, 0) sms_cost = chargeable_sms * FactBilling.rate query = db.session.query( Service.name.label("service_name"), Service.id.label("service_id"), - free_allowance_remainder.c.free_sms_fragment_limit, + allowance_left_at_start_date_query.c.free_sms_fragment_limit, FactBilling.rate.label('sms_rate'), - sms_remainder.label("sms_remainder"), + sms_allowance_left.label("sms_remainder"), sms_billable_units.label('sms_billable_units'), chargeable_sms.label("chargeable_billable_sms"), sms_cost.label('sms_cost'), @@ -616,7 +624,7 @@ def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date): ).select_from( Service ).outerjoin( - free_allowance_remainder, Service.id == free_allowance_remainder.c.service_id + allowance_left_at_start_date_query, Service.id == allowance_left_at_start_date_query.c.service_id ).join( FactBilling, FactBilling.service_id == Service.id, ).filter( @@ -628,8 +636,8 @@ def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date): ).group_by( Service.id, Service.name, - free_allowance_remainder.c.free_sms_fragment_limit, - free_allowance_remainder.c.sms_remainder, + allowance_left_at_start_date_query.c.free_sms_fragment_limit, + allowance_left_at_start_date_query.c.sms_remainder, FactBilling.rate, ).order_by( Service.name diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 24fbbc93c..f67f19686 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -14,7 +14,7 @@ from app.dao.fact_billing_dao import ( fetch_letter_line_items_for_all_services, fetch_monthly_billing_for_year, fetch_sms_billing_for_all_services, - fetch_sms_free_allowance_remainder, + fetch_sms_free_allowance_remainder_until_date, fetch_usage_year_for_organisation, get_rate, get_rates_for_billing, @@ -516,7 +516,7 @@ def test_delete_billing_data(notify_db_session): ) -def test_fetch_sms_free_allowance_remainder_with_two_services(notify_db_session): +def test_fetch_sms_free_allowance_remainder_until_date_with_two_services(notify_db_session): service = create_service(service_name='has free allowance') template = create_template(service=service) org = create_organisation(name="Org for {}".format(service.name)) @@ -534,7 +534,7 @@ def test_fetch_sms_free_allowance_remainder_with_two_services(notify_db_session) create_ft_billing(template=template_2, bst_date=datetime(2016, 4, 22), billable_unit=10, rate=0.11) create_ft_billing(template=template_2, bst_date=datetime(2016, 5, 20), billable_unit=3, rate=0.11) - results = fetch_sms_free_allowance_remainder(datetime(2016, 5, 1)).all() + results = fetch_sms_free_allowance_remainder_until_date(datetime(2016, 5, 1)).all() assert len(results) == 2 service_result = [row for row in results if row[0] == service.id] assert service_result[0] == (service.id, 10, 2, 8) @@ -552,16 +552,16 @@ def test_fetch_sms_billing_for_all_services_for_first_quarter(notify_db_session) create_ft_billing(template=template, bst_date=datetime(2019, 4, 20), billable_unit=44, rate=0.11) results = fetch_sms_billing_for_all_services(datetime(2019, 4, 1), datetime(2019, 5, 30)) assert len(results) == 1 - assert results[0] == (org.name, org.id, service.name, service.id, 25000, Decimal('0.11'), 25000, 44, 0, + assert results[0] == (org.name, org.id, service.name, service.id, 25000, Decimal('0.11'), 24956, 44, 0, Decimal('0')) def test_fetch_sms_billing_for_all_services_with_remainder(notify_db_session): - service = create_service(service_name='a - has free allowance') - template = create_template(service=service) - org = create_organisation(name="Org for {}".format(service.name)) - dao_add_service_to_organisation(service=service, organisation_id=org.id) - create_annual_billing(service_id=service.id, free_sms_fragment_limit=10, financial_year_start=2019) + service_1 = create_service(service_name='a - has free allowance') + template = create_template(service=service_1) + org = create_organisation(name="Org for {}".format(service_1.name)) + dao_add_service_to_organisation(service=service_1, organisation_id=org.id) + create_annual_billing(service_id=service_1.id, free_sms_fragment_limit=10, financial_year_start=2019) create_ft_billing(template=template, bst_date=datetime(2019, 4, 20), billable_unit=2, rate=0.11) create_ft_billing(template=template, bst_date=datetime(2019, 5, 20), billable_unit=2, rate=0.11) create_ft_billing(template=template, bst_date=datetime(2019, 5, 22), billable_unit=1, rate=0.11) @@ -592,11 +592,32 @@ def test_fetch_sms_billing_for_all_services_with_remainder(notify_db_session): results = fetch_sms_billing_for_all_services(datetime(2019, 5, 1), datetime(2019, 5, 31)) assert len(results) == 3 - assert results[0] == (org.name, org.id, service.name, service.id, 10, Decimal('0.11'), 8, 3, 0, Decimal('0')) - assert results[1] == (org_2.name, org_2.id, service_2.name, service_2.id, 10, Decimal('0.11'), 0, 3, 3, - Decimal('0.33')) - assert results[2] == (org_3.name, org_3.id, service_3.name, service_3.id, 10, Decimal('0.11'), 5, 7, 2, - Decimal('0.22')) + + expected_results = [ + # sms_remainder is 5, because "service_1" has 5 sms_billing_units. 2 of them for a period before + # the requested report's start date. + { + "organisation_name": org.name, "organisation_id": org.id, "service_name": service_1.name, + "service_id": service_1.id, "free_sms_fragment_limit": 10, "sms_rate": Decimal('0.11'), "sms_remainder": 5, + "sms_billable_units": 3, "chargeable_billable_sms": 0, "sms_cost": Decimal('0.00') + }, + # sms remainder is 0, because this service sent SMS worth 15 billable units, 12 of which were sent + # before requested report's start date + { + "organisation_name": org_2.name, "organisation_id": org_2.id, "service_name": service_2.name, + "service_id": service_2.id, "free_sms_fragment_limit": 10, "sms_rate": Decimal('0.11'), "sms_remainder": 0, + "sms_billable_units": 3, "chargeable_billable_sms": 3, "sms_cost": Decimal('0.33') + }, + # sms remainder is 0, because this service sent SMS worth 12 billable units, 5 of which were sent + # before requested report's start date + { + "organisation_name": org_3.name, "organisation_id": org_3.id, "service_name": service_3.name, + "service_id": service_3.id, "free_sms_fragment_limit": 10, "sms_rate": Decimal('0.11'), "sms_remainder": 0, + "sms_billable_units": 7, "chargeable_billable_sms": 2, "sms_cost": Decimal('0.22') + }, + ] + + assert [dict(result) for result in results] == expected_results def test_fetch_sms_billing_for_all_services_without_an_organisation_appears(notify_db_session): @@ -604,28 +625,35 @@ def test_fetch_sms_billing_for_all_services_without_an_organisation_appears(noti results = fetch_sms_billing_for_all_services(datetime(2019, 5, 1), datetime(2019, 5, 31)) assert len(results) == 3 - # organisation_name, organisation_id, service_name, service_id, free_sms_fragment_limit, - # sms_rate, sms_remainder, sms_billable_units, chargeable_billable_units, sms_cost - assert results[0] == ( - fixtures["org_1"].name, - fixtures["org_1"].id, - fixtures["service_1_sms_and_letter"].name, - fixtures["service_1_sms_and_letter"].id, - 10, Decimal('0.11'), 8, 3, 0, Decimal('0')) - assert results[1] == ( - None, - None, - fixtures["service_with_sms_without_org"].name, - fixtures["service_with_sms_without_org"].id, - 10, Decimal('0.11'), 0, 3, 3, Decimal('0.33') - ) - assert results[2] == ( - None, - None, - fixtures["service_with_sms_within_allowance"].name, - fixtures["service_with_sms_within_allowance"].id, - 10, Decimal('0.11'), 10, 2, 0, Decimal('0.00') - ) + expected_results = [ + # sms_remainder is 5, because service_1_sms_and_letter has 5 sms_billing_units. 2 of them for a period before + # the requested report's start date. + { + "organisation_name": fixtures["org_1"].name, "organisation_id": fixtures["org_1"].id, + "service_name": fixtures["service_1_sms_and_letter"].name, + "service_id": fixtures["service_1_sms_and_letter"].id, + "free_sms_fragment_limit": 10, "sms_rate": Decimal('0.11'), "sms_remainder": 5, + "sms_billable_units": 3, "chargeable_billable_sms": 0, "sms_cost": Decimal('0.00') + }, + # sms remainder is 0, because this service sent SMS worth 15 billable units, 12 of which were sent + # before requested report's start date + { + "organisation_name": None, "organisation_id": None, + "service_name": fixtures["service_with_sms_without_org"].name, + "service_id": fixtures["service_with_sms_without_org"].id, "free_sms_fragment_limit": 10, + "sms_rate": Decimal('0.11'), "sms_remainder": 0, + "sms_billable_units": 3, "chargeable_billable_sms": 3, "sms_cost": Decimal('0.33') + }, + { + "organisation_name": None, "organisation_id": None, + "service_name": fixtures["service_with_sms_within_allowance"].name, + "service_id": fixtures["service_with_sms_within_allowance"].id, "free_sms_fragment_limit": 10, + "sms_rate": Decimal('0.11'), "sms_remainder": 8, + "sms_billable_units": 2, "chargeable_billable_sms": 0, "sms_cost": Decimal('0.00') + }, + ] + + assert [dict(result) for result in results] == expected_results def test_fetch_letter_costs_and_totals_for_all_services(notify_db_session): @@ -713,7 +741,7 @@ def test_fetch_usage_year_for_organisation(notify_db_session): assert first_row['service_id'] == fixtures["service_1_sms_and_letter"].id assert first_row['service_name'] == fixtures["service_1_sms_and_letter"].name assert first_row['free_sms_limit'] == 10 - assert first_row['sms_remainder'] == 10 + assert first_row['sms_remainder'] == 5 # because there are 5 billable units assert first_row['chargeable_billable_sms'] == 0 assert first_row['sms_cost'] == 0.0 assert first_row['letter_cost'] == 3.4 diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 81fe9c8b0..6a11bcacd 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -831,7 +831,7 @@ def test_get_organisation_services_usage(admin_request, notify_db_session): assert service_usage['free_sms_limit'] == 10 assert service_usage['letter_cost'] == 0 assert service_usage['sms_billable_units'] == 19 - assert service_usage['sms_remainder'] == 10 + assert service_usage['sms_remainder'] == 0 assert service_usage['sms_cost'] == 0.54