From 94605d31fa79f19ec44f7f75f8ac31bbd7f7e625 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 11 Aug 2017 16:53:12 +0100 Subject: [PATCH] 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 --- app/dao/monthly_billing_dao.py | 33 +++++---- app/dao/notification_usage_dao.py | 22 +++--- app/service/rest.py | 15 ++-- tests/app/celery/test_scheduled_tasks.py | 7 +- tests/app/dao/test_monthly_billing.py | 89 ++++++++++-------------- tests/app/service/test_rest.py | 4 +- 6 files changed, 78 insertions(+), 92 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index e83fbf751..4efef0e7d 100644 --- 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 diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 1d9346df2..7b4807f0e 100644 --- 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(): diff --git a/app/service/rest.py b/app/service/rest.py index 743fa5d25..8b947b36b 100644 --- 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 diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 10774d301..2e20ee138 100644 --- 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 diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 044b497b7..4a56da2a6 100644 --- 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( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 005c003fc..48225ab2f 100644 --- 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):