mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-03 18:01:08 -05:00
Fix bug in organisation report for its services and usages.
If a service has not sent any SMS for the financial year the free allowance was showing up as 0 rather than the number in annual billing. The query has been updated to use an outer join so that the free allow will be returned when there is no ft_billing. There is a potential performance enhancement to only return the data for the services of the organisation in the `fetch_sms_free_allowance_remainder_until_date` subquery. I will investigate in a subsequent PR.
This commit is contained in:
@@ -599,7 +599,7 @@ def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date):
|
|||||||
# ASSUMPTION: AnnualBilling has been populated for year.
|
# ASSUMPTION: AnnualBilling has been populated for year.
|
||||||
allowance_left_at_start_date_query = fetch_sms_free_allowance_remainder_until_date(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_billable_units = func.coalesce(func.sum(FactBilling.billable_units * FactBilling.rate_multiplier), 0)
|
||||||
|
|
||||||
# subtract sms_billable_units units accrued since report's start date to get up-to-date
|
# subtract sms_billable_units units accrued since report's start date to get up-to-date
|
||||||
# allowance remainder
|
# allowance remainder
|
||||||
@@ -614,23 +614,25 @@ def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date):
|
|||||||
query = db.session.query(
|
query = db.session.query(
|
||||||
Service.name.label("service_name"),
|
Service.name.label("service_name"),
|
||||||
Service.id.label("service_id"),
|
Service.id.label("service_id"),
|
||||||
allowance_left_at_start_date_query.c.free_sms_fragment_limit,
|
func.coalesce(allowance_left_at_start_date_query.c.free_sms_fragment_limit, 0).label('free_sms_fragment_limit'),
|
||||||
FactBilling.rate.label('sms_rate'),
|
func.coalesce(FactBilling.rate, 0).label('sms_rate'),
|
||||||
sms_allowance_left.label("sms_remainder"),
|
func.coalesce(sms_allowance_left, 0).label("sms_remainder"),
|
||||||
sms_billable_units.label('sms_billable_units'),
|
func.coalesce(sms_billable_units, 0).label('sms_billable_units'),
|
||||||
chargeable_sms.label("chargeable_billable_sms"),
|
func.coalesce(chargeable_sms, 0).label("chargeable_billable_sms"),
|
||||||
sms_cost.label('sms_cost'),
|
func.coalesce(sms_cost, 0).label('sms_cost'),
|
||||||
Service.active.label("active")
|
Service.active.label("active")
|
||||||
).select_from(
|
).select_from(
|
||||||
Service
|
Service
|
||||||
).outerjoin(
|
).outerjoin(
|
||||||
allowance_left_at_start_date_query, Service.id == allowance_left_at_start_date_query.c.service_id
|
allowance_left_at_start_date_query, Service.id == allowance_left_at_start_date_query.c.service_id
|
||||||
).join(
|
).outerjoin(
|
||||||
FactBilling, FactBilling.service_id == Service.id,
|
FactBilling, and_(
|
||||||
|
Service.id == FactBilling.service_id,
|
||||||
|
FactBilling.bst_date >= start_date,
|
||||||
|
FactBilling.bst_date < end_date,
|
||||||
|
FactBilling.notification_type == SMS_TYPE,
|
||||||
|
)
|
||||||
).filter(
|
).filter(
|
||||||
FactBilling.bst_date >= start_date,
|
|
||||||
FactBilling.bst_date <= end_date,
|
|
||||||
FactBilling.notification_type == SMS_TYPE,
|
|
||||||
Service.organisation_id == organisation_id,
|
Service.organisation_id == organisation_id,
|
||||||
Service.restricted.is_(False)
|
Service.restricted.is_(False)
|
||||||
).group_by(
|
).group_by(
|
||||||
|
|||||||
@@ -736,7 +736,7 @@ def test_fetch_usage_year_for_organisation(notify_db_session):
|
|||||||
notifications_sent=1100)
|
notifications_sent=1100)
|
||||||
results = fetch_usage_year_for_organisation(fixtures["org_1"].id, 2019)
|
results = fetch_usage_year_for_organisation(fixtures["org_1"].id, 2019)
|
||||||
|
|
||||||
assert len(results) == 2
|
assert len(results) == 3
|
||||||
first_row = results[str(fixtures["service_1_sms_and_letter"].id)]
|
first_row = results[str(fixtures["service_1_sms_and_letter"].id)]
|
||||||
assert first_row['service_id'] == fixtures["service_1_sms_and_letter"].id
|
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['service_name'] == fixtures["service_1_sms_and_letter"].name
|
||||||
@@ -757,6 +757,16 @@ def test_fetch_usage_year_for_organisation(notify_db_session):
|
|||||||
assert second_row['letter_cost'] == 0
|
assert second_row['letter_cost'] == 0
|
||||||
assert second_row['emails_sent'] == 1100
|
assert second_row['emails_sent'] == 1100
|
||||||
|
|
||||||
|
third_row = results[str(fixtures["service_with_out_ft_billing_this_year"].id)]
|
||||||
|
assert third_row['service_id'] == fixtures["service_with_out_ft_billing_this_year"].id
|
||||||
|
assert third_row['service_name'] == fixtures["service_with_out_ft_billing_this_year"].name
|
||||||
|
assert third_row['free_sms_limit'] == 10
|
||||||
|
assert third_row['sms_remainder'] == 10
|
||||||
|
assert third_row['chargeable_billable_sms'] == 0
|
||||||
|
assert third_row['sms_cost'] == 0
|
||||||
|
assert third_row['letter_cost'] == 0
|
||||||
|
assert third_row['emails_sent'] == 0
|
||||||
|
|
||||||
|
|
||||||
def test_fetch_usage_year_for_organisation_populates_ft_billing_for_today(notify_db_session):
|
def test_fetch_usage_year_for_organisation_populates_ft_billing_for_today(notify_db_session):
|
||||||
create_letter_rate(start_date=datetime.utcnow() - timedelta(days=1))
|
create_letter_rate(start_date=datetime.utcnow() - timedelta(days=1))
|
||||||
|
|||||||
@@ -1054,6 +1054,19 @@ def set_up_usage_data(start_date):
|
|||||||
)
|
)
|
||||||
create_ft_billing(bst_date=one_week_later, template=sms_template_2, billable_unit=2, rate=0.11)
|
create_ft_billing(bst_date=one_week_later, template=sms_template_2, billable_unit=2, rate=0.11)
|
||||||
|
|
||||||
|
# service without ft_billing this year
|
||||||
|
service_with_out_ft_billing_this_year = create_service(
|
||||||
|
service_name='f - without ft_billing',
|
||||||
|
purchase_order_number="sms purchase order number",
|
||||||
|
billing_contact_names="sms billing contact names",
|
||||||
|
billing_contact_email_addresses="sms@billing.contact email@addresses.gov.uk",
|
||||||
|
billing_reference="sms billing reference"
|
||||||
|
)
|
||||||
|
create_annual_billing(
|
||||||
|
service_id=service_with_out_ft_billing_this_year.id, free_sms_fragment_limit=10, financial_year_start=year
|
||||||
|
)
|
||||||
|
dao_add_service_to_organisation(service=service_with_out_ft_billing_this_year, organisation_id=org_1.id)
|
||||||
|
|
||||||
# dictionary with services and orgs to return
|
# dictionary with services and orgs to return
|
||||||
return {
|
return {
|
||||||
"org_1": org_1,
|
"org_1": org_1,
|
||||||
@@ -1065,6 +1078,7 @@ def set_up_usage_data(start_date):
|
|||||||
"service_with_letters_without_org": service_with_letters_without_org,
|
"service_with_letters_without_org": service_with_letters_without_org,
|
||||||
"service_with_sms_without_org": service_with_sms_without_org,
|
"service_with_sms_without_org": service_with_sms_without_org,
|
||||||
"service_with_sms_within_allowance": service_with_sms_within_allowance,
|
"service_with_sms_within_allowance": service_with_sms_within_allowance,
|
||||||
|
"service_with_out_ft_billing_this_year": service_with_out_ft_billing_this_year,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user