Change how we populate and retrieve MonthlyBilling totals:

1. For both email and sms, store [] in monthly_totals if there is no billing data (no notifications sent etc.) and return this via the API 2. General refactoring of indentation
2026-06-19 20:50:35 -04:00 · 2017-08-11 16:53:12 +01:00
parent 46ca086aa2
commit 94605d31fa
6 changed files with 78 additions and 92 deletions
--- a/app/dao/monthly_billing_dao.py
+++ b/app/dao/monthly_billing_dao.py
@@ -1,5 +1,8 @@
 from datetime import datetime

+from sqlalchemy import func
+
+
 from app import db
 from app.dao.dao_utils import transactional
 from app.dao.date_util import get_month_start_and_end_date_in_utc, get_financial_year
@@ -31,16 +34,19 @@ def create_or_update_monthly_billing(service_id, billing_month):
    _update_monthly_billing(service_id, start_date, end_date, EMAIL_TYPE)


-def _monthly_billing_data_to_json(monthly):
-    # total cost must take into account the free allowance.
-    # might be a good idea to capture free allowance in this table
-    return [{
-        "billing_units": x.billing_units,
-        "rate_multiplier": x.rate_multiplier,
-        "international": x.international,
-        "rate": x.rate,
-        "total_cost": (x.billing_units * x.rate_multiplier) * x.rate
-    } for x in monthly]
+def _monthly_billing_data_to_json(billing_data):
+    results = []
+    if billing_data:
+        # total cost must take into account the free allowance.
+        # might be a good idea to capture free allowance in this table
+        results = [{
+            "billing_units": x.billing_units,
+            "rate_multiplier": x.rate_multiplier,
+            "international": x.international,
+            "rate": x.rate,
+            "total_cost": (x.billing_units * x.rate_multiplier) * x.rate
+        } for x in billing_data]
+    return results


@transactional
@@ -82,7 +88,11 @@ def get_monthly_billing_entry(service_id, start_date, notification_type):
 def get_yearly_billing_data_for_date_range(
    service_id, start_date, end_date, notification_types
 ):
