From 5612ca023e094c8eecb9e42761d3c289ca5d85e7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 14:26:42 +0100 Subject: [PATCH 1/2] - Add transactional - Rename function for clarity --- app/dao/monthly_billing_dao.py | 5 ++++- app/dao/notification_usage_dao.py | 10 +++++----- tests/app/dao/test_notification_usage_dao.py | 16 ++++++++-------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index cd51c19d3..49a571e5c 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -1,9 +1,11 @@ from datetime import datetime from app import db +from app.dao.dao_utils import transactional from app.dao.date_util import get_month_start_end_date from app.dao.notification_usage_dao import get_billing_data_for_month from app.models import MonthlyBilling, SMS_TYPE, NotificationHistory +from app.statsd_decorators import statsd def get_service_ids_that_need_sms_billing_populated(start_date, end_date): @@ -17,6 +19,7 @@ def get_service_ids_that_need_sms_billing_populated(start_date, end_date): ).distinct().all() +@transactional def create_or_update_monthly_billing_sms(service_id, billing_month): start_date, end_date = get_month_start_end_date(billing_month) monthly = get_billing_data_for_month(service_id=service_id, start_date=start_date, end_date=end_date) @@ -34,9 +37,9 @@ def create_or_update_monthly_billing_sms(service_id, billing_month): month=datetime.strftime(billing_month, "%B"), monthly_totals=monthly_totals) db.session.add(row) - db.session.commit() +@statsd(namespace="dao") def get_monthly_billing_sms(service_id, billing_month): monthly = MonthlyBilling.query.filter_by(service_id=service_id, year=billing_month.year, diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 43fb6a6cd..7cd6517e9 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -20,7 +20,7 @@ from app.utils import get_london_month_from_utc_column @statsd(namespace="dao") def get_yearly_billing_data(service_id, year): start_date, end_date = get_financial_year(year) - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) def get_valid_from(valid_from): return start_date if valid_from < start_date else valid_from @@ -37,7 +37,7 @@ def get_yearly_billing_data(service_id, year): @statsd(namespace="dao") def get_billing_data_for_month(service_id, start_date, end_date): - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) result = [] # so the start end date in the query are the valid from the rate, not the month - this is going to take some thought for r, n in zip(rates, rates[1:]): @@ -52,7 +52,7 @@ def get_billing_data_for_month(service_id, start_date, end_date): @statsd(namespace="dao") def get_monthly_billing_data(service_id, year): start_date, end_date = get_financial_year(year) - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) result = [] for r, n in zip(rates, rates[1:]): @@ -117,7 +117,7 @@ def sms_yearly_billing_data_query(rate, service_id, start_date, end_date): return result -def get_rates_for_year(start_date, end_date, notification_type): +def get_rates_for_daterange(start_date, end_date, notification_type): rates = Rate.query.filter(Rate.notification_type == notification_type).order_by(Rate.valid_from).all() results = [] for current_rate, current_rate_expiry_date in zip(rates, rates[1:]): @@ -207,7 +207,7 @@ def get_total_billable_units_for_sent_sms_notifications_in_date_range(start_date def discover_rate_bounds_for_billing_query(start_date, end_date): bounds = [] - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) def current_valid_from(index): return rates[index].valid_from diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 83bfa264c..1555bad77 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -6,7 +6,7 @@ from flask import current_app from app.dao.date_util import get_financial_year from app.dao.notification_usage_dao import ( - get_rates_for_year, + get_rates_for_daterange, get_yearly_billing_data, get_monthly_billing_data, get_total_billable_units_for_sent_sms_notifications_in_date_range, @@ -28,7 +28,7 @@ def test_get_rates_for_year(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 3, 31, 23), 0.0158) start_date, end_date = get_financial_year(2017) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 1 assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2017-03-31 23:00:00" assert rates[0].rate == 0.0158 @@ -39,7 +39,7 @@ def test_get_rates_for_year_multiple_result_per_year(notify_db, notify_db_sessio set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 4, 1), 0.0158) start_date, end_date = get_financial_year(2016) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 2 assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2016-04-01 00:00:00" assert rates[0].rate == 0.015 @@ -52,7 +52,7 @@ def test_get_rates_for_year_returns_correct_rates(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2017) - rates_2017 = get_rates_for_year(start_date, end_date, 'sms') + rates_2017 = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates_2017) == 2 assert datetime.strftime(rates_2017[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2016-09-01 00:00:00" assert rates_2017[0].rate == 0.016 @@ -64,7 +64,7 @@ def test_get_rates_for_year_in_the_future(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2018) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2017-06-01 00:00:00" assert rates[0].rate == 0.0175 @@ -73,7 +73,7 @@ def test_get_rates_for_year_returns_empty_list_if_year_is_before_earliest_rate(n set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2015) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert rates == [] @@ -83,7 +83,7 @@ def test_get_rates_for_year_early_rate(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2016) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 3 @@ -91,7 +91,7 @@ def test_get_rates_for_year_edge_case(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 3, 31, 23, 00), 0.015) set_up_rate(notify_db, datetime(2017, 3, 31, 23, 00), 0.0175) start_date, end_date = get_financial_year(2016) - rates = get_rates_for_year(start_date, end_date, 'sms') + rates = get_rates_for_daterange(start_date, end_date, 'sms') assert len(rates) == 1 assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2016-03-31 23:00:00" assert rates[0].rate == 0.015 From e23d38de26132240e36516f7b5d66fe78679b608 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Jul 2017 15:50:14 +0100 Subject: [PATCH 2/2] Fix bug in get rates function. --- app/dao/notification_usage_dao.py | 6 ++-- tests/app/dao/test_monthly_billing.py | 1 + tests/app/dao/test_notification_usage_dao.py | 30 ++++++++++++++------ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 7cd6517e9..79f786dd0 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -129,8 +129,10 @@ def get_rates_for_daterange(start_date, end_date, notification_type): results.append(rates[-1]) if not results: - if start_date >= rates[-1].valid_from: - results.append(rates[-1]) + for x in reversed(rates): + if start_date >= x.valid_from: + results.append(x) + break return results diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 9027ff870..538f19c51 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -13,6 +13,7 @@ def test_add_monthly_billing(sample_template): jan = datetime(2017, 1, 1) feb = datetime(2017, 2, 15) create_rate(start_date=jan, value=0.0158, notification_type='sms') + create_rate(start_date=datetime(2017, 3, 31, 23, 00, 00), value=0.123, notification_type='sms') create_notification(template=sample_template, created_at=jan, billable_units=1, status='delivered') create_notification(template=sample_template, created_at=feb, billable_units=2, status='delivered') diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 1555bad77..9eab4f089 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -24,7 +24,7 @@ from freezegun import freeze_time from tests.conftest import set_config -def test_get_rates_for_year(notify_db, notify_db_session): +def test_get_rates_for_daterange(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 3, 31, 23), 0.0158) start_date, end_date = get_financial_year(2017) @@ -34,7 +34,7 @@ def test_get_rates_for_year(notify_db, notify_db_session): assert rates[0].rate == 0.0158 -def test_get_rates_for_year_multiple_result_per_year(notify_db, notify_db_session): +def test_get_rates_for_daterange_multiple_result_per_year(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2016, 5, 18), 0.016) set_up_rate(notify_db, datetime(2017, 4, 1), 0.0158) @@ -47,7 +47,7 @@ def test_get_rates_for_year_multiple_result_per_year(notify_db, notify_db_sessio assert rates[1].rate == 0.016 -def test_get_rates_for_year_returns_correct_rates(notify_db, notify_db_session): +def test_get_rates_for_daterange_returns_correct_rates(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) @@ -60,7 +60,7 @@ def test_get_rates_for_year_returns_correct_rates(notify_db, notify_db_session): assert rates_2017[1].rate == 0.0175 -def test_get_rates_for_year_in_the_future(notify_db, notify_db_session): +def test_get_rates_for_daterange_in_the_future(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2018) @@ -69,7 +69,7 @@ def test_get_rates_for_year_in_the_future(notify_db, notify_db_session): assert rates[0].rate == 0.0175 -def test_get_rates_for_year_returns_empty_list_if_year_is_before_earliest_rate(notify_db, notify_db_session): +def test_get_rates_for_daterange_returns_empty_list_if_year_is_before_earliest_rate(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 4, 1), 0.015) set_up_rate(notify_db, datetime(2017, 6, 1), 0.0175) start_date, end_date = get_financial_year(2015) @@ -77,7 +77,7 @@ def test_get_rates_for_year_returns_empty_list_if_year_is_before_earliest_rate(n assert rates == [] -def test_get_rates_for_year_early_rate(notify_db, notify_db_session): +def test_get_rates_for_daterange_early_rate(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2015, 6, 1), 0.014) set_up_rate(notify_db, datetime(2016, 6, 1), 0.015) set_up_rate(notify_db, datetime(2016, 9, 1), 0.016) @@ -87,7 +87,7 @@ def test_get_rates_for_year_early_rate(notify_db, notify_db_session): assert len(rates) == 3 -def test_get_rates_for_year_edge_case(notify_db, notify_db_session): +def test_get_rates_for_daterange_edge_case(notify_db, notify_db_session): set_up_rate(notify_db, datetime(2016, 3, 31, 23, 00), 0.015) set_up_rate(notify_db, datetime(2017, 3, 31, 23, 00), 0.0175) start_date, end_date = get_financial_year(2016) @@ -97,6 +97,19 @@ def test_get_rates_for_year_edge_case(notify_db, notify_db_session): assert rates[0].rate == 0.015 +def test_get_rates_for_daterange_where_daterange_is_one_month_that_falls_between_rate_valid_from( + notify_db, notify_db_session +): + set_up_rate(notify_db, datetime(2017, 1, 1), 0.175) + set_up_rate(notify_db, datetime(2017, 3, 31), 0.123) + start_date = datetime(2017, 2, 1, 00, 00, 00) + end_date = datetime(2017, 2, 28, 23, 59, 59, 99999) + rates = get_rates_for_daterange(start_date, end_date, 'sms') + assert len(rates) == 1 + assert datetime.strftime(rates[0].valid_from, '%Y-%m-%d %H:%M:%S') == "2017-01-01 00:00:00" + assert rates[0].rate == 0.175 + + def test_get_yearly_billing_data(notify_db, notify_db_session, sample_template, sample_email_template): set_up_rate(notify_db, datetime(2016, 4, 1), 0.014) set_up_rate(notify_db, datetime(2016, 6, 1), 0.0158) @@ -254,8 +267,7 @@ def test_get_monthly_billing_data_with_multiple_rates(notify_db, notify_db_sessi assert results[3] == ('June', 4, 1, False, 'sms', 0.0175) -def test_get_monthly_billing_data_with_no_notifications_for_year(notify_db, notify_db_session, sample_template, - sample_email_template): +def test_get_monthly_billing_data_with_no_notifications_for_daterange(notify_db, notify_db_session, sample_template): set_up_rate(notify_db, datetime(2016, 4, 1), 0.014) results = get_monthly_billing_data(sample_template.service_id, 2016) assert len(results) == 0