From 19b09f2a2717c5faf3205d4694c35e5643b91a41 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 16:24:48 +0100 Subject: [PATCH 1/9] Rename method to convert from utc to bst for consistency --- .../performance_platform/performance_platform_client.py | 4 ++-- app/models.py | 7 +++++-- app/utils.py | 2 +- tests/app/test_utils.py | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/clients/performance_platform/performance_platform_client.py b/app/clients/performance_platform/performance_platform_client.py index d8685f0f3..045991181 100644 --- a/app/clients/performance_platform/performance_platform_client.py +++ b/app/clients/performance_platform/performance_platform_client.py @@ -5,7 +5,7 @@ from datetime import datetime import requests from flask import current_app -from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc, convert_utc_time_in_bst +from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc, convert_utc_to_bst class PerformancePlatformClient: @@ -27,7 +27,7 @@ class PerformancePlatformClient: def send_performance_stats(self, date, channel, count, period): if self.active: payload = { - '_timestamp': convert_utc_time_in_bst(date).isoformat(), + '_timestamp': convert_utc_to_bst(date).isoformat(), 'service': 'govuk-notify', 'channel': channel, 'count': count, diff --git a/app/models.py b/app/models.py index 0f4fddf57..9cd0b12b6 100644 --- a/app/models.py +++ b/app/models.py @@ -28,7 +28,7 @@ from app import ( ) from app.history_meta import Versioned -from app.utils import convert_utc_time_in_bst, convert_bst_to_utc +from app.utils import convert_utc_to_bst, convert_bst_to_utc SMS_TYPE = 'sms' EMAIL_TYPE = 'email' @@ -950,7 +950,7 @@ class Notification(db.Model): }[self.template.template_type].get(self.status, self.status) def serialize_for_csv(self): - created_at_in_bst = convert_utc_time_in_bst(self.created_at) + created_at_in_bst = convert_utc_to_bst(self.created_at) serialized = { "row_number": '' if self.job_row_number is None else self.job_row_number + 1, "recipient": self.to, @@ -1304,3 +1304,6 @@ class MonthlyBilling(db.Model): "notification_type": self.notification_type, "monthly_totals": self.monthly_totals } + + def __repr__(self): + return str(self.serialized()) diff --git a/app/utils.py b/app/utils.py index 3cb69294e..cf4c28f06 100644 --- a/app/utils.py +++ b/app/utils.py @@ -51,7 +51,7 @@ def get_midnight_for_day_before(date): return get_london_midnight_in_utc(day_before) -def convert_utc_time_in_bst(utc_dt): +def convert_utc_to_bst(utc_dt): return pytz.utc.localize(utc_dt).astimezone(local_timezone).replace(tzinfo=None) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 3b795549f..c250759e1 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -4,7 +4,7 @@ import pytest from app.utils import ( get_london_midnight_in_utc, get_midnight_for_day_before, - convert_utc_time_in_bst, + convert_utc_to_bst, convert_bst_to_utc) @@ -35,7 +35,7 @@ def test_get_midnight_for_day_before_returns_expected_date(date, expected_date): (datetime(2017, 5, 12, 14), datetime(2017, 5, 12, 15, 0)) ]) def test_get_utc_in_bst_returns_expected_date(date, expected_date): - ret_date = convert_utc_time_in_bst(date) + ret_date = convert_utc_to_bst(date) assert ret_date == expected_date From 782f3ea6932b2279a9def0fea65ef78237735db5 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 16:29:13 +0100 Subject: [PATCH 2/9] Rename method to get start and end date of a month for clarity --- app/dao/date_util.py | 2 +- .../versions/0112_add_start_end_dates.py | 4 ++-- tests/app/dao/test_date_utils.py | 20 +++++++++---------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/dao/date_util.py b/app/dao/date_util.py index 8019fdecc..5840fb886 100644 --- a/app/dao/date_util.py +++ b/app/dao/date_util.py @@ -20,7 +20,7 @@ def get_april_fools(year): tzinfo=None) -def get_month_start_end_date(month_year): +def get_month_start_and_end_date_in_utc(month_year): """ This function return the start and date of the month_year as UTC, :param month_year: the datetime to calculate the start and end date for that month diff --git a/migrations/versions/0112_add_start_end_dates.py b/migrations/versions/0112_add_start_end_dates.py index cbb96b64c..96f0806c0 100644 --- a/migrations/versions/0112_add_start_end_dates.py +++ b/migrations/versions/0112_add_start_end_dates.py @@ -8,7 +8,7 @@ Create Date: 2017-07-12 13:35:45.636618 from datetime import datetime from alembic import op import sqlalchemy as sa -from app.dao.date_util import get_month_start_end_date +from app.dao.date_util import get_month_start_and_end_date_in_utc down_revision = '0111_drop_old_service_flags' revision = '0112_add_start_end_dates' @@ -24,7 +24,7 @@ def upgrade(): results = conn.execute("Select id, month, year from monthly_billing") res = results.fetchall() for x in res: - start_date, end_date = get_month_start_end_date( + start_date, end_date = get_month_start_and_end_date_in_utc( datetime(int(x.year), datetime.strptime(x.month, '%B').month, 1)) conn.execute("update monthly_billing set start_date = '{}', end_date = '{}' where id = '{}'".format(start_date, end_date, diff --git a/tests/app/dao/test_date_utils.py b/tests/app/dao/test_date_utils.py index 1533b4834..afa2b4897 100644 --- a/tests/app/dao/test_date_utils.py +++ b/tests/app/dao/test_date_utils.py @@ -2,7 +2,7 @@ from datetime import datetime import pytest -from app.dao.date_util import get_financial_year, get_april_fools, get_month_start_end_date +from app.dao.date_util import get_financial_year, get_april_fools, get_month_start_and_end_date_in_utc def test_get_financial_year(): @@ -17,15 +17,15 @@ def test_get_april_fools(): assert april_fools.tzinfo is None -@pytest.mark.parametrize("month, year, expected_start, expected_end", - [ - (7, 2017, datetime(2017, 6, 30, 23, 00, 00), datetime(2017, 7, 31, 22, 59, 59, 99999)), - (2, 2016, datetime(2016, 2, 1, 00, 00, 00), datetime(2016, 2, 29, 23, 59, 59, 99999)), - (2, 2017, datetime(2017, 2, 1, 00, 00, 00), datetime(2017, 2, 28, 23, 59, 59, 99999)), - (9, 2018, datetime(2018, 8, 31, 23, 00, 00), datetime(2018, 9, 30, 22, 59, 59, 99999)), - (12, 2019, datetime(2019, 12, 1, 00, 00, 00), datetime(2019, 12, 31, 23, 59, 59, 99999))]) -def test_get_month_start_end_date(month, year, expected_start, expected_end): +@pytest.mark.parametrize("month, year, expected_start, expected_end", [ + (7, 2017, datetime(2017, 6, 30, 23, 00, 00), datetime(2017, 7, 31, 22, 59, 59, 99999)), + (2, 2016, datetime(2016, 2, 1, 00, 00, 00), datetime(2016, 2, 29, 23, 59, 59, 99999)), + (2, 2017, datetime(2017, 2, 1, 00, 00, 00), datetime(2017, 2, 28, 23, 59, 59, 99999)), + (9, 2018, datetime(2018, 8, 31, 23, 00, 00), datetime(2018, 9, 30, 22, 59, 59, 99999)), + (12, 2019, datetime(2019, 12, 1, 00, 00, 00), datetime(2019, 12, 31, 23, 59, 59, 99999)) +]) +def test_get_month_start_and_end_date_in_utc(month, year, expected_start, expected_end): month_year = datetime(year, month, 10, 13, 30, 00) - result = get_month_start_end_date(month_year) + result = get_month_start_and_end_date_in_utc(month_year) assert result[0] == expected_start assert result[1] == expected_end From 63e11670983d7253d9b8873078972d74ff03ba28 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 16:37:30 +0100 Subject: [PATCH 3/9] Make sure we update the correct month for billing: When populating the monthly billing records on a schedule, we need to ensure the correct month is being updated. As an example, if the current datetime is 31 Mar 2016, 23:00. The BST equivalent is the 1st April. Therefore we need to ensure we update billing for April, not March. This takes care of that. --- app/celery/scheduled_tasks.py | 14 +++-- tests/app/celery/test_scheduled_tasks.py | 72 +++++++++++++++++++----- tests/app/db.py | 38 +++++++++++-- 3 files changed, 99 insertions(+), 25 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 17533ac08..8e0beef4b 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -9,7 +9,7 @@ from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from app import notify_celery from app import performance_platform_client -from app.dao.date_util import get_month_start_end_date +from app.dao.date_util import get_month_start_and_end_date_in_utc from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago from app.dao.jobs_dao import ( @@ -17,8 +17,8 @@ from app.dao.jobs_dao import ( dao_get_jobs_older_than_limited_by ) from app.dao.monthly_billing_dao import ( - get_service_ids_that_need_sms_billing_populated, - create_or_update_monthly_billing_sms + get_service_ids_that_need_billing_populated, + create_or_update_monthly_billing ) from app.dao.notifications_dao import ( dao_timeout_notifications, @@ -37,6 +37,7 @@ from app.notifications.process_notifications import send_notification_to_queue from app.statsd_decorators import statsd from app.celery.tasks import process_job from app.config import QueueNames +from app.utils import convert_utc_to_bst @notify_celery.task(name="remove_csv_files") @@ -297,6 +298,7 @@ def populate_monthly_billing(): # for every service with billable units this month update billing totals for yesterday # this will overwrite the existing amount. yesterday = datetime.utcnow() - timedelta(days=1) - start_date, end_date = get_month_start_end_date(yesterday) - services = get_service_ids_that_need_sms_billing_populated(start_date=start_date, end_date=end_date) - [create_or_update_monthly_billing_sms(service_id=s.service_id, billing_month=yesterday) for s in services] + yesterday_in_bst = convert_utc_to_bst(yesterday) + start_date, end_date = get_month_start_and_end_date_in_utc(yesterday_in_bst) + services = get_service_ids_that_need_billing_populated(start_date=start_date, end_date=end_date) + [create_or_update_monthly_billing(service_id=s.service_id, billing_month=end_date) for s in services] diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 8ce518dfb..10774d301 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -38,7 +38,7 @@ from app.models import ( Service, Template, SMS_TYPE, LETTER_TYPE, MonthlyBilling) -from app.utils import get_london_midnight_in_utc +from app.utils import get_london_midnight_in_utc, convert_utc_to_bst from tests.app.db import create_notification, create_service, create_template, create_job, create_rate from tests.app.conftest import ( sample_job as create_sample_job, @@ -612,27 +612,71 @@ def test_delete_dvla_response_files_older_than_seven_days_does_not_remove_files( @freeze_time("2017-07-12 02:00:00") -def test_populate_monthly_billing(sample_template): +def test_populate_monthly_billing_populates_correctly(sample_template): yesterday = datetime(2017, 7, 11, 13, 30) + jul_month_start = datetime(2017, 6, 30, 23) + jul_month_end = datetime(2017, 7, 31, 22, 59, 59, 99999) create_rate(datetime(2016, 1, 1), 0.0123, 'sms') + create_notification(template=sample_template, status='delivered', created_at=yesterday) create_notification(template=sample_template, status='delivered', created_at=yesterday - timedelta(days=1)) create_notification(template=sample_template, status='delivered', created_at=yesterday + timedelta(days=1)) # not included in billing create_notification(template=sample_template, status='delivered', created_at=yesterday - timedelta(days=30)) - assert len(MonthlyBilling.query.all()) == 0 populate_monthly_billing() - monthly_billing = MonthlyBilling.query.all() - assert len(monthly_billing) == 1 + monthly_billing = MonthlyBilling.query.order_by(MonthlyBilling.notification_type).all() + + assert len(monthly_billing) == 2 + assert monthly_billing[0].service_id == sample_template.service_id - assert monthly_billing[0].start_date == datetime(2017, 6, 30, 23) - assert monthly_billing[0].end_date == datetime(2017, 7, 31, 22, 59, 59, 99999) - assert monthly_billing[0].notification_type == 'sms' - assert len(monthly_billing[0].monthly_totals) == 1 - assert sorted(monthly_billing[0].monthly_totals[0]) == sorted({'international': False, - 'rate_multiplier': 1, - 'billing_units': 3, - 'rate': 0.0123, - 'total_cost': 0.0369}) + assert monthly_billing[0].start_date == jul_month_start + 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[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, + 'rate_multiplier': 1, + 'billing_units': 3, + 'rate': 0.0123, + 'total_cost': 0.0369 + } + ) + + +@freeze_time("2016-04-01 23:00:00") +def test_populate_monthly_billing_updates_correct_month_in_bst(sample_template): + yesterday = datetime.utcnow() - timedelta(days=1) + apr_month_start = datetime(2016, 3, 31, 23) + apr_month_end = datetime(2016, 4, 30, 22, 59, 59, 99999) + create_rate(datetime(2016, 1, 1), 0.0123, 'sms') + create_notification(template=sample_template, status='delivered', created_at=yesterday) + populate_monthly_billing() + + monthly_billing = MonthlyBilling.query.order_by(MonthlyBilling.notification_type).all() + + assert len(monthly_billing) == 2 + + assert monthly_billing[0].service_id == sample_template.service_id + 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[1].service_id == sample_template.service_id + assert monthly_billing[1].start_date == apr_month_start + assert monthly_billing[1].end_date == apr_month_end + assert monthly_billing[1].notification_type == 'sms' + assert monthly_billing[1].monthly_totals[0]['billing_units'] == 1 + assert monthly_billing[1].monthly_totals[0]['total_cost'] == 0.0123 diff --git a/tests/app/db.py b/tests/app/db.py index 6a4f45cbf..676487135 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -6,9 +6,13 @@ from app.dao.jobs_dao import dao_create_job from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ( ApiKey, + EMAIL_TYPE, + SMS_TYPE, + KEY_TYPE_NORMAL, Service, User, Template, + MonthlyBilling, Notification, ScheduledNotification, ServicePermission, @@ -17,10 +21,8 @@ from app.models import ( InboundSms, InboundNumber, Organisation, - EMAIL_TYPE, - SMS_TYPE, - KEY_TYPE_NORMAL, - ServiceInboundApi) + ServiceInboundApi +) from app.dao.users_dao import save_model_user from app.dao.notifications_dao import dao_create_notification, dao_created_scheduled_notification from app.dao.templates_dao import dao_create_template @@ -258,7 +260,12 @@ def create_organisation(colour='blue', logo='test_x2.png', name='test_org_1'): def create_rate(start_date, value, notification_type): - rate = Rate(id=uuid.uuid4(), valid_from=start_date, rate=value, notification_type=notification_type) + rate = Rate( + id=uuid.uuid4(), + valid_from=start_date, + rate=value, + notification_type=notification_type + ) db.session.add(rate) db.session.commit() return rate @@ -290,3 +297,24 @@ def create_inbound_number(number, provider='mmg', active=True, service_id=None): db.session.add(inbound_number) db.session.commit() return inbound_number + + +def create_monthly_billing_entry( + service, + start_date, + end_date, + notification_type, + monthly_totals=[] +): + entry = MonthlyBilling( + service_id=service.id, + notification_type=notification_type, + monthly_totals=monthly_totals, + start_date=start_date, + end_date=end_date + ) + + db.session.add(entry) + db.session.commit() + + return entry From ae76fd0f30a22f592f066016353e1e2f7b914a7f Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 16:49:12 +0100 Subject: [PATCH 4/9] Small refactor to a test --- tests/app/service/test_rest.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 77a3a9d40..005c003fc 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -8,6 +8,7 @@ import pytest from flask import url_for, current_app from freezegun import freeze_time +from app.dao.monthly_billing_dao import create_or_update_monthly_billing from app.dao.services_dao import dao_remove_user_from_service from app.dao.templates_dao import dao_redact_template from app.dao.users_dao import save_model_user @@ -1753,12 +1754,19 @@ def test_get_template_stats_by_month_returns_error_for_incorrect_year( assert json.loads(response.get_data(as_text=True)) == expected_json -def test_get_yearly_billing_usage(client, notify_db, notify_db_session): +def test_get_yearly_billing_usage(client, notify_db, notify_db_session, sample_service): rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type=SMS_TYPE) notify_db.session.add(rate) - notification = create_sample_notification(notify_db, notify_db_session, created_at=datetime(2016, 6, 5), - sent_at=datetime(2016, 6, 5), - status='sending') + after_rate_created = datetime(2016, 6, 5) + notification = create_sample_notification( + notify_db, + notify_db_session, + created_at=after_rate_created, + sent_at=after_rate_created, + status='sending', + service=sample_service + ) + create_or_update_monthly_billing(sample_service.id, after_rate_created) response = client.get( '/service/{}/yearly-usage?year=2016'.format(notification.service_id), headers=[create_authorization_header()] From 839c161628e4735db183a060744e2bf9f3b580bf Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 17:01:48 +0100 Subject: [PATCH 5/9] Update monthly billing for SMS and Email: * Refactor code to update for EMAIL_TYPE and SMS_TYPE * Make tests more robust (overlapping rates, multiple months, multiple noti * types) * --- app/dao/monthly_billing_dao.py | 50 +++- tests/app/dao/test_monthly_billing.py | 410 +++++++++++++++++++------- 2 files changed, 341 insertions(+), 119 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 4ddacddfa..83766e33a 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -2,38 +2,64 @@ 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.date_util import get_month_start_and_end_date_in_utc, get_financial_year from app.dao.notification_usage_dao import get_billing_data_for_month -from app.models import MonthlyBilling, SMS_TYPE, NotificationHistory +from app.models import ( + SMS_TYPE, + EMAIL_TYPE, + MonthlyBilling, + NotificationHistory +) from app.statsd_decorators import statsd +from app.utils import convert_utc_to_bst -def get_service_ids_that_need_sms_billing_populated(start_date, end_date): +def get_service_ids_that_need_billing_populated(start_date, end_date): return db.session.query( NotificationHistory.service_id ).filter( NotificationHistory.created_at >= start_date, NotificationHistory.created_at <= end_date, - NotificationHistory.notification_type == SMS_TYPE, + NotificationHistory.notification_type.in_([SMS_TYPE, EMAIL_TYPE]), NotificationHistory.billable_units != 0 ).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) - # update monthly - monthly_totals = _monthly_billing_data_to_json(monthly) - row = get_monthly_billing_entry(service_id, start_date, SMS_TYPE) +def create_or_update_monthly_billing(service_id, billing_month): + start_date, end_date = get_month_start_and_end_date_in_utc(billing_month) + _update_monthly_billing(service_id, start_date, end_date, SMS_TYPE) + _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] + + +@transactional +def _update_monthly_billing(service_id, start_date, end_date, notification_type): + billing_data = get_billing_data_for_month( + service_id=service_id, + start_date=start_date, + end_date=end_date, + notification_type=notification_type + ) + monthly_totals = _monthly_billing_data_to_json(billing_data) + row = get_monthly_billing_entry(service_id, start_date, notification_type) if row: row.monthly_totals = monthly_totals row.updated_at = datetime.utcnow() else: row = MonthlyBilling( service_id=service_id, - notification_type=SMS_TYPE, + notification_type=notification_type, monthly_totals=monthly_totals, start_date=start_date, end_date=end_date diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index bb40b7f9f..5c6218a58 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -1,19 +1,56 @@ from datetime import datetime, timedelta +from dateutil.relativedelta import relativedelta from freezegun import freeze_time -from freezegun.api import FakeDatetime +from functools import partial from app import db from app.dao.monthly_billing_dao import ( - create_or_update_monthly_billing_sms, + create_or_update_monthly_billing, get_monthly_billing_entry, - get_monthly_billing_sms, - get_service_ids_that_need_sms_billing_populated + get_monthly_billing_by_notification_type, + get_service_ids_that_need_billing_populated, + get_billing_data_for_financial_year ) -from app.models import MonthlyBilling, SMS_TYPE -from tests.app.db import create_notification, create_rate, create_service, create_template +from app.models import MonthlyBilling, SMS_TYPE, EMAIL_TYPE +from tests.app.db import ( + create_notification, + create_rate, + create_service, + create_template, + create_monthly_billing_entry +) + +FEB_2016_MONTH_START = datetime(2016, 2, 1) +FEB_2016_MONTH_END = datetime(2016, 2, 29, 23, 59, 59, 99999) + +MAR_2016_MONTH_START = datetime(2016, 3, 1) +MAR_2016_MONTH_END = datetime(2016, 3, 31, 22, 59, 59, 99999) + +APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00) +APR_2016_MONTH_END = datetime(2016, 4, 30, 22, 59, 59, 99999) + +MAY_2016_MONTH_START = datetime(2016, 5, 31, 23, 00, 00) +MAY_2016_MONTH_END = MAY_2016_MONTH_START + relativedelta(months=1, seconds=-1) + +APR_2017_MONTH_START = datetime(2017, 3, 31, 23, 00, 00) +APR_2017_MONTH_END = datetime(2017, 4, 30, 23, 59, 59, 99999) + +JAN_2017_MONTH_START = datetime(2017, 1, 1) +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, +} -def create_sample_monthly_billing_entry( +def _create_sample_monthly_billing_entry( service_id, monthly_totals, start_date, @@ -33,118 +70,276 @@ def create_sample_monthly_billing_entry( return entry -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_TYPE) - create_rate(start_date=datetime(2017, 3, 31, 23, 00, 00), value=0.123, notification_type=SMS_TYPE) - 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') +def _assert_monthly_billing(monthly_billing, service_id, notification_type, month_start, month_end): + assert monthly_billing.service_id == service_id + assert monthly_billing.notification_type == notification_type + assert monthly_billing.start_date == month_start + assert monthly_billing.end_date == month_end + + +def _assert_monthly_billing_totals(monthly_billing_totals, expected_dict): + assert sorted(monthly_billing_totals.keys()) == sorted(expected_dict.keys()) + assert sorted(monthly_billing_totals.values()) == sorted(expected_dict.values()) + + +def test_get_monthly_billing_by_notification_type_returns_correct_totals(notify_db, notify_db_session): + service = create_service(service_name="Service One") + + _create_sample_monthly_billing_entry( + service_id=service.id, + monthly_totals=[{ + "billing_units": 12, + "rate": 0.0158, + "rate_multiplier": 5, + "total_cost": 2.1804, + "international": False + }], + start_date=APR_2016_MONTH_START, + end_date=APR_2016_MONTH_END, + notification_type=SMS_TYPE + ) + + monthly_billing_data = get_monthly_billing_by_notification_type(service.id, APR_2016, SMS_TYPE) + + _assert_monthly_billing( + monthly_billing_data, service.id, 'sms', APR_2016_MONTH_START, APR_2016_MONTH_END + ) + _assert_monthly_billing_totals(monthly_billing_data.monthly_totals[0], { + "billing_units": 12, + "rate_multiplier": 5, + "international": False, + "rate": 0.0158, + "total_cost": 2.1804 + }) + + +def test_get_monthly_billing_by_notification_type_filters_by_type(notify_db, notify_db_session): + service = create_service(service_name="Service One") + + _create_sample_monthly_billing_entry( + service_id=service.id, + monthly_totals=[{ + "billing_units": 138, + "rate": 0.0158, + "rate_multiplier": 1, + "total_cost": 2.1804, + "international": None + }], + start_date=APR_2016_MONTH_START, + end_date=APR_2016_MONTH_END, + notification_type=SMS_TYPE + ) + + _create_sample_monthly_billing_entry( + service_id=service.id, + monthly_totals=[], + start_date=APR_2016_MONTH_START, + end_date=APR_2016_MONTH_END, + notification_type=EMAIL_TYPE + ) + + monthly_billing_data = get_monthly_billing_by_notification_type(service.id, APR_2016, EMAIL_TYPE) + + _assert_monthly_billing( + monthly_billing_data, service.id, 'email', APR_2016_MONTH_START, APR_2016_MONTH_END + ) + assert monthly_billing_data.monthly_totals == [] + + +def test_get_monthly_billing_by_notification_type_normalises_start_date(notify_db, notify_db_session): + service = create_service(service_name="Service One") + + _create_sample_monthly_billing_entry( + service_id=service.id, + monthly_totals=[{ + "billing_units": 321, + "rate": 0.0158, + "rate_multiplier": 1, + "total_cost": 2.1804, + "international": None + }], + start_date=APR_2016_MONTH_START, + end_date=APR_2016_MONTH_END, + notification_type=SMS_TYPE + ) + + monthly_billing_data = get_monthly_billing_by_notification_type(service.id, APR_2016 + timedelta(days=5), SMS_TYPE) + + assert monthly_billing_data.start_date == APR_2016_MONTH_START + assert monthly_billing_data.monthly_totals[0]['billing_units'] == 321 + + +def test_add_monthly_billing_for_single_month_populates_correctly( + sample_template, sample_email_template +): + create_rate(start_date=JAN_2017_MONTH_START, value=0.0158, notification_type=SMS_TYPE) + create_notification( + template=sample_template, created_at=JAN_2017_MONTH_START, + billable_units=1, rate_multiplier=2, status='delivered' + ) + + create_or_update_monthly_billing( + service_id=sample_template.service_id, + billing_month=JAN_2017_MONTH_START + ) + + monthly_billing = MonthlyBilling.query.order_by(MonthlyBilling.notification_type).all() - create_or_update_monthly_billing_sms(service_id=sample_template.service_id, - billing_month=jan) - create_or_update_monthly_billing_sms(service_id=sample_template.service_id, - billing_month=feb) - monthly_billing = MonthlyBilling.query.all() assert len(monthly_billing) == 2 - assert monthly_billing[0].start_date == datetime(2017, 1, 1) - assert monthly_billing[1].start_date == datetime(2017, 2, 1) + _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 + }) - january = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=jan) - expected_jan = {"billing_units": 1, - "rate_multiplier": 1, - "international": False, - "rate": 0.0158, - "total_cost": 1 * 0.0158} - assert_monthly_billing(january, sample_template.service_id, 1, expected_jan, - start_date=datetime(2017, 1, 1), end_date=datetime(2017, 1, 31)) - - february = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=feb) - expected_feb = {"billing_units": 2, - "rate_multiplier": 1, - "international": False, - "rate": 0.0158, - "total_cost": 2 * 0.0158} - assert_monthly_billing(february, sample_template.service_id, 1, expected_feb, - start_date=datetime(2017, 2, 1), end_date=datetime(2017, 2, 28)) + _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": 2, + "international": False, + "rate": 0.0158, + "total_cost": 1 * 2 * 0.0158 + }) -def test_add_monthly_billing_multiple_rates_in_a_month(sample_template): - rate_1 = datetime(2016, 12, 1) - rate_2 = datetime(2017, 1, 15) - create_rate(start_date=rate_1, value=0.0158, notification_type=SMS_TYPE) - create_rate(start_date=rate_2, value=0.0124, notification_type=SMS_TYPE) +def test_add_monthly_billing_for_multiple_months_populate_correctly( + sample_template, sample_email_template +): + create_rate(start_date=FEB_2016_MONTH_START - timedelta(days=1), value=0.12, notification_type=SMS_TYPE) + create_notification( + template=sample_template, created_at=FEB_2016_MONTH_START, + billable_units=1, rate_multiplier=2, status='delivered' + ) + create_notification( + template=sample_template, created_at=MAR_2016_MONTH_START, + billable_units=2, rate_multiplier=3, status='delivered' + ) - create_notification(template=sample_template, created_at=datetime(2017, 1, 1), billable_units=1, status='delivered') - create_notification(template=sample_template, created_at=datetime(2017, 1, 14, 23, 59), billable_units=1, - status='delivered') + 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) - create_notification(template=sample_template, created_at=datetime(2017, 1, 15), billable_units=2, - status='delivered') - create_notification(template=sample_template, created_at=datetime(2017, 1, 17, 13, 30, 57), billable_units=4, - status='delivered') - - create_or_update_monthly_billing_sms(service_id=sample_template.service_id, - billing_month=rate_2) monthly_billing = MonthlyBilling.query.all() - assert len(monthly_billing) == 1 - assert monthly_billing[0].start_date == datetime(2017, 1, 1) - january = get_monthly_billing_sms(service_id=sample_template.service_id, billing_month=rate_2) - first_row = {"billing_units": 2, - "rate_multiplier": 1, - "international": False, - "rate": 0.0158, - "total_cost": 3 * 0.0158} - assert_monthly_billing(january, sample_template.service_id, 2, first_row, - start_date=datetime(2017, 1, 1), end_date=datetime(2017, 1, 1)) - second_row = {"billing_units": 6, - "rate_multiplier": 1, - "international": False, - "rate": 0.0124, - "total_cost": 1 * 0.0124} - assert sorted(january.monthly_totals[1]) == sorted(second_row) + assert len(monthly_billing) == 4 + _assert_monthly_billing( + monthly_billing[0], sample_template.service.id, 'sms', FEB_2016_MONTH_START, FEB_2016_MONTH_END + ) + _assert_monthly_billing_totals(monthly_billing[0].monthly_totals[0], { + "billing_units": 1, + "rate_multiplier": 2, + "international": False, + "rate": 0.12, + "total_cost": 0.24 + }) + + _assert_monthly_billing( + monthly_billing[1], sample_template.service.id, 'email', FEB_2016_MONTH_START, FEB_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], { + "billing_units": 2, + "rate_multiplier": 3, + "international": False, + "rate": 0.12, + "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 +): + create_rate(start_date=JAN_2017_MONTH_START, value=0.0158, notification_type=SMS_TYPE) + create_rate(start_date=JAN_2017_MONTH_START + timedelta(days=5), value=0.123, notification_type=SMS_TYPE) + create_notification(template=sample_template, created_at=JAN_2017_MONTH_START, billable_units=1, status='delivered') + create_notification( + template=sample_template, created_at=JAN_2017_MONTH_START + timedelta(days=6), + billable_units=2, status='delivered' + ) + + create_or_update_monthly_billing(service_id=sample_template.service_id, billing_month=JAN_2017_MONTH_START) + + monthly_billing = MonthlyBilling.query.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 + ) + _assert_monthly_billing_totals(monthly_billing[0].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], { + "billing_units": 2, + "rate_multiplier": 1, + "international": False, + "rate": 0.123, + "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): - july = datetime(2017, 7, 1) - create_rate(july, 0.123, SMS_TYPE) - create_notification(template=sample_template, created_at=datetime(2017, 7, 2), billable_units=1, status='delivered') - with freeze_time('2017-07-20 02:30:00'): - create_or_update_monthly_billing_sms(sample_template.service_id, july) - first_update = get_monthly_billing_sms(sample_template.service_id, july) - expected = {"billing_units": 1, - "rate_multiplier": 1, - "international": False, - "rate": 0.123, - "total_cost": 1 * 0.123} - assert_monthly_billing(first_update, sample_template.service_id, 1, expected, - start_date=datetime(2017, 6, 30, 23), end_date=datetime(2017, 7, 31, 23, 59, 59, 99999)) - first_updated_at = first_update.updated_at - with freeze_time('2017-07-20 03:30:00'): - create_notification(template=sample_template, created_at=datetime(2017, 7, 5), billable_units=2, - status='delivered') + create_rate(APR_2016_MONTH_START, 0.123, SMS_TYPE) + create_notification(template=sample_template, created_at=APR_2016_MONTH_START, billable_units=1, status='delivered') - create_or_update_monthly_billing_sms(sample_template.service_id, july) - second_update = get_monthly_billing_sms(sample_template.service_id, july) - expected_update = {"billing_units": 3, - "rate_multiplier": 1, - "international": False, - "rate": 0.123, - "total_cost": 3 * 0.123} - assert_monthly_billing(second_update, sample_template.service_id, 1, expected_update, - start_date=datetime(2017, 6, 30, 23), end_date=datetime(2017, 7, 31, 23, 59, 59, 99999)) - assert second_update.updated_at == FakeDatetime(2017, 7, 20, 3, 30) + create_or_update_monthly_billing(sample_template.service_id, APR_2016_MONTH_END) + first_update = get_monthly_billing_by_notification_type(sample_template.service_id, APR_2016_MONTH_START, SMS_TYPE) + + _assert_monthly_billing( + first_update, sample_template.service.id, 'sms', APR_2016_MONTH_START, APR_2016_MONTH_END + ) + _assert_monthly_billing_totals(first_update.monthly_totals[0], { + "billing_units": 1, + "rate_multiplier": 1, + "international": False, + "rate": 0.123, + "total_cost": 0.123 + }) + + first_updated_at = first_update.updated_at + + with freeze_time(APR_2016_MONTH_START + timedelta(days=3)): + create_notification(template=sample_template, billable_units=2, status='delivered') + create_or_update_monthly_billing(sample_template.service_id, APR_2016_MONTH_END) + + second_update = get_monthly_billing_by_notification_type(sample_template.service_id, APR_2016_MONTH_START, SMS_TYPE) + + _assert_monthly_billing_totals(second_update.monthly_totals[0], { + "billing_units": 3, + "rate_multiplier": 1, + "international": False, + "rate": 0.123, + "total_cost": 0.369 + }) + + assert second_update.updated_at == APR_2016_MONTH_START + timedelta(days=3) assert first_updated_at != second_update.updated_at -def assert_monthly_billing(monthly_billing, service_id, expected_len, first_row, start_date, end_date): - assert monthly_billing.service_id == service_id - assert len(monthly_billing.monthly_totals) == expected_len - assert sorted(monthly_billing.monthly_totals[0]) == sorted(first_row) - - -def test_get_service_id(notify_db_session): +def test_get_service_ids_that_need_billing_populated_return_correctly(notify_db_session): service_1 = create_service(service_name="Service One") template_1 = create_template(service=service_1) service_2 = create_service(service_name="Service Two") @@ -153,8 +348,9 @@ def test_get_service_id(notify_db_session): create_notification(template=template_1, created_at=datetime(2017, 7, 1, 14, 30), status='delivered') create_notification(template=template_2, created_at=datetime(2017, 7, 15, 13, 30)) create_notification(template=template_2, created_at=datetime(2017, 7, 31, 13, 30)) - services = get_service_ids_that_need_sms_billing_populated(start_date=datetime(2017, 7, 1), - end_date=datetime(2017, 7, 16)) + services = get_service_ids_that_need_billing_populated( + start_date=datetime(2017, 7, 1), end_date=datetime(2017, 7, 16) + ) expected_services = [service_1.id, service_2.id] assert sorted([x.service_id for x in services]) == sorted(expected_services) @@ -164,14 +360,14 @@ def test_get_monthly_billing_entry_filters_by_service(notify_db, notify_db_sessi service_2 = create_service(service_name="Service Two") now = datetime.utcnow() - create_sample_monthly_billing_entry( + _create_sample_monthly_billing_entry( service_id=service_1.id, monthly_totals=[], start_date=now, end_date=now + timedelta(days=30) ) - create_sample_monthly_billing_entry( + _create_sample_monthly_billing_entry( service_id=service_2.id, monthly_totals=[], start_date=now, From 35ad5d74f977e00f0d7aed3af2ff6a5851e3f413 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 17:06:31 +0100 Subject: [PATCH 6/9] Retrieve billing data (for month) by notification type: * Previously we were only returning SMS. Let's make this return for a given notification type. * General refactor of the monthly retrieval query * Return an empty BillingData tuple if email billing data is empty --- app/dao/notification_usage_dao.py | 82 +++++++++++++------- tests/app/dao/test_notification_usage_dao.py | 5 +- 2 files changed, 59 insertions(+), 28 deletions(-) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index cf90fe499..1d9346df2 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -1,20 +1,23 @@ +from collections import namedtuple from datetime import datetime, timedelta -from flask import current_app from sqlalchemy import Float, Integer from sqlalchemy import func, case, cast from sqlalchemy import literal_column from app import db -from app.dao.date_util import get_financial_year, get_month_start_end_date -from app.models import (NotificationHistory, - Rate, - NOTIFICATION_STATUS_TYPES_BILLABLE, - KEY_TYPE_TEST, - SMS_TYPE, - EMAIL_TYPE, Service) +from app.dao.date_util import get_financial_year +from app.models import ( + NotificationHistory, + Rate, + Service, + NOTIFICATION_STATUS_TYPES_BILLABLE, + KEY_TYPE_TEST, + SMS_TYPE, + EMAIL_TYPE +) from app.statsd_decorators import statsd -from app.utils import get_london_month_from_utc_column +from app.utils import get_london_month_from_utc_column, convert_utc_to_bst @statsd(namespace="dao") @@ -48,26 +51,39 @@ def get_yearly_billing_data(service_id, year): ) result.append(email_yearly_billing_data_query(service_id, start_date, end_date)) - return sum(result, []) @statsd(namespace="dao") -def get_billing_data_for_month(service_id, start_date, end_date): - rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) +def get_billing_data_for_month(service_id, start_date, end_date, notification_type): + results = [] - if not rates: - return [] + if notification_type == EMAIL_TYPE: + return billing_data_per_month_query(0, service_id, start_date, end_date, EMAIL_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:]): - result.extend(sms_billing_data_per_month_query(r.rate, service_id, max(r.valid_from, start_date), - min(n.valid_from, end_date))) - result.extend( - sms_billing_data_per_month_query(rates[-1].rate, service_id, max(rates[-1].valid_from, start_date), end_date)) + elif notification_type == SMS_TYPE: + rates = get_rates_for_daterange(start_date, end_date, SMS_TYPE) - return result + if not rates: + return [] + + # 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:]): + results.extend( + billing_data_per_month_query( + r.rate, service_id, max(r.valid_from, start_date), + min(n.valid_from, end_date), SMS_TYPE + ) + ) + results.extend( + billing_data_per_month_query( + rates[-1].rate, service_id, max(rates[-1].valid_from, start_date), + end_date, SMS_TYPE + ) + ) + + return results @statsd(namespace="dao") @@ -80,8 +96,8 @@ def get_monthly_billing_data(service_id, year): result = [] for r, n in zip(rates, rates[1:]): - result.extend(sms_billing_data_per_month_query(r.rate, service_id, r.valid_from, n.valid_from)) - result.extend(sms_billing_data_per_month_query(rates[-1].rate, service_id, rates[-1].valid_from, end_date)) + result.extend(billing_data_per_month_query(r.rate, service_id, r.valid_from, n.valid_from, SMS_TYPE)) + result.extend(billing_data_per_month_query(rates[-1].rate, service_id, rates[-1].valid_from, end_date, SMS_TYPE)) return [(datetime.strftime(x[0], "%B"), x[1], x[2], x[3], x[4], x[5]) for x in result] @@ -143,6 +159,10 @@ def sms_yearly_billing_data_query(rate, service_id, start_date, end_date): 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() + + if not rates: + return [] + results = [] for current_rate, current_rate_expiry_date in zip(rates, rates[1:]): if is_between(current_rate.valid_from, start_date, end_date) or \ @@ -165,7 +185,7 @@ def is_between(date, start_date, end_date): return start_date <= date <= end_date -def sms_billing_data_per_month_query(rate, service_id, 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( month.label('month'), @@ -175,7 +195,7 @@ def sms_billing_data_per_month_query(rate, service_id, start_date, end_date): NotificationHistory.notification_type, cast(rate, Float()).label('rate') ).filter( - *billing_data_filter(SMS_TYPE, start_date, end_date, service_id) + *billing_data_filter(notification_type, start_date, end_date, service_id) ).group_by( NotificationHistory.notification_type, month, @@ -186,6 +206,16 @@ def sms_billing_data_per_month_query(rate, service_id, start_date, end_date): 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 diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 394c71540..2de3c7e94 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -277,7 +277,7 @@ def test_get_monthly_billing_data_with_multiple_rates(notify_db, notify_db_sessi 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 + assert results == [] def set_up_rate(notify_db, start_date, value): @@ -768,7 +768,8 @@ def test_get_monthly_billing_data_where_start_date_before_rate_returns_empty( results = get_billing_data_for_month( service_id=sample_template.service_id, start_date=now - timedelta(days=2), - end_date=now - timedelta(days=1) + end_date=now - timedelta(days=1), + notification_type=SMS_TYPE ) assert not results From 69845e9f3791a1d02e610770c55c84cfd8ff95d1 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 17:09:47 +0100 Subject: [PATCH 7/9] Add helper methods to retrieve billing data from the new table: Get yearly and monthly billing data from the new table with filter by notification type. Additionally let's calculate current day delta totals --- app/dao/monthly_billing_dao.py | 47 +++++++---- tests/app/dao/test_monthly_billing.py | 111 ++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 14 deletions(-) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 83766e33a..e83fbf751 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -79,19 +79,38 @@ def get_monthly_billing_entry(service_id, start_date, notification_type): @statsd(namespace="dao") -def get_monthly_billing_sms(service_id, billing_month): - start_date, end_date = get_month_start_end_date(billing_month) - monthly = MonthlyBilling.query.filter_by(service_id=service_id, - start_date=start_date, - notification_type=SMS_TYPE).first() - return monthly +def get_yearly_billing_data_for_date_range( + service_id, start_date, end_date, notification_types +): + results = MonthlyBilling.query.filter( + MonthlyBilling.service_id == service_id, + MonthlyBilling.start_date >= start_date, + MonthlyBilling.end_date <= end_date, + MonthlyBilling.notification_type.in_(notification_types) + ).order_by( + MonthlyBilling.notification_type + ).all() + + return results -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] +@statsd(namespace="dao") +def get_monthly_billing_by_notification_type(service_id, billing_month, notification_type): + billing_month_in_bst = convert_utc_to_bst(billing_month) + start_date, _ = get_month_start_and_end_date_in_utc(billing_month_in_bst) + return get_monthly_billing_entry(service_id, start_date, notification_type) + + +@statsd(namespace="dao") +def get_billing_data_for_financial_year(service_id, year, notification_types=[SMS_TYPE, EMAIL_TYPE]): + # Update totals to the latest so we include data for today + now = convert_utc_to_bst(datetime.utcnow()) + create_or_update_monthly_billing(service_id=service_id, billing_month=now) + + start_date, end_date = get_financial_year(year) + + results = get_yearly_billing_data_for_date_range( + service_id, start_date, end_date, notification_types + ) + + return results diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 5c6218a58..044b497b7 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -378,3 +378,114 @@ def test_get_monthly_billing_entry_filters_by_service(notify_db, notify_db_sessi assert entry.start_date == now assert entry.service_id == service_2.id + + +def test_get_yearly_billing_data_for_year_returns_within_year_only( + sample_template +): + monthly_billing_entry = partial( + create_monthly_billing_entry, service=sample_template.service, notification_type=SMS_TYPE + ) + monthly_billing_entry(start_date=FEB_2016_MONTH_START, end_date=FEB_2016_MONTH_END) + monthly_billing_entry( + monthly_totals=[{ + "billing_units": 138, + "rate": 0.0158, + "rate_multiplier": 1, + "total_cost": 2.1804, + "international": None + }], + start_date=APR_2016_MONTH_START, + end_date=APR_2016_MONTH_END, + notification_type=SMS_TYPE + ) + monthly_billing_entry(start_date=APR_2017_MONTH_START, end_date=APR_2017_MONTH_END) + + billing_data = get_billing_data_for_financial_year(sample_template.service.id, 2016, [SMS_TYPE]) + + assert len(billing_data) == 1 + assert billing_data[0].monthly_totals[0]['billing_units'] == 138 + + +def test_get_yearly_billing_data_for_year_returns_multiple_notification_types(sample_template): + monthly_billing_entry = partial( + create_monthly_billing_entry, service=sample_template.service, + start_date=APR_2016_MONTH_START, end_date=APR_2016_MONTH_END + ) + + monthly_billing_entry( + notification_type=SMS_TYPE, monthly_totals=[] + ) + monthly_billing_entry( + notification_type=EMAIL_TYPE, + monthly_totals=[{ + "billing_units": 2, + "rate": 1.3, + "rate_multiplier": 3, + "total_cost": 2.1804, + "international": False + }] + ) + + billing_data = get_billing_data_for_financial_year( + service_id=sample_template.service.id, + year=2016, + notification_types=[SMS_TYPE, EMAIL_TYPE] + ) + + 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 == [] + + +@freeze_time("2016-04-21 11:00:00") +def test_get_yearly_billing_data_for_year_includes_current_day_totals(sample_template): + create_rate(start_date=FEB_2016_MONTH_START, value=0.0158, notification_type=SMS_TYPE) + + create_monthly_billing_entry( + service=sample_template.service, + start_date=APR_2016_MONTH_START, + end_date=APR_2016_MONTH_END, + notification_type=SMS_TYPE + ) + + billing_data = get_billing_data_for_financial_year( + service_id=sample_template.service.id, + year=2016, + notification_types=[SMS_TYPE] + ) + + 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].monthly_totals == [] + + create_notification( + template=sample_template, + created_at=datetime.utcnow(), + sent_at=datetime.utcnow(), + status='sending', + billable_units=3 + ) + + billing_data = get_billing_data_for_financial_year( + service_id=sample_template.service.id, + year=2016, + notification_types=[SMS_TYPE] + ) + + assert billing_data[0].monthly_totals[0]['billing_units'] == 3 From 46ca086aa26dc1bdf322e0ef7c81dc44d059adc5 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 10 Aug 2017 17:12:03 +0100 Subject: [PATCH 8/9] Update command to populate monthly billing data for email and sms --- app/commands.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/commands.py b/app/commands.py index a2e692ebd..731786cd8 100644 --- a/app/commands.py +++ b/app/commands.py @@ -5,8 +5,8 @@ from flask.ext.script import Command, Manager, Option from app import db -from app.dao.monthly_billing_dao import create_or_update_monthly_billing_sms, get_monthly_billing_sms -from app.models import (PROVIDERS, User) +from app.dao.monthly_billing_dao import create_or_update_monthly_billing, get_monthly_billing_by_notification_type +from app.models import PROVIDERS, User, SMS_TYPE, EMAIL_TYPE from app.dao.services_dao import ( delete_service_and_all_associated_db_objects, dao_fetch_all_services_by_user @@ -166,7 +166,13 @@ class PopulateMonthlyBilling(Command): self.populate(service_id, year, i) def populate(self, service_id, year, month): - create_or_update_monthly_billing_sms(service_id, datetime(int(year), int(month), 1)) - results = get_monthly_billing_sms(service_id, datetime(int(year), int(month), 1)) + create_or_update_monthly_billing(service_id, datetime(int(year), int(month), 1)) + sms_res = get_monthly_billing_by_notification_type( + service_id, datetime(int(year), int(month), 1), SMS_TYPE + ) + email_res = get_monthly_billing_by_notification_type( + service_id, datetime(int(year), int(month), 1), EMAIL_TYPE + ) print("Finished populating data for {} for service id {}".format(month, service_id)) - print(results.monthly_totals) + print('SMS: {}'.format(sms_res.monthly_totals)) + print('Email: {}'.format(email_res.monthly_totals)) From 94605d31fa79f19ec44f7f75f8ac31bbd7f7e625 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 11 Aug 2017 16:53:12 +0100 Subject: [PATCH 9/9] 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):