-    results = MonthlyBilling.query.filter(
+    results = db.session.query(
+        MonthlyBilling.notification_type,
+        MonthlyBilling.monthly_totals,
+        MonthlyBilling.start_date,
+    ).filter(
        MonthlyBilling.service_id == service_id,
        MonthlyBilling.start_date >= start_date,
        MonthlyBilling.end_date <= end_date,
@@ -112,5 +122,4 @@ def get_billing_data_for_financial_year(service_id, year, notification_types=[SM
    results = get_yearly_billing_data_for_date_range(
        service_id, start_date, end_date, notification_types
    )
-
    return results
--- a/app/dao/notification_usage_dao.py
+++ b/app/dao/notification_usage_dao.py
@@ -127,6 +127,7 @@ def email_yearly_billing_data_query(service_id, start_date, end_date, rate=0):
        rate_multiplier(),
        NotificationHistory.international
    ).first()
+
    if not result:
        return [(0, 0, 1, EMAIL_TYPE, False, 0)]
    else:
@@ -187,9 +188,14 @@ def is_between(date, start_date, end_date):

 def billing_data_per_month_query(rate, service_id, start_date, end_date, notification_type):
    month = get_london_month_from_utc_column(NotificationHistory.created_at)
-    result = db.session.query(
+    if notification_type == SMS_TYPE:
+        filter_subq = func.sum(NotificationHistory.billable_units).label('billing_units')
+    elif notification_type == EMAIL_TYPE:
+        filter_subq = func.count(NotificationHistory.billable_units).label('billing_units')
+
+    results = db.session.query(
        month.label('month'),
-        func.sum(NotificationHistory.billable_units).label('billing_units'),
+        filter_subq,
        rate_multiplier().label('rate_multiplier'),
        NotificationHistory.international,
        NotificationHistory.notification_type,
@@ -206,17 +212,7 @@ def billing_data_per_month_query(rate, service_id, start_date, end_date, notific
        rate_multiplier()
    ).all()

-    if not result and notification_type == EMAIL_TYPE:
-        start_date = convert_utc_to_bst(start_date)
-        BillingData = namedtuple(
-            'BillingData',
-            'start_date billing_units rate_multiplier international notification_type rate'
-        )
-        return [BillingData(start_date, 0, 1, False, notification_type, 0)]
-    else:
-        return result
-
-    return result
+    return results


 def rate_multiplier():
--- a/app/service/rest.py
+++ b/app/service/rest.py
@@ -528,13 +528,14 @@ def get_yearly_monthly_usage(service_id):
    try:
        year = int(request.args.get('year'))
        results = notification_usage_dao.get_monthly_billing_data(service_id, year)
-        json_results = [{"month": x[0],
-                         "billing_units": x[1],
-                         "rate_multiplier": x[2],
-                         "international": x[3],
-                         "notification_type": x[4],
-                         "rate": x[5]
-                         } for x in results]
+        json_results = [{
+            "month": x[0],
+            "billing_units": x[1],
+            "rate_multiplier": x[2],
+            "international": x[3],
+            "notification_type": x[4],
+            "rate": x[5]
+        } for x in results]
        return json.dumps(json_results)
    except TypeError:
        return jsonify(result='error', message='No valid year provided'), 400
--- a/tests/app/celery/test_scheduled_tasks.py
+++ b/tests/app/celery/test_scheduled_tasks.py
@@ -635,15 +635,12 @@ def test_populate_monthly_billing_populates_correctly(sample_template):
    assert monthly_billing[0].end_date == jul_month_end
    assert monthly_billing[0].notification_type == 'email'
    assert monthly_billing[0].notification_type == 'email'
-    assert len(monthly_billing[1].monthly_totals) == 1
-    assert monthly_billing[0].monthly_totals[0]['billing_units'] == 0
-    assert monthly_billing[0].monthly_totals[0]['total_cost'] == 0
+    assert monthly_billing[0].monthly_totals == []

    assert monthly_billing[1].service_id == sample_template.service_id
    assert monthly_billing[1].start_date == jul_month_start
    assert monthly_billing[1].end_date == jul_month_end
    assert monthly_billing[1].notification_type == 'sms'
-    assert len(monthly_billing[1].monthly_totals) == 1
    assert sorted(monthly_billing[1].monthly_totals[0]) == sorted(
        {
            'international': False,
@@ -672,7 +669,7 @@ def test_populate_monthly_billing_updates_correct_month_in_bst(sample_template):
    assert monthly_billing[0].start_date == apr_month_start
    assert monthly_billing[0].end_date == apr_month_end
    assert monthly_billing[0].notification_type == 'email'
-    assert monthly_billing[0].monthly_totals[0]['billing_units'] == 0
+    assert monthly_billing[0].monthly_totals == []

    assert monthly_billing[1].service_id == sample_template.service_id
    assert monthly_billing[1].start_date == apr_month_start
--- a/tests/app/dao/test_monthly_billing.py
+++ b/tests/app/dao/test_monthly_billing.py
@@ -41,12 +41,12 @@ JAN_2017_MONTH_END = datetime(2017, 1, 31, 23, 59, 59, 99999)
 FEB_2017 = datetime(2017, 2, 15)
 APR_2016 = datetime(2016, 4, 10)

-EMPTY_BILLING_DATA = {
-    'billing_units': 0,
-    'international': False,
-    'rate': 0,
-    'rate_multiplier': 1,
-    'total_cost': 0,
+NO_BILLING_DATA = {
+    "billing_units": 0,
+    "rate_multiplier": 1,
+    "international": False,
+    "rate": 0,
+    "total_cost": 0
 }


@@ -189,13 +189,7 @@ def test_add_monthly_billing_for_single_month_populates_correctly(
    _assert_monthly_billing(
        monthly_billing[0], sample_template.service.id, 'email', JAN_2017_MONTH_START, JAN_2017_MONTH_END
    )
-    _assert_monthly_billing_totals(monthly_billing[0].monthly_totals[0], {
-        "billing_units": 0,
-        "rate_multiplier": 1,
-        "international": False,
-        "rate": 0,
-        "total_cost": 0
-    })
+    assert monthly_billing[0].monthly_totals == []

    _assert_monthly_billing(
        monthly_billing[1], sample_template.service.id, 'sms', JAN_2017_MONTH_START, JAN_2017_MONTH_END
@@ -225,13 +219,23 @@ def test_add_monthly_billing_for_multiple_months_populate_correctly(
    create_or_update_monthly_billing(service_id=sample_template.service_id, billing_month=FEB_2016_MONTH_START)
    create_or_update_monthly_billing(service_id=sample_template.service_id, billing_month=MAR_2016_MONTH_START)

-    monthly_billing = MonthlyBilling.query.all()
+    monthly_billing = MonthlyBilling.query.order_by(MonthlyBilling.notification_type).all()

    assert len(monthly_billing) == 4
    _assert_monthly_billing(
-        monthly_billing[0], sample_template.service.id, 'sms', FEB_2016_MONTH_START, FEB_2016_MONTH_END
+        monthly_billing[0], sample_template.service.id, 'email', FEB_2016_MONTH_START, FEB_2016_MONTH_END
    )
-    _assert_monthly_billing_totals(monthly_billing[0].monthly_totals[0], {
+    assert monthly_billing[0].monthly_totals == []
+
+    _assert_monthly_billing(
+        monthly_billing[1], sample_template.service.id, 'email', MAR_2016_MONTH_START, MAR_2016_MONTH_END
+    )
+    assert monthly_billing[1].monthly_totals == []
+
+    _assert_monthly_billing(
+        monthly_billing[2], sample_template.service.id, 'sms', FEB_2016_MONTH_START, FEB_2016_MONTH_END
+    )
+    _assert_monthly_billing_totals(monthly_billing[2].monthly_totals[0], {
        "billing_units": 1,
        "rate_multiplier": 2,
        "international": False,
@@ -240,14 +244,9 @@ def test_add_monthly_billing_for_multiple_months_populate_correctly(
    })

    _assert_monthly_billing(
-        monthly_billing[1], sample_template.service.id, 'email', FEB_2016_MONTH_START, FEB_2016_MONTH_END
+        monthly_billing[3], sample_template.service.id, 'sms', MAR_2016_MONTH_START, MAR_2016_MONTH_END
    )
-    _assert_monthly_billing_totals(monthly_billing[1].monthly_totals[0], EMPTY_BILLING_DATA)
-
-    _assert_monthly_billing(
-        monthly_billing[2], sample_template.service.id, 'sms', MAR_2016_MONTH_START, MAR_2016_MONTH_END
-    )
-    _assert_monthly_billing_totals(monthly_billing[2].monthly_totals[0], {
+    _assert_monthly_billing_totals(monthly_billing[3].monthly_totals[0], {
        "billing_units": 2,
        "rate_multiplier": 3,
        "international": False,
@@ -255,11 +254,6 @@ def test_add_monthly_billing_for_multiple_months_populate_correctly(
        "total_cost": 0.72
    })

-    _assert_monthly_billing(
-        monthly_billing[3], sample_template.service.id, 'email', MAR_2016_MONTH_START, MAR_2016_MONTH_END
-    )
-    _assert_monthly_billing_totals(monthly_billing[3].monthly_totals[0], EMPTY_BILLING_DATA)
-

 def test_add_monthly_billing_with_multiple_rates_populate_correctly(
    sample_template
@@ -274,20 +268,25 @@ def test_add_monthly_billing_with_multiple_rates_populate_correctly(

    create_or_update_monthly_billing(service_id=sample_template.service_id, billing_month=JAN_2017_MONTH_START)

-    monthly_billing = MonthlyBilling.query.all()
+    monthly_billing = MonthlyBilling.query.order_by(MonthlyBilling.notification_type).all()

    assert len(monthly_billing) == 2
    _assert_monthly_billing(
-        monthly_billing[0], sample_template.service.id, 'sms', JAN_2017_MONTH_START, JAN_2017_MONTH_END
+        monthly_billing[0], sample_template.service.id, 'email', JAN_2017_MONTH_START, JAN_2017_MONTH_END
    )
-    _assert_monthly_billing_totals(monthly_billing[0].monthly_totals[0], {
+    assert monthly_billing[0].monthly_totals == []
+
+    _assert_monthly_billing(
+        monthly_billing[1], sample_template.service.id, 'sms', JAN_2017_MONTH_START, JAN_2017_MONTH_END
+    )
+    _assert_monthly_billing_totals(monthly_billing[1].monthly_totals[0], {
        "billing_units": 1,
        "rate_multiplier": 1,
        "international": False,
        "rate": 0.0158,
        "total_cost": 0.0158
    })
-    _assert_monthly_billing_totals(monthly_billing[0].monthly_totals[1], {
+    _assert_monthly_billing_totals(monthly_billing[1].monthly_totals[1], {
        "billing_units": 2,
        "rate_multiplier": 1,
        "international": False,
@@ -295,11 +294,6 @@ def test_add_monthly_billing_with_multiple_rates_populate_correctly(
        "total_cost": 0.246
    })

-    _assert_monthly_billing(
-        monthly_billing[1], sample_template.service.id, 'email', JAN_2017_MONTH_START, JAN_2017_MONTH_END
-    )
-    _assert_monthly_billing_totals(monthly_billing[1].monthly_totals[0], EMPTY_BILLING_DATA)
-

 def test_update_monthly_billing_overwrites_old_totals(sample_template):
    create_rate(APR_2016_MONTH_START, 0.123, SMS_TYPE)
@@ -434,21 +428,10 @@ def test_get_yearly_billing_data_for_year_returns_multiple_notification_types(sa
    )

    assert len(billing_data) == 2
-    _assert_monthly_billing(
-        billing_data[0], sample_template.service.id, 'email', APR_2016_MONTH_START, APR_2016_MONTH_END
-    )
-    _assert_monthly_billing_totals(billing_data[0].monthly_totals[0], {
-        "billing_units": 2,
-        "rate_multiplier": 3,
-        "international": False,
-        "rate": 1.3,
-        "total_cost": 2.1804
-    })

-    _assert_monthly_billing(
-        billing_data[1], sample_template.service.id, 'sms', APR_2016_MONTH_START, APR_2016_MONTH_END
-    )
-    assert billing_data[1].monthly_totals == []
+    assert billing_data[0].notification_type == EMAIL_TYPE
+    assert billing_data[0].monthly_totals[0]['billing_units'] == 2
+    assert billing_data[1].notification_type == SMS_TYPE


@freeze_time("2016-04-21 11:00:00")
@@ -469,9 +452,7 @@ def test_get_yearly_billing_data_for_year_includes_current_day_totals(sample_tem
    )

    assert len(billing_data) == 1
-    _assert_monthly_billing(
-        billing_data[0], sample_template.service.id, 'sms', APR_2016_MONTH_START, APR_2016_MONTH_END
-    )
+    assert billing_data[0].notification_type == SMS_TYPE
    assert billing_data[0].monthly_totals == []

    create_notification(
--- a/tests/app/service/test_rest.py
+++ b/tests/app/service/test_rest.py
@@ -1862,7 +1862,9 @@ def test_get_monthly_billing_usage_returns_empty_list_if_no_notifications(client
        headers=[create_authorization_header()]
    )
    assert response.status_code == 200
-    assert json.loads(response.get_data(as_text=True)) == []
+
+    results = json.loads(response.get_data(as_text=True))
+    assert results == []


 def test_search_for_notification_by_to_field(client, notify_db, notify_db_session):