From da8eaaed44f2b4c7fb4c576ed6b12c20b43c215a Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 10 Jul 2020 17:43:40 +0100 Subject: [PATCH] Update letter data for usage-for-all-services report Usage for all services is a platform admin report that groups letters by postage. We want it to show `europe` and `rest-of-world` letters under a single category of `international`, so this updates the query to do that and to order appropriately. --- app/dao/fact_billing_dao.py | 17 +++++++++++++---- app/models.py | 2 ++ app/platform_stats/rest.py | 18 +++++++++++++++++- tests/app/dao/test_ft_billing_dao.py | 8 +++++--- tests/app/db.py | 8 +++++++- tests/app/platform_stats/test_rest.py | 6 ++++-- 6 files changed, 48 insertions(+), 11 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index ac29ecdbf..631b3042b 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -4,7 +4,7 @@ from flask import current_app from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst from sqlalchemy.dialects.postgresql import insert from sqlalchemy import func, desc, Date, Integer, and_ -from sqlalchemy.sql.expression import literal +from sqlalchemy.sql.expression import case, literal from app import db from app.dao.date_util import ( @@ -28,6 +28,7 @@ from app.models import ( NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS, AnnualBilling, Organisation, + INTERNATIONAL_POSTAGE_TYPES, ) from app.utils import get_london_midnight_in_utc, get_notification_table_to_use @@ -146,13 +147,21 @@ def fetch_letter_costs_for_all_services(start_date, end_date): def fetch_letter_line_items_for_all_services(start_date, end_date): + formatted_postage = case( + [(FactBilling.postage.in_(INTERNATIONAL_POSTAGE_TYPES), "international")], else_=FactBilling.postage + ).label("postage") + + postage_order = case(((formatted_postage == "second", 1), + (formatted_postage == "first", 2), + (formatted_postage == "international", 3))) + query = db.session.query( Organisation.name.label("organisation_name"), Organisation.id.label("organisation_id"), Service.name.label("service_name"), Service.id.label("service_id"), FactBilling.rate.label("letter_rate"), - FactBilling.postage.label("postage"), + formatted_postage, func.sum(FactBilling.notifications_sent).label("letters_sent"), ).select_from( Service @@ -170,11 +179,11 @@ def fetch_letter_line_items_for_all_services(start_date, end_date): Service.id, Service.name, FactBilling.rate, - FactBilling.postage + formatted_postage ).order_by( Organisation.name, Service.name, - FactBilling.postage.desc(), + postage_order, FactBilling.rate, ) return query.all() diff --git a/app/models.py b/app/models.py index 81670144c..2f4d7a5bb 100644 --- a/app/models.py +++ b/app/models.py @@ -1356,6 +1356,8 @@ SECOND_CLASS = 'second' EUROPE = 'europe' REST_OF_WORLD = 'rest-of-world' POSTAGE_TYPES = [FIRST_CLASS, SECOND_CLASS, EUROPE, REST_OF_WORLD] +UK_POSTAGE_TYPES = [FIRST_CLASS, SECOND_CLASS] +INTERNATIONAL_POSTAGE_TYPES = [EUROPE, REST_OF_WORLD] RESOLVE_POSTAGE_FOR_FILE_NAME = { FIRST_CLASS: 1, SECOND_CLASS: 2, diff --git a/app/platform_stats/rest.py b/app/platform_stats/rest.py index 316759341..e88961c83 100644 --- a/app/platform_stats/rest.py +++ b/app/platform_stats/rest.py @@ -9,6 +9,7 @@ from app.dao.fact_billing_dao import ( ) from app.dao.fact_notification_status_dao import fetch_notification_status_totals_for_all_services from app.errors import register_errors, InvalidRequest +from app.models import UK_POSTAGE_TYPES from app.platform_stats.platform_stats_schema import platform_stats_request from app.service.statistics import format_admin_stats from app.schema_validation import validate @@ -65,7 +66,8 @@ def get_usage_for_all_services(): letter_breakdown = fetch_letter_line_items_for_all_services(start_date, end_date) lb_by_service = [ - (lb.service_id, "{} {} class letters at {}p".format(lb.letters_sent, lb.postage, int(lb.letter_rate * 100))) + (lb.service_id, + f"{lb.letters_sent} {postage_description(lb.postage)} letters at {format_letter_rate(lb.letter_rate)}") for lb in letter_breakdown ] combined = {} @@ -106,3 +108,17 @@ def get_usage_for_all_services(): x['organisation_name'], x['service_name'] ))) + + +def postage_description(postage): + if postage in UK_POSTAGE_TYPES: + return f'{postage} class' + else: + return 'international' + + +def format_letter_rate(number): + if number >= 1: + return f"£{number:,.2f}" + + return f"{number * 100:.0f}p" diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index fabbc1120..852beea4c 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -623,7 +623,7 @@ def test_fetch_letter_costs_for_all_services(notify_db_session): assert len(results) == 3 assert results[0] == (org.name, org.id, service.name, service.id, Decimal('3.40')) assert results[1] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal('14.00')) - assert results[2] == (None, None, service_3.name, service_3.id, Decimal('8.25')) + assert results[2] == (None, None, service_3.name, service_3.id, Decimal('24.45')) def test_fetch_letter_line_items_for_all_service(notify_db_session): @@ -632,12 +632,14 @@ def test_fetch_letter_line_items_for_all_service(notify_db_session): results = fetch_letter_line_items_for_all_services(datetime(2019, 6, 1), datetime(2019, 9, 30)) - assert len(results) == 5 + assert len(results) == 7 assert results[0] == (org_1.name, org_1.id, service_1.name, service_1.id, Decimal('0.45'), 'second', 6) assert results[1] == (org_1.name, org_1.id, service_1.name, service_1.id, Decimal("0.35"), 'first', 2) assert results[2] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal("0.65"), 'second', 20) assert results[3] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal("0.50"), 'first', 2) - assert results[4] == (None, None, service_3.name, service_3.id, Decimal("0.55"), 'second', 15) + assert results[4] == (None, None, service_3.name, service_3.id, Decimal("0.35"), 'second', 2) + assert results[5] == (None, None, service_3.name, service_3.id, Decimal("0.50"), 'first', 1) + assert results[6] == (None, None, service_3.name, service_3.id, Decimal("1.55"), 'international', 15) @freeze_time('2019-06-01 13:30') diff --git a/tests/app/db.py b/tests/app/db.py index e45bff2af..4575dc815 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -941,7 +941,13 @@ def set_up_usage_data(start_date): notifications_sent=12, billable_unit=5, rate=.65, postage='second') create_ft_billing(bst_date=two_days_later, template=letter_template_4, - notifications_sent=15, billable_unit=4, rate=.55, postage='second') + notifications_sent=7, billable_unit=4, rate=1.55, postage='rest-of-world') + create_ft_billing(bst_date=two_days_later, template=letter_template_4, + notifications_sent=8, billable_unit=4, rate=1.55, postage='europe') + create_ft_billing(bst_date=two_days_later, template=letter_template_4, + notifications_sent=2, billable_unit=1, rate=.35, postage='second') + create_ft_billing(bst_date=two_days_later, template=letter_template_4, + notifications_sent=1, billable_unit=1, rate=.50, postage='first') create_ft_billing(bst_date=start_date, template=email_template, notifications_sent=10) diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index 93b4eda11..ecd5bcd56 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -153,5 +153,7 @@ def test_get_usage_for_all_services(notify_db_session, admin_request): assert response[3]["service_id"] == str(service_3.id) assert response[3]["sms_cost"] == 0 assert response[3]["sms_fragments"] == 0 - assert response[3]["letter_cost"] == 8.25 - assert response[3]["letter_breakdown"] == "15 second class letters at 55p\n" + assert response[3]["letter_cost"] == 24.45 + assert response[3]["letter_breakdown"] == ( + "2 second class letters at 35p\n1 first class letters at 50p\n15 international letters at £1.55\n" + )