From 296c4d30ffd95b49f3f8531ffecbf89b16c0ca48 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sat, 24 Mar 2018 17:12:58 +0000 Subject: [PATCH 01/41] Update celery from 3.1.25 to 3.1.26.post2 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ed1e9deea..887c878b4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ cffi==1.11.5 -celery==3.1.25 # pyup: <4 +celery==3.1.26.post2 # pyup: <4 docopt==0.6.2 Flask-Bcrypt==0.7.1 Flask-Marshmallow==0.8.0 From a3b5e6ec86d071a4508f2876ddd3f3d4fc6a5c59 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Fri, 20 Apr 2018 22:04:37 +0100 Subject: [PATCH 02/41] Update sqlalchemy from 1.2.6 to 1.2.7 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index cbfe6d53b..4d860ebe3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,7 @@ marshmallow==2.15.0 monotonic==1.4 psycopg2-binary==2.7.4 PyJWT==1.6.1 -SQLAlchemy==1.2.6 +SQLAlchemy==1.2.7 notifications-python-client==4.8.1 From f924918d1e3a511f23366e6da5eba9416a8a76f6 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Mon, 30 Apr 2018 19:50:55 +0100 Subject: [PATCH 03/41] Update gunicorn from 19.7.1 to 19.8.1 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b210ee871..ccb726ebe 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,7 @@ Flask-SQLAlchemy==2.3.2 Flask==0.12.2 click-datetime==0.2 eventlet==0.22.1 -gunicorn==19.7.1 +gunicorn==19.8.1 iso8601==0.1.12 jsonschema==2.6.0 marshmallow-sqlalchemy==0.13.2 From 0fb9c1d3180116a953aaab61ba05495446a7c968 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 27 Apr 2018 15:15:55 +0100 Subject: [PATCH 04/41] Add notification_type to query --- app/billing/rest.py | 29 ++++++++ app/dao/fact_billing_dao.py | 12 +++- tests/app/dao/test_ft_billing_dao.py | 101 +++++++++++++++++++++------ 3 files changed, 119 insertions(+), 23 deletions(-) diff --git a/app/billing/rest.py b/app/billing/rest.py index cd7bc3864..82474014f 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -3,6 +3,7 @@ import json from flask import Blueprint, jsonify, request +from app.dao.fact_billing_dao import fetch_monthly_billing_for_year from app.dao.monthly_billing_dao import ( get_billing_data_for_financial_year, get_monthly_billing_by_notification_type @@ -30,6 +31,16 @@ billing_blueprint = Blueprint( register_errors(billing_blueprint) +@billing_blueprint.route('/ft-monthly-usage') +def get_yearly_usage_by_monthy_from_ft_billing(service_id): + try: + year = int(request.args.get('year')) + results = fetch_monthly_billing_for_year(service_id=service_id, year=year) + serialize_ft_billing(results) + except TypeError: + return jsonify(result='error', message='No valid year provided'), 400 + + @billing_blueprint.route('/monthly-usage') def get_yearly_usage_by_month(service_id): try: @@ -192,3 +203,21 @@ def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, fin free_sms_fragment_limit, financial_year_start ) + + +def serialize_ft_billing(data): + results = [] + + for d in data: + j = { + "Month": d.month, + "service_id": d.service_id, + "notifications_type": d.notification_type, + "notifications_sent": d.notifications_sent, + "billable_units": d.billable_units, + "rate": d.rate, + "rate_multiplier": d.rate_multiplier, + "international": d.international, + } + results.append(j) + return results diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index bc6f7d357..538d23ef2 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -20,7 +20,7 @@ from app.models import ( from app.utils import convert_utc_to_bst, convert_bst_to_utc -def fetch_montly_billing_for_year(service_id, year): +def fetch_monthly_billing_for_year(service_id, year): year_start_date, year_end_date = get_financial_year(year) utcnow = datetime.utcnow() today = convert_utc_to_bst(utcnow).date() @@ -39,7 +39,8 @@ def fetch_montly_billing_for_year(service_id, year): FactBilling.service_id, FactBilling.rate, FactBilling.rate_multiplier, - FactBilling.international + FactBilling.international, + FactBilling.notification_type ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start_date, @@ -49,7 +50,12 @@ def fetch_montly_billing_for_year(service_id, year): FactBilling.service_id, FactBilling.rate, FactBilling.rate_multiplier, - FactBilling.international + FactBilling.international, + FactBilling.notification_type + ).order_by( + FactBilling.service_id, + 'Month', + FactBilling.notification_type ).all() return yearly_data diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 4ed43ffc5..8c0d60f94 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -5,7 +5,7 @@ from freezegun import freeze_time from app import db from app.dao.fact_billing_dao import ( - fetch_montly_billing_for_year, fetch_billing_data_for_day, get_rates_for_billing, + fetch_monthly_billing_for_year, fetch_billing_data_for_day, get_rates_for_billing, get_rate ) from app.models import FactBilling @@ -132,6 +132,25 @@ def test_fetch_billing_data_for_day_is_grouped_by_international(notify_db_sessio assert results[1].notifications_sent == 1 +def test_fetch_billing_data_for_day_is_grouped_by_notification_type(notify_db_session): + service = create_service() + sms_template = create_template(service=service, template_type='sms') + email_template = create_template(service=service, template_type='email') + letter_template = create_template(service=service, template_type='letter') + create_notification(template=sms_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=email_template, status='delivered') + create_notification(template=email_template, status='delivered') + create_notification(template=letter_template, status='delivered') + + today = convert_utc_to_bst(datetime.utcnow()) + results = fetch_billing_data_for_day(today) + assert len(results) == 3 + notification_types = [x[2] for x in results if x[2] in ['email', 'sms', 'letter']] + assert len(notification_types) == 3 + + def test_fetch_billing_data_for_day_returns_empty_list(notify_db_session): today = convert_utc_to_bst(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -181,7 +200,7 @@ def test_get_rate(notify_db_session): assert letter_rate == Decimal('4.4') -def test_fetch_annual_billing_for_year(notify_db_session): +def test_fetch_monthly_billing_for_year(notify_db_session): service = create_service() template = create_template(service=service, template_type="sms") for i in range(1, 31): @@ -197,28 +216,30 @@ def test_fetch_annual_billing_for_year(notify_db_session): notification_type='sms', rate=0.158) - results = fetch_montly_billing_for_year(service_id=service.id, year=2018) + results = fetch_monthly_billing_for_year(service_id=service.id, year=2018) assert len(results) == 2 - assert results[0][0] == 6.0 - assert results[0][1] == 30 - assert results[0][2] == Decimal('30') - assert results[0][3] == service.id - assert results[0][4] == Decimal('0.162') - assert results[0][5] == Decimal('1') - assert results[0][6] is False + assert results[0].Month == 6.0 + assert results[0].notifications_sent == 30 + assert results[0].billable_units == Decimal('30') + assert results[0].service_id == service.id + assert results[0].rate == Decimal('0.162') + assert results[0].rate_multiplier == Decimal('1') + assert results[0].international is False + assert results[0].notification_type == 'sms' - assert results[1][0] == 7.0 - assert results[1][1] == 31 - assert results[1][2] == Decimal('31') - assert results[1][3] == service.id - assert results[1][4] == Decimal('0.158') - assert results[1][5] == Decimal('1') - assert results[1][6] is False + assert results[1].Month == 7.0 + assert results[1].notifications_sent == 31 + assert results[1].billable_units == Decimal('31') + assert results[1].service_id == service.id + assert results[1].rate == Decimal('0.158') + assert results[1].rate_multiplier == Decimal('1') + assert results[1].international is False + assert results[1].notification_type == 'sms' @freeze_time('2018-08-01 13:30:00') -def test_fetch_annual_billing_for_year_adds_data_for_today(notify_db_session): +def test_fetch_monthly_billing_for_year_adds_data_for_today(notify_db_session): service = create_service() template = create_template(service=service, template_type="email") for i in range(1, 32): @@ -230,7 +251,47 @@ def test_fetch_annual_billing_for_year_adds_data_for_today(notify_db_session): create_notification(template=template, status='delivered') assert db.session.query(FactBilling.bst_date).count() == 31 - results = fetch_montly_billing_for_year(service_id=service.id, - year=2018) + results = fetch_monthly_billing_for_year(service_id=service.id, + year=2018) assert db.session.query(FactBilling.bst_date).count() == 32 assert len(results) == 2 + + +def test_fetch_monthly_billing_for_year(notify_db_session): + service = create_service() + sms_template = create_template(service=service, template_type="email") + email_template = create_template(service=service, template_type="email") + letter_template = create_template(service=service, template_type="email") + for month in range(1, 13): + mon = str(month).zfill(2) + days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} + for day in range(1, days_in_month[month]): + d = str(day).zfill(2) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=sms_template, + notification_type='sms', + rate=0.162) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=email_template, + notification_type='email', + rate=0) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=letter_template, + notification_type='letter', + rate=0.33) + + results = fetch_monthly_billing_for_year(service.id, 2016) + # returns 3 rows, per month, returns financial year april to december + # Orders by Month + assert len(results) == 27 + assert results[0][0] == 4 + assert results[1][0] == 4 + assert results[2][0] == 4 + assert results[3][0] == 5 + assert results[26][0] == 12 + + + From 18c2b9a56df8b26addc3d8f11a4082a4dafd792e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Apr 2018 16:34:40 +0100 Subject: [PATCH 05/41] Use better date function to get the first of each month. Build the json object to return for the new endpoint. --- app/billing/rest.py | 21 +++++++------ app/dao/fact_billing_dao.py | 8 ++--- tests/app/billing/test_billing.py | 46 +++++++++++++++++++++++++++- tests/app/dao/test_ft_billing_dao.py | 27 ++++++++-------- 4 files changed, 74 insertions(+), 28 deletions(-) diff --git a/app/billing/rest.py b/app/billing/rest.py index 82474014f..6034e1ad4 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -32,14 +32,16 @@ register_errors(billing_blueprint) @billing_blueprint.route('/ft-monthly-usage') -def get_yearly_usage_by_monthy_from_ft_billing(service_id): +def get_yearly_usage_by_monthly_from_ft_billing(service_id): try: year = int(request.args.get('year')) - results = fetch_monthly_billing_for_year(service_id=service_id, year=year) - serialize_ft_billing(results) except TypeError: return jsonify(result='error', message='No valid year provided'), 400 + results = fetch_monthly_billing_for_year(service_id=service_id, year=year) + data = serialize_ft_billing(results) + return jsonify(monthly_usage=data) + @billing_blueprint.route('/monthly-usage') def get_yearly_usage_by_month(service_id): @@ -207,16 +209,15 @@ def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, fin def serialize_ft_billing(data): results = [] - for d in data: j = { - "Month": d.month, - "service_id": d.service_id, + "month": (datetime.strftime(d.month, "%B")), + "service_id": str(d.service_id), "notifications_type": d.notification_type, - "notifications_sent": d.notifications_sent, - "billable_units": d.billable_units, - "rate": d.rate, - "rate_multiplier": d.rate_multiplier, + "notifications_sent": int(d.notifications_sent), + "billable_units": int(d.billable_units), + "rate": float(d.rate), + "rate_multiplier": int(d.rate_multiplier), "international": d.international, } results.append(j) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 538d23ef2..7cc1919c0 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -1,7 +1,7 @@ from datetime import datetime, timedelta, time from flask import current_app -from sqlalchemy import func, case, desc, extract +from sqlalchemy import func, case, desc from app import db from app.dao.date_util import get_financial_year @@ -33,7 +33,7 @@ def fetch_monthly_billing_for_year(service_id, year): update_fact_billing(data=d, process_day=day) yearly_data = db.session.query( - extract('month', FactBilling.bst_date).label("Month"), + func.date_trunc('month', FactBilling.bst_date).label("month"), func.sum(FactBilling.notifications_sent).label("notifications_sent"), func.sum(FactBilling.billable_units).label("billable_units"), FactBilling.service_id, @@ -46,7 +46,7 @@ def fetch_monthly_billing_for_year(service_id, year): FactBilling.bst_date >= year_start_date, FactBilling.bst_date <= year_end_date ).group_by( - 'Month', + 'month', FactBilling.service_id, FactBilling.rate, FactBilling.rate_multiplier, @@ -54,7 +54,7 @@ def fetch_monthly_billing_for_year(service_id, year): FactBilling.notification_type ).order_by( FactBilling.service_id, - 'Month', + 'month', FactBilling.notification_type ).all() diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 8250eaf40..c0cae0a1a 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -17,7 +17,9 @@ from tests.app.db import ( create_monthly_billing_entry, create_annual_billing, create_letter_rate, - create_template + create_template, + create_service, + create_ft_billing ) from app.billing.rest import update_free_sms_fragment_limit_data @@ -409,3 +411,45 @@ def test_update_free_sms_fragment_limit_data(client, sample_service): annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year) assert annual_billing.free_sms_fragment_limit == 9999 + + +def test_get_yearly_usage_by_monthly_from_ft_billing(client, notify_db_session): + service = create_service() + sms_template = create_template(service=service, template_type="sms") + email_template = create_template(service=service, template_type="email") + letter_template = create_template(service=service, template_type="letter") + for month in range(1, 13): + mon = str(month).zfill(2) + days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} + for day in range(1, days_in_month[month]): + d = str(day).zfill(2) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=sms_template, + notification_type='sms', + rate=0.162) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=email_template, + notification_type='email', + rate=0) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=letter_template, + notification_type='letter', + rate=0.33) + + response = client.get('service/{}/billing/ft-monthly-usage?year=2016'.format(service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + json_resp = json.loads(response.get_data(as_text=True)) + + assert json_resp["monthly_usage"][0] == {"month": "April", + "service_id": str(service.id), + "notifications_type": 'email', + "notifications_sent": 30, + "billable_units": 30, + "rate": 0.0, + "rate_multiplier": 1, + "international": False, + } diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 8c0d60f94..5230e72ec 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -219,7 +219,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): results = fetch_monthly_billing_for_year(service_id=service.id, year=2018) assert len(results) == 2 - assert results[0].Month == 6.0 + assert str(results[0].month) == "2018-06-01 00:00:00+01:00" assert results[0].notifications_sent == 30 assert results[0].billable_units == Decimal('30') assert results[0].service_id == service.id @@ -228,7 +228,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert results[0].international is False assert results[0].notification_type == 'sms' - assert results[1].Month == 7.0 + assert str(results[1].month) == "2018-07-01 00:00:00+01:00" assert results[1].notifications_sent == 31 assert results[1].billable_units == Decimal('31') assert results[1].service_id == service.id @@ -257,11 +257,11 @@ def test_fetch_monthly_billing_for_year_adds_data_for_today(notify_db_session): assert len(results) == 2 -def test_fetch_monthly_billing_for_year(notify_db_session): +def test_fetch_monthly_billing_for_year_return_financial_year(notify_db_session): service = create_service() - sms_template = create_template(service=service, template_type="email") + sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") - letter_template = create_template(service=service, template_type="email") + letter_template = create_template(service=service, template_type="letter") for month in range(1, 13): mon = str(month).zfill(2) days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} @@ -286,12 +286,13 @@ def test_fetch_monthly_billing_for_year(notify_db_session): results = fetch_monthly_billing_for_year(service.id, 2016) # returns 3 rows, per month, returns financial year april to december # Orders by Month + assert len(results) == 27 - assert results[0][0] == 4 - assert results[1][0] == 4 - assert results[2][0] == 4 - assert results[3][0] == 5 - assert results[26][0] == 12 - - - + assert str(results[0].month) == "2016-04-01 00:00:00+01:00" + assert results[0].notification_type == 'email' + assert str(results[1].month) == "2016-04-01 00:00:00+01:00" + assert results[1].notification_type == 'letter' + assert str(results[2].month) == "2016-04-01 00:00:00+01:00" + assert results[2].notification_type == 'sms' + assert str(results[3].month) == "2016-05-01 00:00:00+01:00" + assert str(results[26].month) == "2016-12-01 00:00:00+00:00" From ea3523199a6b61493230004274b74ecaa27f74c5 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 4 May 2018 13:09:14 +0100 Subject: [PATCH 06/41] New endpoint to get monthly billing usage from the ft_billing table. New command to compare the results of monthly billing to ft_billing. --- app/billing/rest.py | 22 ++-- app/commands.py | 40 ++++++- app/dao/fact_billing_dao.py | 12 +-- app/dao/monthly_billing_dao.py | 3 +- tests/app/billing/test_billing.py | 150 ++++++++++++++++++++++----- tests/app/dao/test_ft_billing_dao.py | 7 +- 6 files changed, 182 insertions(+), 52 deletions(-) diff --git a/app/billing/rest.py b/app/billing/rest.py index 6034e1ad4..cb6eb4cc6 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -37,10 +37,9 @@ def get_yearly_usage_by_monthly_from_ft_billing(service_id): year = int(request.args.get('year')) except TypeError: return jsonify(result='error', message='No valid year provided'), 400 - results = fetch_monthly_billing_for_year(service_id=service_id, year=year) data = serialize_ft_billing(results) - return jsonify(monthly_usage=data) + return jsonify(data) @billing_blueprint.route('/monthly-usage') @@ -49,13 +48,13 @@ def get_yearly_usage_by_month(service_id): year = int(request.args.get('year')) results = [] for month in get_months_for_financial_year(year): - billing_for_month = get_monthly_billing_by_notification_type(service_id, month, SMS_TYPE) - if billing_for_month: - results.append(_transform_billing_for_month_sms(billing_for_month)) letter_billing_for_month = get_monthly_billing_by_notification_type(service_id, month, LETTER_TYPE) if letter_billing_for_month: results.extend(_transform_billing_for_month_letters(letter_billing_for_month)) - return json.dumps(results) + billing_for_month = get_monthly_billing_by_notification_type(service_id, month, SMS_TYPE) + if billing_for_month: + results.append(_transform_billing_for_month_sms(billing_for_month)) + return jsonify(results) except TypeError: return jsonify(result='error', message='No valid year provided'), 400 @@ -209,16 +208,13 @@ def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, fin def serialize_ft_billing(data): results = [] - for d in data: + no_emails = [x for x in data if x.notification_type != 'email'] + for d in no_emails: j = { "month": (datetime.strftime(d.month, "%B")), - "service_id": str(d.service_id), - "notifications_type": d.notification_type, - "notifications_sent": int(d.notifications_sent), - "billable_units": int(d.billable_units), + "notification_type": d.notification_type, + "billing_units": int(d.billable_units), "rate": float(d.rate), - "rate_multiplier": int(d.rate_multiplier), - "international": d.international, } results.append(j) return results diff --git a/app/commands.py b/app/commands.py index d68df2cd1..1a804679a 100644 --- a/app/commands.py +++ b/app/commands.py @@ -7,16 +7,18 @@ from decimal import Decimal import click import flask from click_datetime import Datetime as click_dt -from flask import current_app +from flask import current_app, json from sqlalchemy.orm.exc import NoResultFound from sqlalchemy import func from notifications_utils.statsd_decorators import statsd from app import db, DATETIME_FORMAT, encryption, redis_store +from app.billing.rest import get_yearly_usage_by_month, get_yearly_usage_by_monthly_from_ft_billing from app.celery.scheduled_tasks import send_total_sent_notifications_to_performance_platform from app.celery.service_callback_tasks import send_delivery_status_to_service from app.celery.letters_pdf_tasks import create_letters_pdf from app.config import QueueNames +from app.dao.date_util import get_financial_year from app.dao.fact_billing_dao import fetch_billing_data_for_day, update_fact_billing from app.dao.monthly_billing_dao import ( create_or_update_monthly_billing, @@ -562,3 +564,39 @@ def rebuild_ft_billing_for_month_and_service(service_id, day): transit_data = fetch_billing_data_for_day(process_day=day, service_id=service_id) for data in transit_data: update_fact_billing(data, day) + + +@notify_command(name='compare-ft-billing-to-monthly-billing') +@click.option('-y', '--year', required=True) +@click.option('-s', '--service_id', required=False, type=click.UUID) +def compare_ft_billing_to_monthly_billing(year, service_id=None): + """ + This command checks the results of monthly_billing to ft_billing for the given year. + If service id is not included all services are compared for the given year. + """ + + def compare_monthly_billing_to_ft_billing(ft_billing_response, monthly_billing_response): + # Remove the rows with 0 billing_units and rate, ft_billing doesn't populate those rows. + mo_json = json.loads(monthly_billing_response.get_data(as_text=True)) + rm_zero_rows = [x for x in mo_json if x['billing_units'] != 0 and x['rate'] != 0] + assert rm_zero_rows == json.loads(ft_billing_response.get_data(as_text=True)) + + if not service_id: + start_date, end_date = get_financial_year(year=int(year)) + services = get_service_ids_that_need_billing_populated(start_date, end_date) + for service_id in services: + with current_app.test_request_context( + path='/service/{}/billing/monthly-usage?year={}'.format(service_id, year)): + monthly_billing_response = get_yearly_usage_by_month(service_id) + with current_app.test_request_context( + path='/service/{}/billing/ft-monthly-usage?year={}'.format(service_id, year)): + ft_billing_response = get_yearly_usage_by_monthly_from_ft_billing(service_id) + compare_monthly_billing_to_ft_billing(ft_billing_response, monthly_billing_response) + else: + with current_app.test_request_context( + path='/service/{}/billing/monthly-usage?year={}'.format(service_id, year)): + monthly_billing_response = get_yearly_usage_by_month(service_id) + with current_app.test_request_context( + path='/service/{}/billing/ft-monthly-usage?year={}'.format(service_id, year)): + ft_billing_response = get_yearly_usage_by_monthly_from_ft_billing(service_id) + compare_ft_billing_to_monthly_billing(ft_billing_response, monthly_billing_response) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 7cc1919c0..5fc8a7904 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -23,9 +23,9 @@ from app.utils import convert_utc_to_bst, convert_bst_to_utc def fetch_monthly_billing_for_year(service_id, year): year_start_date, year_end_date = get_financial_year(year) utcnow = datetime.utcnow() - today = convert_utc_to_bst(utcnow).date() + today = convert_utc_to_bst(utcnow) # if year end date is less than today, we are calculating for data in the past and have no need for deltas. - if year_end_date.date() >= today: + if year_end_date >= today: yesterday = today - timedelta(days=1) for day in [yesterday, today]: data = fetch_billing_data_for_day(process_day=day, service_id=service_id) @@ -35,11 +35,9 @@ def fetch_monthly_billing_for_year(service_id, year): yearly_data = db.session.query( func.date_trunc('month', FactBilling.bst_date).label("month"), func.sum(FactBilling.notifications_sent).label("notifications_sent"), - func.sum(FactBilling.billable_units).label("billable_units"), + func.sum(FactBilling.billable_units * FactBilling.rate_multiplier).label("billable_units"), FactBilling.service_id, FactBilling.rate, - FactBilling.rate_multiplier, - FactBilling.international, FactBilling.notification_type ).filter( FactBilling.service_id == service_id, @@ -49,8 +47,6 @@ def fetch_monthly_billing_for_year(service_id, year): 'month', FactBilling.service_id, FactBilling.rate, - FactBilling.rate_multiplier, - FactBilling.international, FactBilling.notification_type ).order_by( FactBilling.service_id, @@ -125,7 +121,7 @@ def update_fact_billing(data, process_day): inserted_records = 0 updated_records = 0 non_letter_rates, letter_rates = get_rates_for_billing() - + print("process_day: {} {}".format(type(process_day), process_day)) update_count = FactBilling.query.filter( FactBilling.bst_date == datetime.date(process_day), FactBilling.template_id == data.template_id, diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index dafbbb104..715cd164b 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -100,7 +100,8 @@ def get_yearly_billing_data_for_date_range( MonthlyBilling.end_date <= end_date, MonthlyBilling.notification_type.in_(notification_types) ).order_by( - MonthlyBilling.notification_type + MonthlyBilling.start_date, + MonthlyBilling.notification_type, ).all() return results diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index c0cae0a1a..94de414c8 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -8,8 +8,8 @@ from app.dao.monthly_billing_dao import ( create_or_update_monthly_billing, get_monthly_billing_by_notification_type, ) -from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE -from app.dao.date_util import get_current_financial_year_start_year +from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, FactBilling +from app.dao.date_util import get_current_financial_year_start_year, get_month_start_and_end_date_in_utc from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year from tests.app.db import ( create_notification, @@ -143,30 +143,29 @@ def test_get_yearly_usage_by_month_returns_correctly(client, sample_template): resp_json = json.loads(response.get_data(as_text=True)) _assert_dict_equals(resp_json[0], { + 'billing_units': 0, + 'month': 'May', + 'notification_type': LETTER_TYPE, + 'rate': 0 + }) + _assert_dict_equals(resp_json[1], { 'billing_units': 2, 'month': 'May', 'notification_type': SMS_TYPE, 'rate': 0.12 }) - _assert_dict_equals(resp_json[1], { + _assert_dict_equals(resp_json[2], { 'billing_units': 0, - 'month': 'May', + 'month': 'June', 'notification_type': LETTER_TYPE, 'rate': 0 }) - - _assert_dict_equals(resp_json[2], { + _assert_dict_equals(resp_json[3], { 'billing_units': 6, 'month': 'June', 'notification_type': SMS_TYPE, 'rate': 0.12 }) - _assert_dict_equals(resp_json[3], { - 'billing_units': 0, - 'month': 'June', - 'notification_type': LETTER_TYPE, - 'rate': 0 - }) def test_transform_billing_for_month_returns_empty_if_no_monthly_totals(sample_service): @@ -413,6 +412,25 @@ def test_update_free_sms_fragment_limit_data(client, sample_service): assert annual_billing.free_sms_fragment_limit == 9999 +def test_get_yearly_usage_by_monthly_from_ft_billing_populates_deltas(client, notify_db_session): + service = create_service() + sms_template = create_template(service=service, template_type="sms") + create_rate(start_date=datetime.utcnow() - timedelta(days=1), value=0.158, notification_type='sms') + + create_notification(template=sms_template, status='delivered') + + assert FactBilling.query.count() == 0 + + response = client.get('service/{}/billing/ft-monthly-usage?year=2018'.format(service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 200 + assert len(json.loads(response.get_data(as_text=True))) == 1 + fact_billing = FactBilling.query.all() + assert len(fact_billing) == 1 + assert fact_billing[0].notification_type == 'sms' + + def test_get_yearly_usage_by_monthly_from_ft_billing(client, notify_db_session): service = create_service() sms_template = create_template(service=service, template_type="sms") @@ -427,29 +445,113 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(client, notify_db_session): service=service, template=sms_template, notification_type='sms', + billable_unit=1, rate=0.162) create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), service=service, template=email_template, notification_type='email', rate=0) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=letter_template, + notification_type='letter', + billable_unit=1, + rate=0.33) + + response = client.get('service/{}/billing/ft-monthly-usage?year=2016'.format(service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + json_resp = json.loads(response.get_data(as_text=True)) + ft_letters = [x for x in json_resp if x['notification_type'] == 'letter'] + ft_sms = [x for x in json_resp if x['notification_type'] == 'sms'] + ft_email = [x for x in json_resp if x['notification_type'] == 'email'] + keys = [x.keys() for x in ft_sms][0] + expected_sms_april = {"month": "April", + "notification_type": "sms", + "billing_units": 30, + "rate": 0.162 + } + expected_letter_april = {"month": "April", + "notification_type": "letter", + "billing_units": 30, + "rate": 0.33 + } + + for k in keys: + assert ft_sms[0][k] == expected_sms_april[k] + assert ft_letters[0][k] == expected_letter_april[k] + assert len(ft_email) == 0 + + +def test_compare_ft_billing_to_monthly_billing(client, notify_db_session): + service = create_service() + sms_template = create_template(service=service, template_type="sms") + email_template = create_template(service=service, template_type="email") + letter_template = create_template(service=service, template_type="letter") + for month in range(4, 9): + mon = str(month).zfill(2) + days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} + for day in range(1, days_in_month[month]): + d = str(day).zfill(2) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=sms_template, + notification_type='sms', + rate=0.0162) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=sms_template, + notification_type='sms', + rate_multiplier=2, + rate=0.0162) + create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), + service=service, + template=email_template, + notification_type='email', + rate=0) create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), service=service, template=letter_template, notification_type='letter', rate=0.33) - response = client.get('service/{}/billing/ft-monthly-usage?year=2016'.format(service.id), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) + start_date, end_date = get_month_start_and_end_date_in_utc(datetime(2016, int(mon), 1)) + create_monthly_billing_entry(service=service, start_date=start_date, + end_date=end_date, + notification_type='sms', + monthly_totals=[ + {"rate": 0.0162, "international": False, + "rate_multiplier": 1, "billing_units": int(d), + "total_cost": 0.0162 * int(d)}, + {"rate": 0.0162, "international": False, + "rate_multiplier": 2, "billing_units": int(d), + "total_cost": 0.0162 * int(d)}] + ) + create_monthly_billing_entry(service=service, start_date=start_date, + end_date=end_date, + notification_type='email', + monthly_totals=[ + {"rate": 0, "international": False, + "rate_multiplier": 1, "billing_units": int(d), + "total_cost": 0}] + ) + create_monthly_billing_entry(service=service, start_date=start_date, + end_date=end_date, + notification_type='letter', + monthly_totals=[ + {"rate": 0.33, "international": False, + "rate_multiplier": 1, "billing_units": int(d), + "total_cost": 0.33 * int(d)}] + ) - json_resp = json.loads(response.get_data(as_text=True)) + monthly_billing_response = client.get('/service/{}/billing/monthly-usage?year=2016'.format(service.id), + headers=[create_authorization_header()]) - assert json_resp["monthly_usage"][0] == {"month": "April", - "service_id": str(service.id), - "notifications_type": 'email', - "notifications_sent": 30, - "billable_units": 30, - "rate": 0.0, - "rate_multiplier": 1, - "international": False, - } + ft_billing_response = client.get('service/{}/billing/ft-monthly-usage?year=2016'.format(service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + monthly_billing_json_resp = json.loads(monthly_billing_response.get_data(as_text=True)) + ft_billing_json_resp = json.loads(ft_billing_response.get_data(as_text=True)) + + assert monthly_billing_json_resp == ft_billing_json_resp diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 5230e72ec..eac9a13ac 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -208,6 +208,7 @@ def test_fetch_monthly_billing_for_year(notify_db_session): service=service, template=template, notification_type='sms', + rate_multiplier=2, rate=0.162) for i in range(1, 32): create_ft_billing(bst_date='2018-07-{}'.format(i), @@ -221,11 +222,9 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert len(results) == 2 assert str(results[0].month) == "2018-06-01 00:00:00+01:00" assert results[0].notifications_sent == 30 - assert results[0].billable_units == Decimal('30') + assert results[0].billable_units == Decimal('60') assert results[0].service_id == service.id assert results[0].rate == Decimal('0.162') - assert results[0].rate_multiplier == Decimal('1') - assert results[0].international is False assert results[0].notification_type == 'sms' assert str(results[1].month) == "2018-07-01 00:00:00+01:00" @@ -233,8 +232,6 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert results[1].billable_units == Decimal('31') assert results[1].service_id == service.id assert results[1].rate == Decimal('0.158') - assert results[1].rate_multiplier == Decimal('1') - assert results[1].international is False assert results[1].notification_type == 'sms' From fd6e5f39cfbb76bc6c8da1be2171ed2e10c26766 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 8 May 2018 12:09:29 +0100 Subject: [PATCH 07/41] Changes as per requested from code review Move the serialize method to the billing_schema Update variable names Improve tests Fix bug in command --- app/billing/billing_schemas.py | 16 ++++++++ app/billing/rest.py | 41 ++++++++------------ app/commands.py | 2 +- tests/app/billing/test_billing.py | 10 ++--- tests/app/dao/test_ft_billing_dao.py | 57 +++++++++++++++++----------- 5 files changed, 71 insertions(+), 55 deletions(-) diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index 00d92f1c3..572228439 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -1,3 +1,5 @@ +from datetime import datetime + create_or_update_free_sms_fragment_limit_schema = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "POST annual billing schema", @@ -8,3 +10,17 @@ create_or_update_free_sms_fragment_limit_schema = { }, "required": ["free_sms_fragment_limit"] } + + +def serialize_ft_billing_remove_emails(data): + results = [] + billed_notifications = [x for x in data if x.notification_type != 'email'] + for notifications in billed_notifications: + json_result = { + "month": (datetime.strftime(notifications.month, "%B")), + "notification_type": notifications.notification_type, + "billing_units": int(notifications.billable_units), + "rate": float(notifications.rate), + } + results.append(json_result) + return results diff --git a/app/billing/rest.py b/app/billing/rest.py index cb6eb4cc6..77203c80c 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -1,25 +1,30 @@ -from datetime import datetime import json +from datetime import datetime from flask import Blueprint, jsonify, request +from app.billing.billing_schemas import ( + create_or_update_free_sms_fragment_limit_schema, + serialize_ft_billing_remove_emails +) +from app.dao.annual_billing_dao import ( + dao_get_free_sms_fragment_limit_for_year, + dao_get_all_free_sms_fragment_limit, + dao_create_or_update_annual_billing_for_year, + dao_update_annual_billing_for_future_years +) +from app.dao.date_util import get_current_financial_year_start_year +from app.dao.date_util import get_months_for_financial_year from app.dao.fact_billing_dao import fetch_monthly_billing_for_year from app.dao.monthly_billing_dao import ( get_billing_data_for_financial_year, get_monthly_billing_by_notification_type ) -from app.dao.date_util import get_months_for_financial_year +from app.errors import InvalidRequest from app.errors import register_errors from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE -from app.utils import convert_utc_to_bst -from app.dao.annual_billing_dao import (dao_get_free_sms_fragment_limit_for_year, - dao_get_all_free_sms_fragment_limit, - dao_create_or_update_annual_billing_for_year, - dao_update_annual_billing_for_future_years) -from app.billing.billing_schemas import create_or_update_free_sms_fragment_limit_schema -from app.errors import InvalidRequest from app.schema_validation import validate -from app.dao.date_util import get_current_financial_year_start_year +from app.utils import convert_utc_to_bst billing_blueprint = Blueprint( 'billing', @@ -38,7 +43,7 @@ def get_yearly_usage_by_monthly_from_ft_billing(service_id): except TypeError: return jsonify(result='error', message='No valid year provided'), 400 results = fetch_monthly_billing_for_year(service_id=service_id, year=year) - data = serialize_ft_billing(results) + data = serialize_ft_billing_remove_emails(results) return jsonify(data) @@ -204,17 +209,3 @@ def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, fin free_sms_fragment_limit, financial_year_start ) - - -def serialize_ft_billing(data): - results = [] - no_emails = [x for x in data if x.notification_type != 'email'] - for d in no_emails: - j = { - "month": (datetime.strftime(d.month, "%B")), - "notification_type": d.notification_type, - "billing_units": int(d.billable_units), - "rate": float(d.rate), - } - results.append(j) - return results diff --git a/app/commands.py b/app/commands.py index 1a804679a..45cb2cabd 100644 --- a/app/commands.py +++ b/app/commands.py @@ -599,4 +599,4 @@ def compare_ft_billing_to_monthly_billing(year, service_id=None): with current_app.test_request_context( path='/service/{}/billing/ft-monthly-usage?year={}'.format(service_id, year)): ft_billing_response = get_yearly_usage_by_monthly_from_ft_billing(service_id) - compare_ft_billing_to_monthly_billing(ft_billing_response, monthly_billing_response) + compare_monthly_billing_to_ft_billing(ft_billing_response, monthly_billing_response) diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 94de414c8..06a7c7a06 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -1,3 +1,4 @@ +from calendar import monthrange from datetime import datetime, timedelta import json @@ -438,8 +439,7 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(client, notify_db_session): letter_template = create_template(service=service, template_type="letter") for month in range(1, 13): mon = str(month).zfill(2) - days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} - for day in range(1, days_in_month[month]): + for day in range(1, monthrange(2016, month)[1] + 1): d = str(day).zfill(2) create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), service=service, @@ -489,10 +489,9 @@ def test_compare_ft_billing_to_monthly_billing(client, notify_db_session): sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") letter_template = create_template(service=service, template_type="letter") - for month in range(4, 9): + for month in range(1, 13): mon = str(month).zfill(2) - days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} - for day in range(1, days_in_month[month]): + for day in range(1, monthrange(2016, month)[1] + 1): d = str(day).zfill(2) create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), service=service, @@ -515,7 +514,6 @@ def test_compare_ft_billing_to_monthly_billing(client, notify_db_session): template=letter_template, notification_type='letter', rate=0.33) - start_date, end_date = get_month_start_and_end_date_in_utc(datetime(2016, int(mon), 1)) create_monthly_billing_entry(service=service, start_date=start_date, end_date=end_date, diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index eac9a13ac..45ad4cc0a 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -1,3 +1,4 @@ +from calendar import monthrange from decimal import Decimal from datetime import datetime, timedelta @@ -259,37 +260,47 @@ def test_fetch_monthly_billing_for_year_return_financial_year(notify_db_session) sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") letter_template = create_template(service=service, template_type="letter") - for month in range(1, 13): - mon = str(month).zfill(2) - days_in_month = {1: 32, 2: 30, 3: 32, 4: 31, 5: 32, 6: 31, 7: 32, 8: 32, 9: 31, 10: 32, 11: 31, 12: 32} - for day in range(1, days_in_month[month]): - d = str(day).zfill(2) - create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), - service=service, - template=sms_template, - notification_type='sms', - rate=0.162) - create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), - service=service, - template=email_template, - notification_type='email', - rate=0) - create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), - service=service, - template=letter_template, - notification_type='letter', - rate=0.33) + + for year in (2016, 2017): + for month in range(1, 13): + mon = str(month).zfill(2) + for day in range(1, monthrange(year, month)[1] + 1): + d = str(day).zfill(2) + create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), + service=service, + template=sms_template, + notification_type='sms', + rate=0.162) + create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), + service=service, + template=email_template, + notification_type='email', + rate=0) + create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), + service=service, + template=letter_template, + notification_type='letter', + rate=0.33) results = fetch_monthly_billing_for_year(service.id, 2016) - # returns 3 rows, per month, returns financial year april to december + # returns 3 rows, per month, returns financial year april to end of march # Orders by Month - assert len(results) == 27 + assert len(results) == 36 assert str(results[0].month) == "2016-04-01 00:00:00+01:00" assert results[0].notification_type == 'email' + assert results[0].notifications_sent == 30 + assert results[0].billable_units == 30 + assert results[0].rate == Decimal('0') assert str(results[1].month) == "2016-04-01 00:00:00+01:00" assert results[1].notification_type == 'letter' + assert results[1].notifications_sent == 30 + assert results[1].billable_units == 30 + assert results[1].rate == Decimal('0.33') assert str(results[2].month) == "2016-04-01 00:00:00+01:00" assert results[2].notification_type == 'sms' + assert results[2].notifications_sent == 30 + assert results[2].billable_units == 30 + assert results[2].rate == Decimal('0.162') assert str(results[3].month) == "2016-05-01 00:00:00+01:00" - assert str(results[26].month) == "2016-12-01 00:00:00+00:00" + assert str(results[35].month) == "2017-03-01 00:00:00+00:00" From dd113a8e86e4724811197dda573ceffce36cf3bc Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 8 May 2018 13:44:06 +0100 Subject: [PATCH 08/41] Update the tests so that they ignore timezone in the month returned by query. The timezone added is that of the database - locally a db will get timezone=GB, but the servers are using UTC (which is right) This date is trucated to month, and is BST, here time and timezone is irrelevant. --- tests/app/dao/test_ft_billing_dao.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 45ad4cc0a..2f7eef368 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -221,14 +221,14 @@ def test_fetch_monthly_billing_for_year(notify_db_session): results = fetch_monthly_billing_for_year(service_id=service.id, year=2018) assert len(results) == 2 - assert str(results[0].month) == "2018-06-01 00:00:00+01:00" + assert str(results[0].month.date()) == "2018-06-01" assert results[0].notifications_sent == 30 assert results[0].billable_units == Decimal('60') assert results[0].service_id == service.id assert results[0].rate == Decimal('0.162') assert results[0].notification_type == 'sms' - assert str(results[1].month) == "2018-07-01 00:00:00+01:00" + assert str(results[1].month.date()) == "2018-07-01" assert results[1].notifications_sent == 31 assert results[1].billable_units == Decimal('31') assert results[1].service_id == service.id @@ -287,20 +287,20 @@ def test_fetch_monthly_billing_for_year_return_financial_year(notify_db_session) # Orders by Month assert len(results) == 36 - assert str(results[0].month) == "2016-04-01 00:00:00+01:00" + assert str(results[0].month.date()) == "2016-04-01" assert results[0].notification_type == 'email' assert results[0].notifications_sent == 30 assert results[0].billable_units == 30 assert results[0].rate == Decimal('0') - assert str(results[1].month) == "2016-04-01 00:00:00+01:00" + assert str(results[1].month.date()) == "2016-04-01" assert results[1].notification_type == 'letter' assert results[1].notifications_sent == 30 assert results[1].billable_units == 30 assert results[1].rate == Decimal('0.33') - assert str(results[2].month) == "2016-04-01 00:00:00+01:00" + assert str(results[2].month.date()) == "2016-04-01" assert results[2].notification_type == 'sms' assert results[2].notifications_sent == 30 assert results[2].billable_units == 30 assert results[2].rate == Decimal('0.162') - assert str(results[3].month) == "2016-05-01 00:00:00+01:00" - assert str(results[35].month) == "2017-03-01 00:00:00+00:00" + assert str(results[3].month.date()) == "2016-05-01" + assert str(results[35].month.date()) == "2017-03-01" From 3e3b885bdc82eb084828e575b04514c1637b17b1 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 8 May 2018 13:53:44 +0100 Subject: [PATCH 09/41] Realised that it's best to cast the Month as date. --- app/dao/fact_billing_dao.py | 4 ++-- tests/app/dao/test_ft_billing_dao.py | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 5fc8a7904..f69df16f3 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -1,7 +1,7 @@ from datetime import datetime, timedelta, time from flask import current_app -from sqlalchemy import func, case, desc +from sqlalchemy import func, case, desc, Date from app import db from app.dao.date_util import get_financial_year @@ -33,7 +33,7 @@ def fetch_monthly_billing_for_year(service_id, year): update_fact_billing(data=d, process_day=day) yearly_data = db.session.query( - func.date_trunc('month', FactBilling.bst_date).label("month"), + func.date_trunc('month', FactBilling.bst_date).cast(Date).label("month"), func.sum(FactBilling.notifications_sent).label("notifications_sent"), func.sum(FactBilling.billable_units * FactBilling.rate_multiplier).label("billable_units"), FactBilling.service_id, diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 2f7eef368..216caa447 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -221,14 +221,14 @@ def test_fetch_monthly_billing_for_year(notify_db_session): results = fetch_monthly_billing_for_year(service_id=service.id, year=2018) assert len(results) == 2 - assert str(results[0].month.date()) == "2018-06-01" + assert str(results[0].month) == "2018-06-01" assert results[0].notifications_sent == 30 assert results[0].billable_units == Decimal('60') assert results[0].service_id == service.id assert results[0].rate == Decimal('0.162') assert results[0].notification_type == 'sms' - assert str(results[1].month.date()) == "2018-07-01" + assert str(results[1].month) == "2018-07-01" assert results[1].notifications_sent == 31 assert results[1].billable_units == Decimal('31') assert results[1].service_id == service.id @@ -287,20 +287,20 @@ def test_fetch_monthly_billing_for_year_return_financial_year(notify_db_session) # Orders by Month assert len(results) == 36 - assert str(results[0].month.date()) == "2016-04-01" + assert str(results[0].month) == "2016-04-01" assert results[0].notification_type == 'email' assert results[0].notifications_sent == 30 assert results[0].billable_units == 30 assert results[0].rate == Decimal('0') - assert str(results[1].month.date()) == "2016-04-01" + assert str(results[1].month) == "2016-04-01" assert results[1].notification_type == 'letter' assert results[1].notifications_sent == 30 assert results[1].billable_units == 30 assert results[1].rate == Decimal('0.33') - assert str(results[2].month.date()) == "2016-04-01" + assert str(results[2].month) == "2016-04-01" assert results[2].notification_type == 'sms' assert results[2].notifications_sent == 30 assert results[2].billable_units == 30 assert results[2].rate == Decimal('0.162') - assert str(results[3].month.date()) == "2016-05-01" - assert str(results[35].month.date()) == "2017-03-01" + assert str(results[3].month) == "2016-05-01" + assert str(results[35].month) == "2017-03-01" From 844f5b2e5d35c2e6273c765e9934eb8c3bbe923e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 8 May 2018 16:32:28 +0100 Subject: [PATCH 10/41] Added logging message if the comparison failed. --- app/commands.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/commands.py b/app/commands.py index 45cb2cabd..6c4d1f659 100644 --- a/app/commands.py +++ b/app/commands.py @@ -579,7 +579,10 @@ def compare_ft_billing_to_monthly_billing(year, service_id=None): # Remove the rows with 0 billing_units and rate, ft_billing doesn't populate those rows. mo_json = json.loads(monthly_billing_response.get_data(as_text=True)) rm_zero_rows = [x for x in mo_json if x['billing_units'] != 0 and x['rate'] != 0] - assert rm_zero_rows == json.loads(ft_billing_response.get_data(as_text=True)) + try: + assert rm_zero_rows == json.loads(ft_billing_response.get_data(as_text=True)) + except AssertionError: + print("Comparison failed for service: {} and year: {}".format(service_id, year)) if not service_id: start_date, end_date = get_financial_year(year=int(year)) From c30f13ea6748d5020c697f20f71c64a56ebb8da6 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 9 May 2018 11:36:42 +0100 Subject: [PATCH 11/41] create new stats endpoint the admin app currently calls get_detailed_service, which gets notification stats, adds them on to the rest of the detailed service endpoint, and returns them. However, the admin app then only looks at the stats - it doesn't look at the rest of the service object. This is called in a few high profile places - the dashboard, the notification summary page, and when you send a job. By creating a separate endpoint that ignores the rest of the service object (and no marshmallow too!), the hope is that we'll improve some slowness we've been seeing. Note: The old detailed function will still need to stay - it's used by get_services(detailed=True) for the platform admin page. --- app/service/rest.py | 17 +++++++++++++---- tests/app/service/test_rest.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 8a32baebc..136ddc99e 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -170,6 +170,11 @@ def get_service_by_id(service_id): return jsonify(data=data) +@service_blueprint.route('//statistics') +def get_service_notification_statistics(service_id): + return jsonify(data=get_service_statistics(service_id, request.args.get('today_only') == 'True')) + + @service_blueprint.route('', methods=['POST']) def create_service(): data = request.get_json() @@ -411,12 +416,16 @@ def get_monthly_notification_stats(service_id): def get_detailed_service(service_id, today_only=False): service = dao_fetch_service_by_id(service_id) + + service.statistics = get_service_statistics(service_id, today_only) + return detailed_service_schema.dump(service).data + + +def get_service_statistics(service_id, today_only): + # today_only flag is used by the send page to work out if the service will exceed their daily usage by sending a job stats_fn = dao_fetch_todays_stats_for_service if today_only else dao_fetch_stats_for_service stats = stats_fn(service_id) - - service.statistics = statistics.format_statistics(stats) - - return detailed_service_schema.dump(service).data + return statistics.format_statistics(stats) def get_detailed_services(start_date, end_date, only_active=False, include_from_test_key=True): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 9dd47f2e0..1f19922ad 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -3085,3 +3085,35 @@ def test_get_platform_stats_creates_zero_stats(client, notify_db_session): assert json_resp['email'] == {'failed': 1, 'requested': 2, 'delivered': 1} assert json_resp['letter'] == {'failed': 0, 'requested': 0, 'delivered': 0} assert json_resp['sms'] == {'failed': 0, 'requested': 4, 'delivered': 3} + + +@pytest.mark.parametrize('today_only, stats', [ + (False, {'requested': 2, 'delivered': 1, 'failed': 0}), + (True, {'requested': 1, 'delivered': 0, 'failed': 0}) +], ids=['seven_days', 'today']) +def test_get_service_notification_statistics(admin_request, sample_template, today_only, stats): + with freeze_time('2000-01-01T12:00:00'): + create_notification(sample_template, status='delivered') + with freeze_time('2000-01-02T12:00:00'): + create_notification(sample_template, status='created') + resp = admin_request.get( + 'service.get_service_notification_statistics', + service_id=sample_template.service_id, + today_only=today_only + ) + + assert set(resp['data'].keys()) == {SMS_TYPE, EMAIL_TYPE, LETTER_TYPE} + assert resp['data'][SMS_TYPE] == stats + + +def test_get_service_notification_statistics_with_unknown_service(admin_request): + resp = admin_request.get( + 'service.get_service_notification_statistics', + service_id=uuid.uuid4() + ) + + assert resp['data'] == { + SMS_TYPE: {'requested': 0, 'delivered': 0, 'failed': 0}, + EMAIL_TYPE: {'requested': 0, 'delivered': 0, 'failed': 0}, + LETTER_TYPE: {'requested': 0, 'delivered': 0, 'failed': 0}, + } From 2abbb590abe89a44286ffe8ac6df617e2be40f65 Mon Sep 17 00:00:00 2001 From: chrisw Date: Mon, 30 Apr 2018 16:58:26 +0100 Subject: [PATCH 12/41] Only get inbound messages that are a maximum of 7 days old --- app/dao/inbound_sms_dao.py | 18 ++++-- tests/app/dao/test_inbound_sms_dao.py | 79 ++++++++++++++++++++++----- tests/app/inbound_sms/test_rest.py | 21 ++++--- 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index cbe0ac0db..22b0d468b 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -1,6 +1,7 @@ from datetime import ( timedelta, - datetime + datetime, + date ) from flask import current_app from notifications_utils.statsd_decorators import statsd @@ -10,6 +11,7 @@ from sqlalchemy.orm import aliased from app import db from app.dao.dao_utils import transactional from app.models import InboundSms +from app.utils import get_london_midnight_in_utc @transactional @@ -18,8 +20,10 @@ def dao_create_inbound_sms(inbound_sms): def dao_get_inbound_sms_for_service(service_id, limit=None, user_number=None): + start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) q = InboundSms.query.filter( - InboundSms.service_id == service_id + InboundSms.service_id == service_id, + InboundSms.created_at >= start_date ).order_by( InboundSms.created_at.desc() ) @@ -56,8 +60,10 @@ def dao_get_paginated_inbound_sms_for_service_for_public_api( def dao_count_inbound_sms_for_service(service_id): + start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) return InboundSms.query.filter( - InboundSms.service_id == service_id + InboundSms.service_id == service_id, + InboundSms.created_at >= start_date ).count() @@ -102,6 +108,7 @@ def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service( ORDER BY t1.created_at DESC; LIMIT 50 OFFSET :page """ + start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) t2 = aliased(InboundSms) q = db.session.query( InboundSms @@ -110,11 +117,12 @@ def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service( and_( InboundSms.user_number == t2.user_number, InboundSms.service_id == t2.service_id, - InboundSms.created_at < t2.created_at + InboundSms.created_at < t2.created_at, ) ).filter( t2.id == None, # noqa - InboundSms.service_id == service_id + InboundSms.service_id == service_id, + InboundSms.created_at >= start_date ).order_by( InboundSms.created_at.desc() ) diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index fc899af26..6fd43a47b 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -37,7 +37,8 @@ def test_get_all_inbound_sms_limits_and_orders(sample_service): with freeze_time('2017-01-02'): two = create_inbound_sms(sample_service) - res = dao_get_inbound_sms_for_service(sample_service.id, limit=2) + res = dao_get_inbound_sms_for_service(sample_service.id, limit=2) + assert len(res) == 2 assert res[0] == three assert res[0].created_at == datetime(2017, 1, 3) @@ -57,6 +58,22 @@ def test_get_all_inbound_sms_filters_on_service(notify_db_session): assert res[0] == sms_one +def test_get_all_inbound_sms_filters_on_time(sample_service, notify_db_session): + create_inbound_sms(sample_service, user_number='447700900111', content='111 1', created_at=datetime(2017, 1, 2)) + sms_two = create_inbound_sms( + sample_service, + user_number='447700900111', + content='111 2', + created_at=datetime(2017, 1, 3) + ) + + with freeze_time('2017-01-09'): + res = dao_get_inbound_sms_for_service(sample_service.id) + + assert len(res) == 1 + assert res[0] == sms_two + + def test_count_inbound_sms_for_service(notify_db_session): service_one = create_service(service_name='one') service_two = create_service(service_name='two') @@ -68,6 +85,14 @@ def test_count_inbound_sms_for_service(notify_db_session): assert dao_count_inbound_sms_for_service(service_one.id) == 2 +def test_count_inbound_sms_for_service_filters_messages_older_than_seven_days(sample_service, notify_db_session): + create_inbound_sms(sample_service, user_number='447700900111', content='111 2', created_at=datetime(2017, 1, 2)) + create_inbound_sms(sample_service, user_number='447700900111', content='111 2', created_at=datetime(2017, 1, 3)) + + with freeze_time('2017-01-09'): + assert dao_count_inbound_sms_for_service(sample_service.id) == 1 + + @freeze_time("2017-01-01 12:00:00") def test_should_delete_inbound_sms_older_than_seven_days(sample_service): older_than_seven_days = datetime.utcnow() - timedelta(days=7, seconds=1) @@ -187,7 +212,8 @@ def test_most_recent_inbound_sms_only_returns_most_recent_for_each_number(notify create_inbound_sms(sample_service, user_number='447700900222', content='222 2', created_at=datetime(2017, 1, 2)) with set_config(notify_api, 'PAGE_SIZE', 3): - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) + with freeze_time('2017-01-02'): + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) assert len(res.items) == 2 assert res.has_next is False @@ -207,17 +233,40 @@ def test_most_recent_inbound_sms_paginates_properly(notify_api, sample_service): create_inbound_sms(sample_service, user_number='447700900444', content='444 2', created_at=datetime(2017, 1, 8)) with set_config(notify_api, 'PAGE_SIZE', 2): - # first page has most recent 444 and 333 - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) - assert len(res.items) == 2 - assert res.has_next is True - assert res.per_page == 2 - assert res.items[0].content == '444 2' - assert res.items[1].content == '333 2' + with freeze_time('2017-01-02'): + # first page has most recent 444 and 333 + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) + assert len(res.items) == 2 + assert res.has_next is True + assert res.per_page == 2 + assert res.items[0].content == '444 2' + assert res.items[1].content == '333 2' - # second page has no 444 or 333 - just most recent 222 and 111 - res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=2) - assert len(res.items) == 2 - assert res.has_next is False - assert res.items[0].content == '222 2' - assert res.items[1].content == '111 2' + # second page has no 444 or 333 - just most recent 222 and 111 + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=2) + assert len(res.items) == 2 + assert res.has_next is False + assert res.items[0].content == '222 2' + assert res.items[1].content == '111 2' + + +def test_most_recent_inbound_sms_only_returns_values_within_7_days(notify_api, sample_service): + create_inbound_sms(sample_service, user_number='447700900111', content='111 1', created_at=datetime(2017, 4, 1)) + create_inbound_sms(sample_service, user_number='447700900111', content='111 2', created_at=datetime(2017, 4, 1)) + create_inbound_sms(sample_service, user_number='447700900222', content='222 1', created_at=datetime(2017, 4, 1)) + create_inbound_sms(sample_service, user_number='447700900222', content='222 2', created_at=datetime(2017, 4, 1)) + create_inbound_sms(sample_service, user_number='447700900333', content='333 1', created_at=datetime(2017, 4, 2)) + create_inbound_sms(sample_service, user_number='447700900333', content='333 2', created_at=datetime(2017, 4, 3)) + create_inbound_sms(sample_service, user_number='447700900444', content='444 1', created_at=datetime(2017, 4, 4)) + create_inbound_sms(sample_service, user_number='447700900444', content='444 2', created_at=datetime(2017, 4, 5)) + + # 7 days ago BST midnight + create_inbound_sms(sample_service, user_number='447700900666', content='666 1', created_at='2017-04-02T23:00:00') + + with freeze_time('2017-04-09T12:00:00'): + res = dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service(sample_service.id, page=1) + + assert len(res.items) == 3 + assert res.items[0].content == '444 2' + assert res.items[1].content == '333 2' + assert res.items[2].content == '666 1' diff --git a/tests/app/inbound_sms/test_rest.py b/tests/app/inbound_sms/test_rest.py index 1415e7fa2..eb04d397a 100644 --- a/tests/app/inbound_sms/test_rest.py +++ b/tests/app/inbound_sms/test_rest.py @@ -37,13 +37,12 @@ def test_post_to_get_inbound_sms_with_limit(admin_request, sample_service): with freeze_time('2017-01-02'): two = create_inbound_sms(sample_service) - data = {'limit': 1} - - sms = admin_request.post( - 'inbound_sms.post_query_inbound_sms_for_service', - service_id=sample_service.id, - _data=data - )['data'] + data = {'limit': 1} + sms = admin_request.post( + 'inbound_sms.post_query_inbound_sms_for_service', + service_id=sample_service.id, + _data=data + )['data'] assert len(sms) == 1 assert sms[0]['id'] == str(two.id) @@ -220,10 +219,10 @@ def test_get_inbound_sms_summary(admin_request, sample_service): with freeze_time('2017-01-03'): create_inbound_sms(other_service) - summary = admin_request.get( - 'inbound_sms.get_inbound_sms_summary_for_service', - service_id=sample_service.id - ) + summary = admin_request.get( + 'inbound_sms.get_inbound_sms_summary_for_service', + service_id=sample_service.id + ) assert summary == { 'count': 2, From 8028f6cc285dc448f25ee5e71595957bc5a129f0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 10 May 2018 15:35:58 +0100 Subject: [PATCH 13/41] We found that the reporting task failed twice because of integrity constraints. This was because the rate_multiplier was being added as 1 and 1.0 which was not resolving to the same. This updates the table to use Integrer. Also changed the logging for the task. --- app/celery/reporting_tasks.py | 3 +++ app/dao/fact_billing_dao.py | 5 +---- app/models.py | 4 ++-- tests/app/dao/test_ft_billing_dao.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index f9c336638..607fad160 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -1,5 +1,6 @@ from datetime import datetime, timedelta +from flask import current_app from notifications_utils.statsd_decorators import statsd from app import notify_celery @@ -24,3 +25,5 @@ def create_nightly_billing(day_start=None): for data in transit_data: update_fact_billing(data, process_day) + + current_app.logger.info("create-nightly-billing task complete. {} rows updated".format(len(transit_data))) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index f69df16f3..fc8ebd044 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -1,6 +1,5 @@ from datetime import datetime, timedelta, time -from flask import current_app from sqlalchemy import func, case, desc, Date from app import db @@ -97,6 +96,7 @@ def fetch_billing_data_for_day(process_day, service_id=None): ) if service_id: transit_data = transit_data.filter(Notification.service_id == service_id) + return transit_data.all() @@ -121,7 +121,6 @@ def update_fact_billing(data, process_day): inserted_records = 0 updated_records = 0 non_letter_rates, letter_rates = get_rates_for_billing() - print("process_day: {} {}".format(type(process_day), process_day)) update_count = FactBilling.query.filter( FactBilling.bst_date == datetime.date(process_day), FactBilling.template_id == data.template_id, @@ -147,8 +146,6 @@ def update_fact_billing(data, process_day): inserted_records += 1 updated_records += update_count db.session.commit() - current_app.logger.info('ft_billing for {}: {} rows updated, {} rows inserted' - .format(process_day, updated_records, inserted_records)) def create_billing_record(data, rate, process_day): diff --git a/app/models.py b/app/models.py index 547c0a0d9..9d7591e28 100644 --- a/app/models.py +++ b/app/models.py @@ -1784,10 +1784,10 @@ class FactBilling(db.Model): service_id = db.Column(UUID(as_uuid=True), nullable=False, index=True) notification_type = db.Column(db.Text, nullable=False, primary_key=True) provider = db.Column(db.Text, nullable=True, primary_key=True) - rate_multiplier = db.Column(db.Numeric(), nullable=True, primary_key=True) + rate_multiplier = db.Column(db.Integer(), nullable=True, primary_key=True) international = db.Column(db.Boolean, nullable=False, primary_key=False) rate = db.Column(db.Numeric(), nullable=True) - billable_units = db.Column(db.Numeric(), nullable=True) + billable_units = db.Column(db.Integer(), nullable=True) notifications_sent = db.Column(db.Integer(), nullable=True) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 216caa447..f6df9e4bd 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -7,7 +7,7 @@ from freezegun import freeze_time from app import db from app.dao.fact_billing_dao import ( fetch_monthly_billing_for_year, fetch_billing_data_for_day, get_rates_for_billing, - get_rate + get_rate, ) from app.models import FactBilling from app.utils import convert_utc_to_bst From d50a0d4cb4690be82a6992e273b05160bccb301f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 10 May 2018 16:41:24 +0100 Subject: [PATCH 14/41] Migration script Change to command. --- app/commands.py | 23 ++++++++----- .../versions/0189_ft_billing_data_type.py | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 migrations/versions/0189_ft_billing_data_type.py diff --git a/app/commands.py b/app/commands.py index 6c4d1f659..57365c1a0 100644 --- a/app/commands.py +++ b/app/commands.py @@ -551,19 +551,26 @@ def populate_redis_template_usage(service_id, day): ) -@notify_command(name='rebuild-ft-billing-for-month-and-service') -@click.option('-s', '--service_id', required=True, type=click.UUID) +@notify_command(name='rebuild-ft-billing-for-day') +@click.option('-s', '--service_id', required=False, type=click.UUID) @click.option('-d', '--day', help="The date to recalculate, as YYYY-MM-DD", required=True, type=click_dt(format='%Y-%m-%d')) -def rebuild_ft_billing_for_month_and_service(service_id, day): +def rebuild_ft_billing_for_day(service_id, day): """ Rebuild the data in ft_billing for the given service_id and date """ - # confirm the service exists - dao_fetch_service_by_id(service_id) - transit_data = fetch_billing_data_for_day(process_day=day, service_id=service_id) - for data in transit_data: - update_fact_billing(data, day) + def rebuild_ft_data(process_day, service): + transit_data = fetch_billing_data_for_day(process_day=process_day, service_id=service) + for data in transit_data: + update_fact_billing(data, process_day) + if service_id: + # confirm the service exists + dao_fetch_service_by_id(service_id) + rebuild_ft_data(day, service_id) + else: + services = get_service_ids_that_need_billing_populated(day, day) + for service_id in services: + rebuild_ft_data(day, service_id) @notify_command(name='compare-ft-billing-to-monthly-billing') diff --git a/migrations/versions/0189_ft_billing_data_type.py b/migrations/versions/0189_ft_billing_data_type.py new file mode 100644 index 000000000..951583ad6 --- /dev/null +++ b/migrations/versions/0189_ft_billing_data_type.py @@ -0,0 +1,32 @@ +""" + +Revision ID: 0189_ft_billing_data_type +Revises: 0187_another_letter_org +Create Date: 2018-05-10 14:57:52.589773 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '0189_ft_billing_data_type' +down_revision = '0187_another_letter_org' + + +def upgrade(): + op.alter_column('ft_billing', 'billable_units', + existing_type=sa.NUMERIC(), + type_=sa.Integer(), + existing_nullable=True) + op.alter_column('ft_billing', 'rate_multiplier', + existing_type=sa.NUMERIC(), + type_=sa.Integer()) + + +def downgrade(): + op.alter_column('ft_billing', 'rate_multiplier', + existing_type=sa.Integer(), + type_=sa.NUMERIC()) + op.alter_column('ft_billing', 'billable_units', + existing_type=sa.Integer(), + type_=sa.NUMERIC(), + existing_nullable=True) From 99d1357c379b2df5219891da7f64e4584060f069 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 10 May 2018 17:13:38 +0100 Subject: [PATCH 15/41] Fix the logging message in the nightly task --- app/celery/reporting_tasks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index 607fad160..5834c73f3 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -26,4 +26,5 @@ def create_nightly_billing(day_start=None): for data in transit_data: update_fact_billing(data, process_day) - current_app.logger.info("create-nightly-billing task complete. {} rows updated".format(len(transit_data))) + current_app.logger.info( + "create-nightly-billing task complete. {} rows updated for day: {}".format(len(transit_data, process_day))) From 178a851d8168c51490edd3ea4e200dc1748a3f9a Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Thu, 10 May 2018 23:43:16 +0100 Subject: [PATCH 16/41] Update requests-mock from 1.4.0 to 1.5.0 --- requirements_for_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 8166cfcaf..e9107ac16 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -8,7 +8,7 @@ pytest-cov==2.5.1 pytest-xdist==1.22.2 coveralls==1.3.0 freezegun==0.3.10 -requests-mock==1.4.0 +requests-mock==1.5.0 # optional requirements for jsonschema strict-rfc3339==0.7 rfc3987==1.3.7 From 307fd24072575b31594fbab249c4f087a1186a09 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 11 May 2018 10:57:04 +0100 Subject: [PATCH 17/41] Revert "Update gunicorn to 19.8.1" --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 8960b54a3..4e487d5e1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,7 @@ Flask-SQLAlchemy==2.3.2 Flask==0.12.2 click-datetime==0.2 eventlet==0.22.1 -gunicorn==19.8.1 +gunicorn==19.7.1 iso8601==0.1.12 jsonschema==2.6.0 marshmallow-sqlalchemy==0.13.2 From d98581cfe65f41054256183ea4ae03ccc5016b81 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 11 May 2018 16:25:16 +0100 Subject: [PATCH 18/41] Added a new endpoint for yearly usage totals using ft_billing. --- app/billing/billing_schemas.py | 15 +++++ app/billing/rest.py | 25 ++++++-- app/celery/reporting_tasks.py | 2 +- app/dao/fact_billing_dao.py | 25 ++++++++ tests/app/billing/test_billing.py | 90 ++++++++++++++++++++++++---- tests/app/dao/test_ft_billing_dao.py | 79 ++++++++++++++++-------- 6 files changed, 193 insertions(+), 43 deletions(-) diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index 572228439..007a1cd49 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -1,5 +1,6 @@ from datetime import datetime + create_or_update_free_sms_fragment_limit_schema = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "POST annual billing schema", @@ -24,3 +25,17 @@ def serialize_ft_billing_remove_emails(data): } results.append(json_result) return results + + +def serialize_ft_billing_yearly_totals(data): + yearly_totals = [] + for total in data: + json_result = { + "notification_type": total.notification_type, + "billing_units": total.billable_units, + "rate": float(total.rate), + "letter_total": float(total.billable_units * total.rate) if total.notification_type == 'letter' else 0 + } + yearly_totals.append(json_result) + + return yearly_totals diff --git a/app/billing/rest.py b/app/billing/rest.py index 77203c80c..e18f08282 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -5,7 +5,8 @@ from flask import Blueprint, jsonify, request from app.billing.billing_schemas import ( create_or_update_free_sms_fragment_limit_schema, - serialize_ft_billing_remove_emails + serialize_ft_billing_remove_emails, + serialize_ft_billing_yearly_totals, ) from app.dao.annual_billing_dao import ( dao_get_free_sms_fragment_limit_for_year, @@ -15,7 +16,7 @@ from app.dao.annual_billing_dao import ( ) from app.dao.date_util import get_current_financial_year_start_year from app.dao.date_util import get_months_for_financial_year -from app.dao.fact_billing_dao import fetch_monthly_billing_for_year +from app.dao.fact_billing_dao import fetch_monthly_billing_for_year, fetch_billing_totals_for_year from app.dao.monthly_billing_dao import ( get_billing_data_for_financial_year, get_monthly_billing_by_notification_type @@ -47,6 +48,18 @@ def get_yearly_usage_by_monthly_from_ft_billing(service_id): return jsonify(data) +@billing_blueprint.route('/ft-yearly-usage-summary') +def get_yearly_billing_usage_summary_from_ft_billing(service_id): + try: + year = int(request.args.get('year')) + except TypeError: + return jsonify(result='error', message='No valid year provided'), 400 + + billing_data = fetch_billing_totals_for_year(service_id, year) + data = serialize_ft_billing_yearly_totals(billing_data) + return jsonify(data) + + @billing_blueprint.route('/monthly-usage') def get_yearly_usage_by_month(service_id): try: @@ -70,7 +83,7 @@ def get_yearly_billing_usage_summary(service_id): try: year = int(request.args.get('year')) billing_data = get_billing_data_for_financial_year(service_id, year) - notification_types = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] + notification_types = [EMAIL_TYPE, LETTER_TYPE, SMS_TYPE] response = [ _get_total_billable_units_and_rate_for_notification_type(billing_data, notification_type) for notification_type in notification_types @@ -102,8 +115,8 @@ def _get_total_billable_units_and_rate_for_notification_type(billing_data, noti_ return { "notification_type": noti_type, "billing_units": total_sent, - "rate": rate, - "letter_total": letter_total + "rate": float(rate), + "letter_total": round(float(letter_total), 3) } @@ -132,7 +145,7 @@ def _transform_billing_for_month_letters(billing_for_month): "month": month_name, "billing_units": (total['billing_units'] * total['rate_multiplier']), "notification_type": billing_for_month.notification_type, - "rate": total['rate'] + "rate": float(total['rate']) } x.append(y) if len(billing_for_month.monthly_totals) == 0: diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index 5834c73f3..b86310785 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -27,4 +27,4 @@ def create_nightly_billing(day_start=None): update_fact_billing(data, process_day) current_app.logger.info( - "create-nightly-billing task complete. {} rows updated for day: {}".format(len(transit_data, process_day))) + "create-nightly-billing task complete. {} rows updated for day: {}".format(len(transit_data), process_day)) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index fc8ebd044..6c4ed7a90 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -19,6 +19,31 @@ from app.models import ( from app.utils import convert_utc_to_bst, convert_bst_to_utc +def fetch_billing_totals_for_year(service_id, year): + year_start_date, year_end_date = get_financial_year(year) + + yearly_data = db.session.query( + func.sum(FactBilling.notifications_sent).label("notifications_sent"), + func.sum(FactBilling.billable_units * FactBilling.rate_multiplier).label("billable_units"), + FactBilling.service_id, + FactBilling.rate, + FactBilling.notification_type + ).filter( + FactBilling.service_id == service_id, + FactBilling.bst_date >= year_start_date, + FactBilling.bst_date <= year_end_date + ).group_by( + FactBilling.service_id, + FactBilling.rate, + FactBilling.notification_type + ).order_by( + FactBilling.service_id, + FactBilling.notification_type + ).all() + + return yearly_data + + def fetch_monthly_billing_for_year(service_id, year): year_start_date, year_end_date = get_financial_year(year) utcnow = datetime.utcnow() diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 06a7c7a06..e13c0777b 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -67,24 +67,23 @@ def test_get_yearly_billing_summary_returns_correct_breakdown(client, sample_tem assert len(resp_json) == 3 _assert_dict_equals(resp_json[0], { - 'notification_type': SMS_TYPE, - 'billing_units': 8, - 'rate': 0.12, - 'letter_total': 0 - }) - - _assert_dict_equals(resp_json[1], { 'notification_type': EMAIL_TYPE, 'billing_units': 0, 'rate': 0, 'letter_total': 0 }) - _assert_dict_equals(resp_json[2], { + _assert_dict_equals(resp_json[1], { 'notification_type': LETTER_TYPE, 'billing_units': 2, 'rate': 0, 'letter_total': 0.72 }) + _assert_dict_equals(resp_json[2], { + 'notification_type': SMS_TYPE, + 'billing_units': 8, + 'rate': 0.12, + 'letter_total': 0 + }) def test_get_yearly_billing_usage_breakdown_returns_400_if_missing_year(client, sample_service): @@ -485,6 +484,21 @@ def test_get_yearly_usage_by_monthly_from_ft_billing(client, notify_db_session): def test_compare_ft_billing_to_monthly_billing(client, notify_db_session): + service = set_up_yearly_data() + + monthly_billing_response = client.get('/service/{}/billing/monthly-usage?year=2016'.format(service.id), + headers=[create_authorization_header()]) + + ft_billing_response = client.get('service/{}/billing/ft-monthly-usage?year=2016'.format(service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + monthly_billing_json_resp = json.loads(monthly_billing_response.get_data(as_text=True)) + ft_billing_json_resp = json.loads(ft_billing_response.get_data(as_text=True)) + + assert monthly_billing_json_resp == ft_billing_json_resp + + +def set_up_yearly_data(): service = create_service() sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") @@ -542,14 +556,68 @@ def test_compare_ft_billing_to_monthly_billing(client, notify_db_session): "rate_multiplier": 1, "billing_units": int(d), "total_cost": 0.33 * int(d)}] ) + return service - monthly_billing_response = client.get('/service/{}/billing/monthly-usage?year=2016'.format(service.id), + +def test_get_yearly_billing_usage_summary_from_ft_billing_comapre_to_monthyl_billing( + client, notify_db_session +): + service = set_up_yearly_data() + monthly_billing_response = client.get('/service/{}/billing/yearly-usage-summary?year=2016'.format(service.id), headers=[create_authorization_header()]) - ft_billing_response = client.get('service/{}/billing/ft-monthly-usage?year=2016'.format(service.id), + ft_billing_response = client.get('service/{}/billing/ft-yearly-usage-summary?year=2016'.format(service.id), headers=[('Content-Type', 'application/json'), create_authorization_header()]) monthly_billing_json_resp = json.loads(monthly_billing_response.get_data(as_text=True)) ft_billing_json_resp = json.loads(ft_billing_response.get_data(as_text=True)) - assert monthly_billing_json_resp == ft_billing_json_resp + assert len(monthly_billing_json_resp) == 3 + assert len(ft_billing_json_resp) == 3 + for i in range(0, 3): + assert sorted(monthly_billing_json_resp[i]) == sorted(ft_billing_json_resp[i]) + + +def test_get_yearly_billing_usage_summary_from_ft_billing_returns_400_if_missing_year(client, sample_service): + response = client.get( + '/service/{}/billing/ft-yearly-usage-summary'.format(sample_service.id), + headers=[create_authorization_header()] + ) + assert response.status_code == 400 + assert json.loads(response.get_data(as_text=True)) == { + 'message': 'No valid year provided', 'result': 'error' + } + + +def test_get_yearly_billing_usage_summary_from_ft_billing_returns_empty_list_if_no_billing_data( + client, sample_service +): + response = client.get( + '/service/{}/billing/ft-yearly-usage-summary?year=2016'.format(sample_service.id), + headers=[create_authorization_header()] + ) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == [] + + +def test_get_yearly_billing_usage_summary_from_ft_billing(client, notify_db_session): + service = set_up_yearly_data() + + response = client.get('/service/{}/billing/ft-yearly-usage-summary?year=2016'.format(service.id), + headers=[create_authorization_header()] + ) + assert response.status_code == 200 + json_response = json.loads(response.get_data(as_text=True)) + assert len(json_response) == 3 + assert json_response[0]['notification_type'] == 'email' + assert json_response[0]['billing_units'] == 275 + assert json_response[0]['rate'] == 0 + assert json_response[0]['letter_total'] == 0 + assert json_response[1]['notification_type'] == 'letter' + assert json_response[1]['billing_units'] == 275 + assert json_response[1]['rate'] == 0.33 + assert json_response[1]['letter_total'] == 90.75 + assert json_response[2]['notification_type'] == 'sms' + assert json_response[2]['billing_units'] == 825 + assert json_response[2]['rate'] == 0.0162 + assert json_response[0]['letter_total'] == 0 diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index f6df9e4bd..4cd880ea0 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -8,6 +8,7 @@ from app import db from app.dao.fact_billing_dao import ( fetch_monthly_billing_for_year, fetch_billing_data_for_day, get_rates_for_billing, get_rate, + fetch_billing_totals_for_year, ) from app.models import FactBilling from app.utils import convert_utc_to_bst @@ -21,6 +22,34 @@ from tests.app.db import ( ) +def set_up_yearly_data(): + service = create_service() + sms_template = create_template(service=service, template_type="sms") + email_template = create_template(service=service, template_type="email") + letter_template = create_template(service=service, template_type="letter") + for year in (2016, 2017): + for month in range(1, 13): + mon = str(month).zfill(2) + for day in range(1, monthrange(year, month)[1] + 1): + d = str(day).zfill(2) + create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), + service=service, + template=sms_template, + notification_type='sms', + rate=0.162) + create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), + service=service, + template=email_template, + notification_type='email', + rate=0) + create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), + service=service, + template=letter_template, + notification_type='letter', + rate=0.33) + return service + + def test_fetch_billing_data_for_today_includes_data_with_the_right_status(notify_db_session): service = create_service() template = create_template(service=service, template_type="email") @@ -256,31 +285,7 @@ def test_fetch_monthly_billing_for_year_adds_data_for_today(notify_db_session): def test_fetch_monthly_billing_for_year_return_financial_year(notify_db_session): - service = create_service() - sms_template = create_template(service=service, template_type="sms") - email_template = create_template(service=service, template_type="email") - letter_template = create_template(service=service, template_type="letter") - - for year in (2016, 2017): - for month in range(1, 13): - mon = str(month).zfill(2) - for day in range(1, monthrange(year, month)[1] + 1): - d = str(day).zfill(2) - create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), - service=service, - template=sms_template, - notification_type='sms', - rate=0.162) - create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), - service=service, - template=email_template, - notification_type='email', - rate=0) - create_ft_billing(bst_date='{}-{}-{}'.format(year, mon, d), - service=service, - template=letter_template, - notification_type='letter', - rate=0.33) + service = set_up_yearly_data() results = fetch_monthly_billing_for_year(service.id, 2016) # returns 3 rows, per month, returns financial year april to end of march @@ -304,3 +309,27 @@ def test_fetch_monthly_billing_for_year_return_financial_year(notify_db_session) assert results[2].rate == Decimal('0.162') assert str(results[3].month) == "2016-05-01" assert str(results[35].month) == "2017-03-01" + + +def test_fetch_billing_totals_for_year(notify_db_session): + service = set_up_yearly_data() + results = fetch_billing_totals_for_year(service_id=service.id, year=2016) + + assert len(results) == 3 + assert results[0].notification_type == 'email' + assert results[0].service_id == service.id + assert results[0].notifications_sent == 365 + assert results[0].billable_units == 365 + assert results[0].rate == Decimal('0') + + assert results[1].notification_type == 'letter' + assert results[1].service_id == service.id + assert results[1].notifications_sent == 365 + assert results[1].billable_units == 365 + assert results[1].rate == Decimal('0.33') + + assert results[2].notification_type == 'sms' + assert results[2].service_id == service.id + assert results[2].notifications_sent == 365 + assert results[2].billable_units == 365 + assert results[2].rate == Decimal('0.162') From bb9eb367b76b566ffdecb16c76e67d9a0a83c35d Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 3 May 2018 16:40:54 +0100 Subject: [PATCH 19/41] Migration to create the ft_notification_status table Added the migration for the `ft_notification_status` table which will be used to look up the counts for notifications by various different fields. The `job_id` column is non-nullable because we want to include it in the composite primary key. For notifications which weren't part of a job, the `job_id` will be set to `0000-00000-0000-00000` when populating the data. --- .../0188_add_ft_notification_status.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 migrations/versions/0188_add_ft_notification_status.py diff --git a/migrations/versions/0188_add_ft_notification_status.py b/migrations/versions/0188_add_ft_notification_status.py new file mode 100644 index 000000000..403735837 --- /dev/null +++ b/migrations/versions/0188_add_ft_notification_status.py @@ -0,0 +1,39 @@ +""" + +Revision ID: 0188_add_ft_notification_status +Revises: 0187_another_letter_org +Create Date: 2018-05-03 10:10:41.824981 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0188_add_ft_notification_status' +down_revision = '0187_another_letter_org' + + +def upgrade(): + op.create_table('ft_notification_status', + sa.Column('bst_date', sa.Date(), nullable=False), + sa.Column('template_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('job_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('notification_type', sa.Text(), nullable=False), + sa.Column('key_type', sa.Text(), nullable=False), + sa.Column('notification_status', sa.Text(), nullable=False), + sa.Column('notification_count', sa.Integer(), nullable=False), + sa.PrimaryKeyConstraint('bst_date', 'template_id', 'service_id', 'job_id', 'notification_type', 'key_type', 'notification_status') + ) + op.create_index(op.f('ix_ft_notification_status_bst_date'), 'ft_notification_status', ['bst_date'], unique=False) + op.create_index(op.f('ix_ft_notification_status_job_id'), 'ft_notification_status', ['job_id'], unique=False) + op.create_index(op.f('ix_ft_notification_status_service_id'), 'ft_notification_status', ['service_id'], unique=False) + op.create_index(op.f('ix_ft_notification_status_template_id'), 'ft_notification_status', ['template_id'], unique=False) + + +def downgrade(): + op.drop_index(op.f('ix_ft_notification_status_bst_date'), table_name='ft_notification_status') + op.drop_index(op.f('ix_ft_notification_status_template_id'), table_name='ft_notification_status') + op.drop_index(op.f('ix_ft_notification_status_service_id'), table_name='ft_notification_status') + op.drop_index(op.f('ix_ft_notification_status_job_id'), table_name='ft_notification_status') + op.drop_table('ft_notification_status') From c9dc6f724dff46cc8c794b277e8b6ddb203aeb7d Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 9 May 2018 14:06:46 +0100 Subject: [PATCH 20/41] Add FactNotificationStatus model Added the FactNotificationStatus model for the ft_notification_status table. --- app/models.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/models.py b/app/models.py index 547c0a0d9..a3f4eb1ec 100644 --- a/app/models.py +++ b/app/models.py @@ -1812,3 +1812,16 @@ class DateTimeDimension(db.Model): Index('ix_dm_datetime_yearmonth', DateTimeDimension.year, DateTimeDimension.month) + + +class FactNotificationStatus(db.Model): + __tablename__ = "ft_notification_status" + + bst_date = db.Column(db.Date, index=True, primary_key=True, nullable=False) + template_id = db.Column(UUID(as_uuid=True), primary_key=True, index=True, nullable=False) + service_id = db.Column(UUID(as_uuid=True), primary_key=True, index=True, nullable=False, ) + job_id = db.Column(UUID(as_uuid=True), primary_key=True, index=True, nullable=False) + notification_type = db.Column(db.Text, primary_key=True, nullable=False) + key_type = db.Column(db.Text, primary_key=True, nullable=False) + notification_status = db.Column(db.Text, primary_key=True, nullable=False) + notification_count = db.Column(db.Integer(), nullable=False) From 13f36620515894cd36731969b5526bd723806271 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 9 May 2018 16:05:07 +0100 Subject: [PATCH 21/41] Add command to populate data --- app/commands.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/app/commands.py b/app/commands.py index 6c4d1f659..0e9a33193 100644 --- a/app/commands.py +++ b/app/commands.py @@ -603,3 +603,83 @@ def compare_ft_billing_to_monthly_billing(year, service_id=None): path='/service/{}/billing/ft-monthly-usage?year={}'.format(service_id, year)): ft_billing_response = get_yearly_usage_by_monthly_from_ft_billing(service_id) compare_monthly_billing_to_ft_billing(ft_billing_response, monthly_billing_response) + + +@notify_command(name='migrate-data-to-ft-notification-status') +@click.option('-s', '--start_date', required=True, help="start date inclusive", type=click_dt(format='%Y-%m-%d')) +@click.option('-e', '--end_date', required=True, help="end date inclusive", type=click_dt(format='%Y-%m-%d')) +@statsd(namespace="tasks") +def migrate_data_to_ft_notification_status(start_date, end_date): + + print('Notification statuses migration from date {} to {}'.format(start_date, end_date)) + + process_date = start_date + total_updated = 0 + + while process_date < end_date: + sql = \ + """ + select count(*) from notification_history + where created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' + and created_at < (date :end + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' + """ + num_notifications = db.session.execute(sql, {"start": process_date, + "end": process_date + timedelta(days=1)}).fetchall()[0][0] + + sql = \ + """ + select count(*) from + (select distinct template_id, service_id, job_id, notification_type, key_type, notification_status + from notification_history + where created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' + and created_at < (date :end + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' + ) as distinct_records + """ + predicted_records = db.session.execute(sql, {"start": process_date, + "end": process_date + timedelta(days=1)}).fetchall()[0][0] + + start_time = datetime.now() + print('ft_notification-status: Migrating date: {}, notifications: {}, expecting {} ft_notification_status rows' + .format(process_date.date(), num_notifications, predicted_records)) + + # migrate data into ft_notification_status and update if record already exists + sql = \ + """ + insert into ft_notification_status (bst_date, template_id, service_id, job_id, notification_type, key_type, + notification_status, notification_count) + select bst_date, template_id, service_id, job_id, notification_type, key_type, notification_status, + sum(notification_count) as notification_count + from ( + select + da.bst_date, + n.template_id, + n.service_id, + coalesce(n.job_id, '00000000-0000-0000-0000-000000000000') as job_id, + n.notification_type, + n.key_type, + n.notification_status, + 1 as notification_count + from public.notification_history n + left join dm_datetime da on n.created_at >= da.utc_daytime_start + and n.created_at < da.utc_daytime_end + where n.created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' + at time zone 'UTC' + and n.created_at < (date :end + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' + ) as individual_record + group by bst_date, template_id, service_id, job_id, notification_type, key_type, notification_status + order by bst_date + on conflict on constraint ft_notification_status_pkey do update set + notification_count = excluded.notification_count + """ + result = db.session.execute(sql, {"start": process_date, "end": process_date + timedelta(days=1)}) + db.session.commit() + print('ft_notification_status: --- Completed took {}ms. Migrated {} rows.'.format(datetime.now() - start_time, + result.rowcount)) + if predicted_records != result.rowcount: + print(' : ^^^ Result mismatch by {} rows ^^^' + .format(predicted_records - result.rowcount)) + + process_date += timedelta(days=1) + + total_updated += result.rowcount + print('Total inserted/updated records = {}'.format(total_updated)) From 56d7a7242b97f13650f49b97e0b50b96c348270e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 May 2018 09:49:24 +0100 Subject: [PATCH 22/41] Fix merge conflict in db migration --- migrations/versions/0189_ft_billing_data_type.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/versions/0189_ft_billing_data_type.py b/migrations/versions/0189_ft_billing_data_type.py index 951583ad6..e18714f8b 100644 --- a/migrations/versions/0189_ft_billing_data_type.py +++ b/migrations/versions/0189_ft_billing_data_type.py @@ -1,7 +1,7 @@ """ Revision ID: 0189_ft_billing_data_type -Revises: 0187_another_letter_org +Revises: 0188_add_ft_notification_status Create Date: 2018-05-10 14:57:52.589773 """ @@ -9,7 +9,7 @@ from alembic import op import sqlalchemy as sa revision = '0189_ft_billing_data_type' -down_revision = '0187_another_letter_org' +down_revision = '0188_add_ft_notification_status' def upgrade(): From 828ebb60793004462404fdcc66bf31a029196431 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 May 2018 12:48:42 +0100 Subject: [PATCH 23/41] Fix test --- tests/app/billing/test_billing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index e13c0777b..43b49dfa2 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -620,4 +620,4 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(client, notify_db_sess assert json_response[2]['notification_type'] == 'sms' assert json_response[2]['billing_units'] == 825 assert json_response[2]['rate'] == 0.0162 - assert json_response[0]['letter_total'] == 0 + assert json_response[2]['letter_total'] == 0 From 3615f3d00f533dddc1eaece4a462adcf1b5a33fd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 May 2018 16:21:16 +0100 Subject: [PATCH 24/41] In order to re-run the create_nightly_billing for dates in the past, we added a condition on which table is used. This will allow us to re-run nightly billing for those 2 days where it failed. For the majority of time the query will run on Notiifcations. --- app/dao/fact_billing_dao.py | 50 ++++++++++++++++------------ tests/app/dao/test_ft_billing_dao.py | 15 ++++++++- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 6c4ed7a90..0c86e1ac9 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -14,7 +14,8 @@ from app.models import ( LETTER_TYPE, SMS_TYPE, Rate, - LetterRate + LetterRate, + NotificationHistory ) from app.utils import convert_utc_to_bst, convert_bst_to_utc @@ -84,43 +85,48 @@ def fetch_monthly_billing_for_year(service_id, year): def fetch_billing_data_for_day(process_day, service_id=None): start_date = convert_bst_to_utc(datetime.combine(process_day, time.min)) end_date = convert_bst_to_utc(datetime.combine(process_day + timedelta(days=1), time.min)) + # use notification_history if process day is older than 7 days + # this is useful if we need to rebuild the ft_billing table for a date older than 7 days ago. + table = Notification + if start_date < datetime.utcnow() - timedelta(days=7): + table = NotificationHistory transit_data = db.session.query( - Notification.template_id, - Notification.service_id, - Notification.notification_type, - func.coalesce(Notification.sent_by, + table.template_id, + table.service_id, + table.notification_type, + func.coalesce(table.sent_by, case( [ - (Notification.notification_type == 'letter', 'dvla'), - (Notification.notification_type == 'sms', 'unknown'), - (Notification.notification_type == 'email', 'ses') + (table.notification_type == 'letter', 'dvla'), + (table.notification_type == 'sms', 'unknown'), + (table.notification_type == 'email', 'ses') ]), ).label('sent_by'), - func.coalesce(Notification.rate_multiplier, 1).label('rate_multiplier'), - func.coalesce(Notification.international, False).label('international'), - func.sum(Notification.billable_units).label('billable_units'), + func.coalesce(table.rate_multiplier, 1).label('rate_multiplier'), + func.coalesce(table.international, False).label('international'), + func.sum(table.billable_units).label('billable_units'), func.count().label('notifications_sent'), Service.crown, ).filter( - Notification.status != NOTIFICATION_CREATED, # at created status, provider information is not available - Notification.status != NOTIFICATION_TECHNICAL_FAILURE, - Notification.key_type != KEY_TYPE_TEST, - Notification.created_at >= start_date, - Notification.created_at < end_date + table.status != NOTIFICATION_CREATED, # at created status, provider information is not available + table.status != NOTIFICATION_TECHNICAL_FAILURE, + table.key_type != KEY_TYPE_TEST, + table.created_at >= start_date, + table.created_at < end_date ).group_by( - Notification.template_id, - Notification.service_id, - Notification.notification_type, + table.template_id, + table.service_id, + table.notification_type, 'sent_by', - Notification.rate_multiplier, - Notification.international, + table.rate_multiplier, + table.international, Service.crown ).join( Service ) if service_id: - transit_data = transit_data.filter(Notification.service_id == service_id) + transit_data = transit_data.filter(table.service_id == service_id) return transit_data.all() diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 4cd880ea0..09deb9266 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -10,7 +10,7 @@ from app.dao.fact_billing_dao import ( get_rate, fetch_billing_totals_for_year, ) -from app.models import FactBilling +from app.models import FactBilling, Notification from app.utils import convert_utc_to_bst from tests.app.db import ( create_ft_billing, @@ -187,6 +187,19 @@ def test_fetch_billing_data_for_day_returns_empty_list(notify_db_session): assert results == [] +def test_fetch_billing_data_for_day_uses_notification_history(notify_db_session): + service = create_service() + sms_template = create_template(service=service, template_type='sms') + create_notification(template=sms_template, status='delivered', created_at=datetime.utcnow() - timedelta(days=8)) + create_notification(template=sms_template, status='delivered', created_at=datetime.utcnow() - timedelta(days=8)) + + Notification.query.delete() + db.session.commit() + results = fetch_billing_data_for_day(process_day=datetime.utcnow() - timedelta(days=8), service_id=service.id) + assert len(results) == 1 + assert results[0].notifications_sent == 2 + + def test_fetch_billing_data_for_day_returns_list_for_given_service(notify_db_session): service = create_service() service_2 = create_service(service_name='Service 2') From 271ce6d76efb7f8a07df98bc5140508b4d53057a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 May 2018 11:21:10 +0100 Subject: [PATCH 25/41] Changed the update/insert to a postgres upsert to avoid concurrency issues. --- app/dao/fact_billing_dao.py | 66 +++++++++++++++--------- tests/app/celery/test_reporting_tasks.py | 15 ++++++ 2 files changed, 56 insertions(+), 25 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 0c86e1ac9..18387a25e 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -1,5 +1,6 @@ from datetime import datetime, timedelta, time +from sqlalchemy.dialects.postgresql import insert from sqlalchemy import func, case, desc, Date from app import db @@ -149,33 +150,48 @@ def get_rate(non_letter_rates, letter_rates, notification_type, date, crown=None def update_fact_billing(data, process_day): - inserted_records = 0 - updated_records = 0 non_letter_rates, letter_rates = get_rates_for_billing() - update_count = FactBilling.query.filter( - FactBilling.bst_date == datetime.date(process_day), - FactBilling.template_id == data.template_id, - FactBilling.service_id == data.service_id, - FactBilling.provider == data.sent_by, # This could be zero - this is a bug that needs to be fixed. - FactBilling.rate_multiplier == data.rate_multiplier, - FactBilling.notification_type == data.notification_type, - FactBilling.international == data.international - ).update( - {"notifications_sent": data.notifications_sent, - "billable_units": data.billable_units}, - synchronize_session=False) + rate = get_rate(non_letter_rates, + letter_rates, + data.notification_type, + process_day, + data.crown, + data.rate_multiplier) + billing_record = create_billing_record(data, rate, process_day) - if update_count == 0: - rate = get_rate(non_letter_rates, - letter_rates, - data.notification_type, - process_day, - data.crown, - data.rate_multiplier) - billing_record = create_billing_record(data, rate, process_day) - db.session.add(billing_record) - inserted_records += 1 - updated_records += update_count + table = FactBilling.__table__ + ''' + This uses the Postgres upsert to avoid race conditions when two threads try to insert + at the same row. The excluded object refers to values that we tried to insert but were + rejected. + http://docs.sqlalchemy.org/en/latest/dialects/postgresql.html#insert-on-conflict-upsert + ''' + stmt = insert(table).values( + bst_date=billing_record.bst_date, + template_id=billing_record.template_id, + service_id=billing_record.service_id, + provider=billing_record.provider, + rate_multiplier=billing_record.rate_multiplier, + notification_type=billing_record.notification_type, + international=billing_record.international, + billable_units=billing_record.billable_units, + notifications_sent=billing_record.notifications_sent, + rate=billing_record.rate + ) + + stmt = stmt.on_conflict_do_update( + index_elements=[table.c.bst_date, + table.c.template_id, + table.c.service_id, + table.c.provider, + table.c.rate_multiplier, + table.c.notification_type, + table.c.international], + set_={"notifications_sent": stmt.excluded.notifications_sent, + "billable_units": stmt.excluded.billable_units + } + ) + db.session.connection().execute(stmt) db.session.commit() diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 2e33fcc45..9f058654c 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -438,7 +438,22 @@ def test_create_nightly_billing_update_when_record_exists( assert len(records) == 1 assert records[0].bst_date == date(2018, 1, 14) + assert records[0].billable_units == 1 + + sample_notification( + notify_db, + notify_db_session, + created_at=datetime.now() - timedelta(days=1), + service=sample_service, + template=sample_template, + status='delivered', + sent_by=None, + international=False, + rate_multiplier=1.0, + billable_units=1, + ) # run again, make sure create_nightly_billing() updates with no error create_nightly_billing() assert len(records) == 1 + assert records[0].billable_units == 2 From 40d8f78b2bb0d15d3cdf0a0927911e011590dba4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 May 2018 14:00:06 +0100 Subject: [PATCH 26/41] Convert the day_start from a string to a datetime. --- app/celery/reporting_tasks.py | 4 +++- app/commands.py | 7 +++---- tests/app/celery/test_reporting_tasks.py | 22 +++++++++++++++------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index b86310785..572625e35 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -17,7 +17,9 @@ def create_nightly_billing(day_start=None): # 3 days of data counting back from day_start is consolidated if day_start is None: day_start = datetime.today() - timedelta(days=1) - + else: + # When calling the task its a string in the format of "YYYY-MM-DD" + day_start = datetime.strptime(day_start, "%Y-%m-%d") for i in range(0, 3): process_day = day_start - timedelta(days=i) diff --git a/app/commands.py b/app/commands.py index 329054317..841da2eff 100644 --- a/app/commands.py +++ b/app/commands.py @@ -581,13 +581,12 @@ def compare_ft_billing_to_monthly_billing(year, service_id=None): This command checks the results of monthly_billing to ft_billing for the given year. If service id is not included all services are compared for the given year. """ - - def compare_monthly_billing_to_ft_billing(ft_billing_response, monthly_billing_response): + def compare_monthly_billing_to_ft_billing(ft_billing_resp, monthly_billing_resp): # Remove the rows with 0 billing_units and rate, ft_billing doesn't populate those rows. - mo_json = json.loads(monthly_billing_response.get_data(as_text=True)) + mo_json = json.loads(monthly_billing_resp.get_data(as_text=True)) rm_zero_rows = [x for x in mo_json if x['billing_units'] != 0 and x['rate'] != 0] try: - assert rm_zero_rows == json.loads(ft_billing_response.get_data(as_text=True)) + assert rm_zero_rows == json.loads(ft_billing_resp.get_data(as_text=True)) except AssertionError: print("Comparison failed for service: {} and year: {}".format(service_id, year)) diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 9f058654c..7f181d046 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -76,7 +76,9 @@ def test_create_nightly_billing_sms_rate_multiplier( records = FactBilling.query.all() assert len(records) == 0 - create_nightly_billing(yesterday) + # Celery expects the arguments to be a string or primitive type. + yesterday_str = datetime.strftime(yesterday, "%Y-%m-%d") + create_nightly_billing(yesterday_str) records = FactBilling.query.order_by('rate_multiplier').all() assert len(records) == records_num for i, record in enumerate(records): @@ -124,8 +126,9 @@ def test_create_nightly_billing_different_templates( records = FactBilling.query.all() assert len(records) == 0 - - create_nightly_billing(yesterday) + # Celery expects the arguments to be a string or primitive type. + yesterday_str = datetime.strftime(yesterday, "%Y-%m-%d") + create_nightly_billing(yesterday_str) records = FactBilling.query.order_by('rate_multiplier').all() assert len(records) == 2 @@ -179,7 +182,9 @@ def test_create_nightly_billing_different_sent_by( records = FactBilling.query.all() assert len(records) == 0 - create_nightly_billing(yesterday) + # Celery expects the arguments to be a string or primitive type. + yesterday_str = datetime.strftime(yesterday, "%Y-%m-%d") + create_nightly_billing(yesterday_str) records = FactBilling.query.order_by('rate_multiplier').all() assert len(records) == 2 @@ -215,8 +220,9 @@ def test_create_nightly_billing_letter( records = FactBilling.query.all() assert len(records) == 0 - - create_nightly_billing(yesterday) + # Celery expects the arguments to be a string or primitive type. + yesterday_str = datetime.strftime(yesterday, "%Y-%m-%d") + create_nightly_billing(yesterday_str) records = FactBilling.query.order_by('rate_multiplier').all() assert len(records) == 1 record = records[0] @@ -253,7 +259,9 @@ def test_create_nightly_billing_null_sent_by_sms( records = FactBilling.query.all() assert len(records) == 0 - create_nightly_billing(yesterday) + # Celery expects the arguments to be a string or primitive type. + yesterday_str = datetime.strftime(yesterday, "%Y-%m-%d") + create_nightly_billing(yesterday_str) records = FactBilling.query.all() assert len(records) == 1 From f6b8611b18348b2015c8b17f2b951214395fa612 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 16 May 2018 10:18:55 +0100 Subject: [PATCH 27/41] Add letter logos for TWFRS and Thames Valley Police --- .../versions/0190_another_letter_org.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 migrations/versions/0190_another_letter_org.py diff --git a/migrations/versions/0190_another_letter_org.py b/migrations/versions/0190_another_letter_org.py new file mode 100644 index 000000000..be2776750 --- /dev/null +++ b/migrations/versions/0190_another_letter_org.py @@ -0,0 +1,37 @@ +"""empty message + +Revision ID: 0190_another_letter_org +Revises: 0189_ft_billing_data_type +Create Date: 2017-06-29 12:44:16.815039 + +""" + +# revision identifiers, used by Alembic. +revision = '0190_another_letter_org' +down_revision = '0189_ft_billing_data_type' + +from alembic import op + + +NEW_ORGANISATIONS = [ + ('506', 'Tyne and Wear Fire and Rescue Service'), + ('507', 'Thames Valley Police'), +] + + +def upgrade(): + for numeric_id, name in NEW_ORGANISATIONS: + op.execute(""" + INSERT + INTO dvla_organisation + VALUES ('{}', '{}') + """.format(numeric_id, name)) + + +def downgrade(): + for numeric_id, _ in NEW_ORGANISATIONS: + op.execute(""" + DELETE + FROM dvla_organisation + WHERE id = '{}' + """.format(numeric_id)) From eb083e30edb0b78dee0daff9f5fa271c8c2f0f79 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 16 May 2018 12:21:59 +0100 Subject: [PATCH 28/41] - Only rebuild current month for monthly_billing if today is in the current year. - Change the usage queries to a union so that billing_units is correct for all notification types. Removing the business logic from the schema. - Added tests for different fragment counts, rates and sheet counts. --- app/billing/billing_schemas.py | 10 +- app/dao/fact_billing_dao.py | 76 ++++++++++--- app/dao/monthly_billing_dao.py | 6 +- tests/app/billing/test_billing.py | 147 +++++++++++++++++++++++++- tests/app/dao/test_ft_billing_dao.py | 5 - tests/app/dao/test_monthly_billing.py | 10 ++ 6 files changed, 224 insertions(+), 30 deletions(-) diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index 007a1cd49..fb41901d8 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -16,12 +16,12 @@ create_or_update_free_sms_fragment_limit_schema = { def serialize_ft_billing_remove_emails(data): results = [] billed_notifications = [x for x in data if x.notification_type != 'email'] - for notifications in billed_notifications: + for notification in billed_notifications: json_result = { - "month": (datetime.strftime(notifications.month, "%B")), - "notification_type": notifications.notification_type, - "billing_units": int(notifications.billable_units), - "rate": float(notifications.rate), + "month": (datetime.strftime(notification.month, "%B")), + "notification_type": notification.notification_type, + "billing_units": notification.billable_units, + "rate": float(notification.rate), } results.append(json_result) return results diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 18387a25e..9a3a30148 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -1,5 +1,6 @@ from datetime import datetime, timedelta, time +from flask import current_app from sqlalchemy.dialects.postgresql import insert from sqlalchemy import func, case, desc, Date @@ -16,31 +17,55 @@ from app.models import ( SMS_TYPE, Rate, LetterRate, - NotificationHistory + NotificationHistory, + EMAIL_TYPE ) from app.utils import convert_utc_to_bst, convert_bst_to_utc def fetch_billing_totals_for_year(service_id, year): year_start_date, year_end_date = get_financial_year(year) - - yearly_data = db.session.query( + """ + Billing for email: only record the total number of emails. + Billing for letters: The billing units is used to fetch the correct rate for the sheet count of the letter. + Total cost is notifications_sent * rate. + Rate multiplier does not apply to email or letters. + """ + email_and_letters = db.session.query( + func.sum(FactBilling.notifications_sent).label("notifications_sent"), + func.sum(FactBilling.notifications_sent).label("billable_units"), + FactBilling.rate.label('rate'), + FactBilling.notification_type.label('notification_type') + ).filter( + FactBilling.service_id == service_id, + FactBilling.bst_date >= year_start_date, + FactBilling.bst_date <= year_end_date, + FactBilling.notification_type.in_([EMAIL_TYPE, LETTER_TYPE]) + ).group_by( + FactBilling.rate, + FactBilling.notification_type + ) + """ + Billing for SMS using the billing_units * rate_multiplier. Billing unit of SMS is the fragment count of a message + """ + sms = db.session.query( func.sum(FactBilling.notifications_sent).label("notifications_sent"), func.sum(FactBilling.billable_units * FactBilling.rate_multiplier).label("billable_units"), - FactBilling.service_id, FactBilling.rate, FactBilling.notification_type ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start_date, - FactBilling.bst_date <= year_end_date + FactBilling.bst_date <= year_end_date, + FactBilling.notification_type == SMS_TYPE ).group_by( - FactBilling.service_id, FactBilling.rate, FactBilling.notification_type - ).order_by( - FactBilling.service_id, - FactBilling.notification_type + ) + + yearly_data = email_and_letters.union_all(sms).order_by( + 'notification_type', + 'rate' ).all() return yearly_data @@ -58,26 +83,44 @@ def fetch_monthly_billing_for_year(service_id, year): for d in data: update_fact_billing(data=d, process_day=day) - yearly_data = db.session.query( + email_and_letters = db.session.query( + func.date_trunc('month', FactBilling.bst_date).cast(Date).label("month"), + func.sum(FactBilling.notifications_sent).label("notifications_sent"), + func.sum(FactBilling.notifications_sent).label("billable_units"), + FactBilling.rate.label('rate'), + FactBilling.notification_type.label('notification_type') + ).filter( + FactBilling.service_id == service_id, + FactBilling.bst_date >= year_start_date, + FactBilling.bst_date <= year_end_date, + FactBilling.notification_type.in_([EMAIL_TYPE, LETTER_TYPE]) + ).group_by( + 'month', + FactBilling.rate, + FactBilling.notification_type + ) + + sms = db.session.query( func.date_trunc('month', FactBilling.bst_date).cast(Date).label("month"), func.sum(FactBilling.notifications_sent).label("notifications_sent"), func.sum(FactBilling.billable_units * FactBilling.rate_multiplier).label("billable_units"), - FactBilling.service_id, FactBilling.rate, FactBilling.notification_type ).filter( FactBilling.service_id == service_id, FactBilling.bst_date >= year_start_date, - FactBilling.bst_date <= year_end_date + FactBilling.bst_date <= year_end_date, + FactBilling.notification_type == SMS_TYPE ).group_by( 'month', - FactBilling.service_id, FactBilling.rate, FactBilling.notification_type - ).order_by( - FactBilling.service_id, + ) + + yearly_data = email_and_letters.union_all(sms).order_by( 'month', - FactBilling.notification_type + 'notification_type', + 'rate' ).all() return yearly_data @@ -88,6 +131,7 @@ def fetch_billing_data_for_day(process_day, service_id=None): end_date = convert_bst_to_utc(datetime.combine(process_day + timedelta(days=1), time.min)) # use notification_history if process day is older than 7 days # this is useful if we need to rebuild the ft_billing table for a date older than 7 days ago. + current_app.logger.info("Populate ft_billing for {} to {}".format(start_date, end_date)) table = Notification if start_date < datetime.utcnow() - timedelta(days=7): table = NotificationHistory diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 715cd164b..502df8fa5 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -116,11 +116,11 @@ def get_monthly_billing_by_notification_type(service_id, billing_month, notifica @statsd(namespace="dao") def get_billing_data_for_financial_year(service_id, year, notification_types=[SMS_TYPE, EMAIL_TYPE, LETTER_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) + if start_date <= now <= end_date: + # Update totals to the latest so we include data for today + create_or_update_monthly_billing(service_id=service_id, billing_month=now) results = get_yearly_billing_data_for_date_range( service_id, start_date, end_date, notification_types diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 43b49dfa2..f931edbde 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -522,6 +522,7 @@ def set_up_yearly_data(): service=service, template=email_template, notification_type='email', + billable_unit=0, rate=0) create_ft_billing(bst_date='2016-{}-{}'.format(mon, d), service=service, @@ -559,7 +560,7 @@ def set_up_yearly_data(): return service -def test_get_yearly_billing_usage_summary_from_ft_billing_comapre_to_monthyl_billing( +def test_get_yearly_billing_usage_summary_from_ft_billing_compare_to_monthly_billing( client, notify_db_session ): service = set_up_yearly_data() @@ -621,3 +622,147 @@ def test_get_yearly_billing_usage_summary_from_ft_billing(client, notify_db_sess assert json_response[2]['billing_units'] == 825 assert json_response[2]['rate'] == 0.0162 assert json_response[2]['letter_total'] == 0 + + +def test_get_yearly_usage_by_monthly_from_ft_billing_all_cases(client, notify_db_session): + service = set_up_data_for_all_cases() + response = client.get('service/{}/billing/ft-monthly-usage?year=2018'.format(service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 200 + json_response = json.loads(response.get_data(as_text=True)) + assert len(json_response) == 5 + assert json_response[0]['month'] == 'May' + assert json_response[0]['notification_type'] == 'letter' + assert json_response[0]['rate'] == 0.33 + assert json_response[0]['billing_units'] == 1 + + assert json_response[1]['month'] == 'May' + assert json_response[1]['notification_type'] == 'letter' + assert json_response[1]['rate'] == 0.36 + assert json_response[1]['billing_units'] == 1 + + assert json_response[2]['month'] == 'May' + assert json_response[2]['notification_type'] == 'letter' + assert json_response[2]['rate'] == 0.39 + assert json_response[2]['billing_units'] == 1 + + assert json_response[3]['month'] == 'May' + assert json_response[3]['notification_type'] == 'sms' + assert json_response[3]['rate'] == 0.0150 + assert json_response[3]['billing_units'] == 4 + + assert json_response[4]['month'] == 'May' + assert json_response[4]['notification_type'] == 'sms' + assert json_response[4]['rate'] == 0.162 + assert json_response[4]['billing_units'] == 5 + + +def test_get_yearly_billing_usage_summary_from_ft_billing_all_cases(client, notify_db_session): + service = set_up_data_for_all_cases() + response = client.get('/service/{}/billing/ft-yearly-usage-summary?year=2018'.format(service.id), + headers=[create_authorization_header()]) + assert response.status_code == 200 + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response) == 6 + assert json_response[0]["notification_type"] == 'email' + assert json_response[0]["billing_units"] == 1 + assert json_response[0]["rate"] == 0 + assert json_response[0]["letter_total"] == 0 + + assert json_response[1]["notification_type"] == 'letter' + assert json_response[1]["billing_units"] == 1 + assert json_response[1]["rate"] == 0.33 + assert json_response[1]["letter_total"] == 0.33 + + assert json_response[2]["notification_type"] == 'letter' + assert json_response[2]["billing_units"] == 1 + assert json_response[2]["rate"] == 0.36 + assert json_response[2]["letter_total"] == 0.36 + + assert json_response[3]["notification_type"] == 'letter' + assert json_response[3]["billing_units"] == 1 + assert json_response[3]["rate"] == 0.39 + assert json_response[3]["letter_total"] == 0.39 + + assert json_response[4]["notification_type"] == 'sms' + assert json_response[4]["billing_units"] == 4 + assert json_response[4]["rate"] == 0.0150 + assert json_response[4]["letter_total"] == 0 + + assert json_response[5]["notification_type"] == 'sms' + assert json_response[5]["billing_units"] == 5 + assert json_response[5]["rate"] == 0.162 + assert json_response[5]["letter_total"] == 0 + + +def set_up_data_for_all_cases(): + service = create_service() + sms_template = create_template(service=service, template_type="sms") + email_template = create_template(service=service, template_type="email") + letter_template = create_template(service=service, template_type="letter") + create_ft_billing(bst_date='2018-05-16', + notification_type='sms', + template=sms_template, + service=service, + rate_multiplier=1, + international=False, + rate=0.162, + billable_unit=1, + notifications_sent=1) + create_ft_billing(bst_date='2018-05-17', + notification_type='sms', + template=sms_template, + service=service, + rate_multiplier=2, + international=False, + rate=0.162, + billable_unit=2, + notifications_sent=1) + create_ft_billing(bst_date='2018-05-16', + notification_type='sms', + template=sms_template, + service=service, + rate_multiplier=2, + international=False, + rate=0.0150, + billable_unit=2, + notifications_sent=1) + create_ft_billing(bst_date='2018-05-16', + notification_type='email', + template=email_template, + service=service, + rate_multiplier=1, + international=False, + rate=0, + billable_unit=0, + notifications_sent=1) + create_ft_billing(bst_date='2018-05-16', + notification_type='letter', + template=letter_template, + service=service, + rate_multiplier=1, + international=False, + rate=0.33, + billable_unit=1, + notifications_sent=1) + create_ft_billing(bst_date='2018-05-17', + notification_type='letter', + template=letter_template, + service=service, + rate_multiplier=1, + international=False, + rate=0.36, + billable_unit=2, + notifications_sent=1) + create_ft_billing(bst_date='2018-05-18', + notification_type='letter', + template=letter_template, + service=service, + rate_multiplier=1, + international=False, + rate=0.39, + billable_unit=3, + notifications_sent=1) + return service diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 09deb9266..7d64a43a0 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -266,14 +266,12 @@ def test_fetch_monthly_billing_for_year(notify_db_session): assert str(results[0].month) == "2018-06-01" assert results[0].notifications_sent == 30 assert results[0].billable_units == Decimal('60') - assert results[0].service_id == service.id assert results[0].rate == Decimal('0.162') assert results[0].notification_type == 'sms' assert str(results[1].month) == "2018-07-01" assert results[1].notifications_sent == 31 assert results[1].billable_units == Decimal('31') - assert results[1].service_id == service.id assert results[1].rate == Decimal('0.158') assert results[1].notification_type == 'sms' @@ -330,19 +328,16 @@ def test_fetch_billing_totals_for_year(notify_db_session): assert len(results) == 3 assert results[0].notification_type == 'email' - assert results[0].service_id == service.id assert results[0].notifications_sent == 365 assert results[0].billable_units == 365 assert results[0].rate == Decimal('0') assert results[1].notification_type == 'letter' - assert results[1].service_id == service.id assert results[1].notifications_sent == 365 assert results[1].billable_units == 365 assert results[1].rate == Decimal('0.33') assert results[2].notification_type == 'sms' - assert results[2].service_id == service.id assert results[2].notifications_sent == 365 assert results[2].billable_units == 365 assert results[2].rate == Decimal('0.162') diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index e09814708..e41057501 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -511,3 +511,13 @@ def test_get_yearly_billing_data_for_year_includes_current_day_totals(sample_tem ) assert billing_data[0].monthly_totals[0]['billing_units'] == 3 + + +@freeze_time("2017-06-16 13:00:00") +def test_get_billing_data_for_financial_year_updated_monthly_billing_if_today_is_in_current_year( + sample_service, + mocker +): + mock = mocker.patch("app.dao.monthly_billing_dao.create_or_update_monthly_billing") + get_billing_data_for_financial_year(sample_service.id, 2016) + mock.assert_not_called() From 3c90c0ffcd93a48001eee4196a85c09ae3c3e932 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 16 May 2018 13:18:36 +0100 Subject: [PATCH 29/41] Fix code style --- app/dao/fact_billing_dao.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 9a3a30148..7d5dfcdf1 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -27,8 +27,8 @@ def fetch_billing_totals_for_year(service_id, year): year_start_date, year_end_date = get_financial_year(year) """ Billing for email: only record the total number of emails. - Billing for letters: The billing units is used to fetch the correct rate for the sheet count of the letter. - Total cost is notifications_sent * rate. + Billing for letters: The billing units is used to fetch the correct rate for the sheet count of the letter. + Total cost is notifications_sent * rate. Rate multiplier does not apply to email or letters. """ email_and_letters = db.session.query( From 1969c83f573e727fe786f3c3005f96c51cd4e8d9 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 17 May 2018 10:06:58 +0100 Subject: [PATCH 30/41] Fix letter rate bug in migtion --- app/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/commands.py b/app/commands.py index 841da2eff..3db9de113 100644 --- a/app/commands.py +++ b/app/commands.py @@ -467,7 +467,7 @@ def migrate_data_to_ft_billing(start_date, end_date): coalesce((select rates.rate from rates where n.notification_type = rates.notification_type and n.sent_at > rates.valid_from order by rates.valid_from desc limit 1), 0) as sms_rate, - coalesce((select l.rate from letter_rates l where n.rate_multiplier = l.sheet_count + coalesce((select l.rate from letter_rates l where n.billable_units = l.sheet_count and s.crown = l.crown and n.notification_type='letter'), 0) as letter_rate, coalesce(n.international, false) as international, n.billable_units, From 4b36fe0d9ec412c51ee20c318e00ab0f97707b1f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 17 May 2018 13:37:55 +0100 Subject: [PATCH 31/41] Use created_at for the rate, there are some rows that do not have a sent_at set. --- app/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/commands.py b/app/commands.py index 3db9de113..804f4c9b0 100644 --- a/app/commands.py +++ b/app/commands.py @@ -465,7 +465,7 @@ def migrate_data_to_ft_billing(start_date, end_date): coalesce(n.rate_multiplier,1) as rate_multiplier, s.crown, coalesce((select rates.rate from rates - where n.notification_type = rates.notification_type and n.sent_at > rates.valid_from + where n.notification_type = rates.notification_type and n.created_at > rates.valid_from order by rates.valid_from desc limit 1), 0) as sms_rate, coalesce((select l.rate from letter_rates l where n.billable_units = l.sheet_count and s.crown = l.crown and n.notification_type='letter'), 0) as letter_rate, From 64aec4a64cfe93ae1d375c6d8461817c133e7ce7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 18 May 2018 17:22:51 +0100 Subject: [PATCH 32/41] Update the command to migrate data for ft_billing. There is a massive performance improvement. Another commit will need to alter the pk to include rate. --- app/commands.py | 46 +++++----------------------------------------- 1 file changed, 5 insertions(+), 41 deletions(-) diff --git a/app/commands.py b/app/commands.py index 804f4c9b0..4ab636245 100644 --- a/app/commands.py +++ b/app/commands.py @@ -401,43 +401,13 @@ def setup_commands(application): @statsd(namespace="tasks") def migrate_data_to_ft_billing(start_date, end_date): - print('Billing migration from date {} to {}'.format(start_date, end_date)) + current_app.logger.info('Billing migration from date {} to {}'.format(start_date, end_date)) process_date = start_date total_updated = 0 while process_date < end_date: - - sql = \ - """ - select count(*) from notification_history where notification_status!='technical-failure' - and key_type!='test' - and notification_status!='created' - and created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' - and created_at < (date :end + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' - """ - num_notifications = db.session.execute(sql, {"start": process_date, - "end": process_date + timedelta(days=1)}).fetchall()[0][0] - sql = \ - """ - select count(*) from - (select distinct service_id, template_id, rate_multiplier, - sent_by from notification_history - where notification_status!='technical-failure' - and key_type!='test' - and notification_status!='created' - and created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' - and created_at < (date :end + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' - ) as distinct_records - """ - - predicted_records = db.session.execute(sql, {"start": process_date, - "end": process_date + timedelta(days=1)}).fetchall()[0][0] - - start_time = datetime.now() - print('ft_billing: Migrating date: {}, notifications: {}, expecting {} ft_billing rows' - .format(process_date.date(), num_notifications, predicted_records)) - + start_time = datetime.utcnow() # migrate data into ft_billing, ignore if records already exist - do not do upsert sql = \ """ @@ -449,7 +419,7 @@ def migrate_data_to_ft_billing(start_date, end_date): from ( select n.id, - da.bst_date, + (n.created_at at time zone 'UTC' at time zone 'Europe/London')::timestamp::date as bst_date, coalesce(n.template_id, '00000000-0000-0000-0000-000000000000') as template_id, coalesce(n.service_id, '00000000-0000-0000-0000-000000000000') as service_id, n.notification_type, @@ -473,9 +443,6 @@ def migrate_data_to_ft_billing(start_date, end_date): n.billable_units, 1 as notifications_sent from public.notification_history n - left join templates t on t.id = n.template_id - left join dm_datetime da on n.created_at>= da.utc_daytime_start - and n.created_at < da.utc_daytime_end left join services s on s.id = n.service_id where n.notification_status!='technical-failure' and n.key_type!='test' @@ -495,16 +462,13 @@ def migrate_data_to_ft_billing(start_date, end_date): result = db.session.execute(sql, {"start": process_date, "end": process_date + timedelta(days=1)}) db.session.commit() - print('ft_billing: --- Completed took {}ms. Migrated {} rows.'.format(datetime.now() - start_time, + current_app.logger.info('ft_billing: --- Completed took {}ms. Migrated {} rows.'.format(datetime.now() - start_time, result.rowcount)) - if predicted_records != result.rowcount: - print(' : ^^^ Result mismatch by {} rows ^^^' - .format(predicted_records - result.rowcount)) process_date += timedelta(days=1) total_updated += result.rowcount - print('Total inserted/updated records = {}'.format(total_updated)) + current_app.logger.info('Total inserted/updated records = {}'.format(total_updated)) @notify_command() From 84c3f53902ee0807d54363c3de85047d278e813b Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sun, 20 May 2018 19:11:35 +0100 Subject: [PATCH 33/41] Update marshmallow from 2.15.1 to 2.15.3 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 4e487d5e1..dc30b71b0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ gunicorn==19.7.1 iso8601==0.1.12 jsonschema==2.6.0 marshmallow-sqlalchemy==0.13.2 -marshmallow==2.15.1 +marshmallow==2.15.3 monotonic==1.4 psycopg2-binary==2.7.4 PyJWT==1.6.1 From 9fad623d91859c4bcc578c7ef6e2b2ba27ea8952 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 21 May 2018 10:56:16 +0100 Subject: [PATCH 34/41] Update the ft_billing query to only include billable notification status types. Update test_provider_statistics dao - this is really irrelevant since the endpoint using the query is not being used. We have a PR coming to delete the unused code. Update rate_multiplier to always be an integer --- app/commands.py | 6 ++++-- app/dao/fact_billing_dao.py | 12 +++++------- app/models.py | 1 - tests/app/dao/test_provider_statistics_dao.py | 11 +++++++---- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/commands.py b/app/commands.py index 4ab636245..ad425139b 100644 --- a/app/commands.py +++ b/app/commands.py @@ -446,6 +446,8 @@ def migrate_data_to_ft_billing(start_date, end_date): left join services s on s.id = n.service_id where n.notification_status!='technical-failure' and n.key_type!='test' + and n.notification_status in + ('sending', 'sent', 'delivered', 'temporary-failure', 'permanent-failure', 'failed') and n.notification_status!='created' and n.created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' @@ -462,8 +464,8 @@ def migrate_data_to_ft_billing(start_date, end_date): result = db.session.execute(sql, {"start": process_date, "end": process_date + timedelta(days=1)}) db.session.commit() - current_app.logger.info('ft_billing: --- Completed took {}ms. Migrated {} rows.'.format(datetime.now() - start_time, - result.rowcount)) + current_app.logger.info('ft_billing: --- Completed took {}ms. Migrated {} rows for {}'.format( + datetime.now() - start_time, result.rowcount, process_date)) process_date += timedelta(days=1) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 18387a25e..f527c05a7 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -1,7 +1,7 @@ from datetime import datetime, timedelta, time from sqlalchemy.dialects.postgresql import insert -from sqlalchemy import func, case, desc, Date +from sqlalchemy import func, case, desc, Date, Integer from app import db from app.dao.date_util import get_financial_year @@ -9,14 +9,13 @@ from app.models import ( FactBilling, Notification, Service, - NOTIFICATION_CREATED, - NOTIFICATION_TECHNICAL_FAILURE, KEY_TYPE_TEST, LETTER_TYPE, SMS_TYPE, Rate, LetterRate, - NotificationHistory + NotificationHistory, + NOTIFICATION_STATUS_TYPES_BILLABLE ) from app.utils import convert_utc_to_bst, convert_bst_to_utc @@ -104,14 +103,13 @@ def fetch_billing_data_for_day(process_day, service_id=None): (table.notification_type == 'email', 'ses') ]), ).label('sent_by'), - func.coalesce(table.rate_multiplier, 1).label('rate_multiplier'), + func.coalesce(table.rate_multiplier, 1).cast(Integer).label('rate_multiplier'), func.coalesce(table.international, False).label('international'), func.sum(table.billable_units).label('billable_units'), func.count().label('notifications_sent'), Service.crown, ).filter( - table.status != NOTIFICATION_CREATED, # at created status, provider information is not available - table.status != NOTIFICATION_TECHNICAL_FAILURE, + table.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE), table.key_type != KEY_TYPE_TEST, table.created_at >= start_date, table.created_at < end_date diff --git a/app/models.py b/app/models.py index 925c72265..2984edeab 100644 --- a/app/models.py +++ b/app/models.py @@ -1068,7 +1068,6 @@ NOTIFICATION_STATUS_TYPES_BILLABLE = [ NOTIFICATION_SENT, NOTIFICATION_DELIVERED, NOTIFICATION_FAILED, - NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, ] diff --git a/tests/app/dao/test_provider_statistics_dao.py b/tests/app/dao/test_provider_statistics_dao.py index f23550b6b..c34395c76 100644 --- a/tests/app/dao/test_provider_statistics_dao.py +++ b/tests/app/dao/test_provider_statistics_dao.py @@ -4,7 +4,10 @@ import uuid import pytest from freezegun import freeze_time -from app.models import NotificationHistory, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, NOTIFICATION_STATUS_TYPES +from app.models import ( + NotificationHistory, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, + NOTIFICATION_STATUS_TYPES_BILLABLE +) from app.dao.provider_statistics_dao import get_fragment_count @@ -24,10 +27,10 @@ def test_get_fragment_count_separates_sms_and_email(notify_db, sample_template, def test_get_fragment_count_filters_on_status(notify_db, sample_template): - for status in NOTIFICATION_STATUS_TYPES: + for status in NOTIFICATION_STATUS_TYPES_BILLABLE: noti_hist(notify_db, sample_template, status=status) - # sending, sent, delivered, failed, technical-failure, temporary-failure, permanent-failure - assert get_fragment_count(sample_template.service_id)['sms_count'] == 7 + # sending, sent, delivered, failed, temporary-failure, permanent-failure + assert get_fragment_count(sample_template.service_id)['sms_count'] == 6 def test_get_fragment_count_filters_on_service_id(notify_db, sample_template, service_factory): From c2b888a6aff07c8a40e7aef5cc92fa947b5b6586 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 21 May 2018 11:51:06 +0100 Subject: [PATCH 35/41] Make sure the date does not include time --- app/dao/fact_billing_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index a508d3bfc..0f284a2e5 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -239,7 +239,7 @@ def update_fact_billing(data, process_day): def create_billing_record(data, rate, process_day): billing_record = FactBilling( - bst_date=process_day, + bst_date=process_day.date(), template_id=data.template_id, service_id=data.service_id, notification_type=data.notification_type, From e4e253ce4446bd1dfe8171dfaee30ae6d66c2c1a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 21 May 2018 11:54:49 +0100 Subject: [PATCH 36/41] Remove where clause for notification_status --- app/commands.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/commands.py b/app/commands.py index ad425139b..d55f8a1b5 100644 --- a/app/commands.py +++ b/app/commands.py @@ -444,11 +444,9 @@ def migrate_data_to_ft_billing(start_date, end_date): 1 as notifications_sent from public.notification_history n left join services s on s.id = n.service_id - where n.notification_status!='technical-failure' - and n.key_type!='test' + where n.key_type!='test' and n.notification_status in ('sending', 'sent', 'delivered', 'temporary-failure', 'permanent-failure', 'failed') - and n.notification_status!='created' and n.created_at >= (date :start + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' and n.created_at < (date :end + time '00:00:00') at time zone 'Europe/London' at time zone 'UTC' From e5584348ef0c8233fd2530eca1c2d0c2ad43fd5e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 21 May 2018 14:38:25 +0100 Subject: [PATCH 37/41] Add rate to the primary key of ft_billing. Use the primary key constraint in the upsert statement (easier to read than listing all the columns) --- app/dao/fact_billing_dao.py | 8 +----- migrations/versions/0191_ft_billing_pkey.py | 29 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 migrations/versions/0191_ft_billing_pkey.py diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 0f284a2e5..b460153f5 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -222,13 +222,7 @@ def update_fact_billing(data, process_day): ) stmt = stmt.on_conflict_do_update( - index_elements=[table.c.bst_date, - table.c.template_id, - table.c.service_id, - table.c.provider, - table.c.rate_multiplier, - table.c.notification_type, - table.c.international], + constraint="ft_billing_pkey", set_={"notifications_sent": stmt.excluded.notifications_sent, "billable_units": stmt.excluded.billable_units } diff --git a/migrations/versions/0191_ft_billing_pkey.py b/migrations/versions/0191_ft_billing_pkey.py new file mode 100644 index 000000000..80093083c --- /dev/null +++ b/migrations/versions/0191_ft_billing_pkey.py @@ -0,0 +1,29 @@ +""" + +Revision ID: 0191_ft_billing_pkey +Revises: 0190_another_letter_org +Create Date: 2018-05-21 14:24:27.229511 + +""" +from alembic import op + +revision = '0191_ft_billing_pkey' +down_revision = '0190_another_letter_org' + + +def upgrade(): + op.get_bind() + op.execute("ALTER TABLE ft_billing DROP CONSTRAINT ft_billing_pkey") + sql = """ALTER TABLE ft_billing ADD CONSTRAINT + ft_billing_pkey PRIMARY KEY + (bst_date, template_id, service_id, rate_multiplier, provider, notification_type, international, rate)""" + op.execute(sql) + + +def downgrade(): + op.get_bind() + op.execute("ALTER TABLE ft_billing DROP CONSTRAINT ft_billing_pkey") + sql = """ALTER TABLE ft_billing ADD CONSTRAINT + ft_billing_pkey PRIMARY KEY + (bst_date, template_id, service_id, rate_multiplier, provider, notification_type, international)""" + op.execute(sql) \ No newline at end of file From 71796311fa93406431681d181d256256205ba95c Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 21 May 2018 11:23:54 +0100 Subject: [PATCH 38/41] Delete unused aggregate_statistics endpoint `@service_blueprint.route('//fragment/aggregate_statistics')` is not being used anywhere, so has been removed. The `provider_statistics_dao` can also be removed, since the function this contained was only used for the endpoint. --- app/dao/provider_statistics_dao.py | 42 --------- app/service/rest.py | 12 --- tests/app/dao/test_provider_statistics_dao.py | 90 ------------------- tests/app/service/test_rest.py | 20 ----- 4 files changed, 164 deletions(-) delete mode 100644 app/dao/provider_statistics_dao.py delete mode 100644 tests/app/dao/test_provider_statistics_dao.py diff --git a/app/dao/provider_statistics_dao.py b/app/dao/provider_statistics_dao.py deleted file mode 100644 index 705d08fec..000000000 --- a/app/dao/provider_statistics_dao.py +++ /dev/null @@ -1,42 +0,0 @@ -from sqlalchemy import func - -from app import db -from app.dao.date_util import get_financial_year -from app.models import ( - NotificationHistory, - SMS_TYPE, - EMAIL_TYPE, - NOTIFICATION_STATUS_TYPES_BILLABLE, - KEY_TYPE_TEST -) - - -def get_fragment_count(service_id, year=None): - shared_filters = [ - NotificationHistory.service_id == service_id, - NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE), - NotificationHistory.key_type != KEY_TYPE_TEST - ] - - if year: - shared_filters.append(NotificationHistory.created_at.between( - *get_financial_year(year) - )) - - sms_count = db.session.query( - func.sum(NotificationHistory.billable_units) - ).filter( - NotificationHistory.notification_type == SMS_TYPE, - *shared_filters - ) - - email_count = db.session.query( - func.count(NotificationHistory.id) - ).filter( - NotificationHistory.notification_type == EMAIL_TYPE, - *shared_filters - ) - return { - 'sms_count': int(sms_count.scalar() or 0), - 'email_count': email_count.scalar() or 0 - } diff --git a/app/service/rest.py b/app/service/rest.py index 136ddc99e..2f0d516f4 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -65,7 +65,6 @@ from app.dao.service_letter_contact_dao import ( add_letter_contact_for_service, update_letter_contact ) -from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id from app.errors import ( InvalidRequest, @@ -299,17 +298,6 @@ def remove_user_from_service(service_id, user_id): return jsonify({}), 204 -@service_blueprint.route('//fragment/aggregate_statistics') -def get_service_provider_aggregate_statistics(service_id): - year = request.args.get('year') - if year is not None: - try: - year = int(year) - except ValueError: - raise InvalidRequest('Year must be a number', status_code=400) - return jsonify(data=get_fragment_count(service_id, year=year)) - - # This is placeholder get method until more thought # goes into how we want to fetch and view various items in history # tables. This is so product owner can pass stories as done diff --git a/tests/app/dao/test_provider_statistics_dao.py b/tests/app/dao/test_provider_statistics_dao.py deleted file mode 100644 index c34395c76..000000000 --- a/tests/app/dao/test_provider_statistics_dao.py +++ /dev/null @@ -1,90 +0,0 @@ -from datetime import datetime -import uuid - -import pytest -from freezegun import freeze_time - -from app.models import ( - NotificationHistory, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - NOTIFICATION_STATUS_TYPES_BILLABLE -) -from app.dao.provider_statistics_dao import get_fragment_count - - -def test_get_fragment_count_with_no_data(sample_template): - assert get_fragment_count(sample_template.service_id)['sms_count'] == 0 - assert get_fragment_count(sample_template.service_id)['email_count'] == 0 - - -def test_get_fragment_count_separates_sms_and_email(notify_db, sample_template, sample_email_template): - noti_hist(notify_db, sample_template) - noti_hist(notify_db, sample_template) - noti_hist(notify_db, sample_email_template) - assert get_fragment_count(sample_template.service_id) == { - 'sms_count': 2, - 'email_count': 1 - } - - -def test_get_fragment_count_filters_on_status(notify_db, sample_template): - for status in NOTIFICATION_STATUS_TYPES_BILLABLE: - noti_hist(notify_db, sample_template, status=status) - # sending, sent, delivered, failed, temporary-failure, permanent-failure - assert get_fragment_count(sample_template.service_id)['sms_count'] == 6 - - -def test_get_fragment_count_filters_on_service_id(notify_db, sample_template, service_factory): - service_2 = service_factory.get('service 2', email_from='service.2') - noti_hist(notify_db, sample_template) - assert get_fragment_count(service_2.id)['sms_count'] == 0 - - -@pytest.mark.parametrize('creation_time, expected_count', [ - ('2000-03-31 22:59:59', 0), # before the start of the year - ('2000-04-01 00:00:00', 1), # after the start of the year - ('2001-03-31 22:59:59', 1), # before the end of the year - ('2001-04-01 00:00:00', 0), # after the end of the year -]) -def test_get_fragment_count_filters_on_year( - notify_db, sample_template, creation_time, expected_count -): - with freeze_time(creation_time): - noti_hist(notify_db, sample_template) - assert get_fragment_count(sample_template.service_id, year=2000)['sms_count'] == expected_count - - -def test_get_fragment_count_sums_billable_units_for_sms(notify_db, sample_template): - noti_hist(notify_db, sample_template, billable_units=1) - noti_hist(notify_db, sample_template, billable_units=2) - assert get_fragment_count(sample_template.service_id)['sms_count'] == 3 - - -@pytest.mark.parametrize('key_type,sms_count', [ - (KEY_TYPE_NORMAL, 1), - (KEY_TYPE_TEAM, 1), - (KEY_TYPE_TEST, 0), -]) -def test_get_fragment_count_ignores_test_api_keys(notify_db, sample_template, key_type, sms_count): - noti_hist(notify_db, sample_template, key_type=key_type) - assert get_fragment_count(sample_template.service_id)['sms_count'] == sms_count - - -def noti_hist(notify_db, template, status='delivered', billable_units=None, key_type=KEY_TYPE_NORMAL): - if not billable_units and template.template_type == 'sms': - billable_units = 1 - - notification_history = NotificationHistory( - id=uuid.uuid4(), - service=template.service, - template_id=template.id, - template_version=template.version, - status=status, - created_at=datetime.utcnow(), - billable_units=billable_units, - notification_type=template.template_type, - key_type=key_type - ) - notify_db.session.add(notification_history) - notify_db.session.commit() - - return notification_history diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1f19922ad..23c273727 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1678,26 +1678,6 @@ def test_get_detailed_services_for_date_range(notify_db, notify_db_session, set_ } -@pytest.mark.parametrize('query_string, expected_status, expected_json', [ - ('', 200, {'data': {'email_count': 0, 'sms_count': 0}}), - ('?year=2000', 200, {'data': {'email_count': 0, 'sms_count': 0}}), - ('?year=abcd', 400, {'message': 'Year must be a number', 'result': 'error'}), -]) -def test_get_service_provider_aggregate_statistics( - client, - sample_service, - query_string, - expected_status, - expected_json, -): - response = client.get( - '/service/{}/fragment/aggregate_statistics{}'.format(sample_service.id, query_string), - headers=[create_authorization_header()] - ) - assert response.status_code == expected_status - assert json.loads(response.get_data(as_text=True)) == expected_json - - @freeze_time('2017-11-11 02:00') def test_get_template_usage_by_month_returns_correct_data( notify_db, From 0d4fb81235881909d59a14fd40a64bbc2b9ae6b3 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 21 May 2018 12:12:53 +0100 Subject: [PATCH 39/41] Delete ProviderStatistics model This is not being used anywhere, so can be deleted safely. --- app/dao/services_dao.py | 2 -- app/models.py | 14 -------------- tests/app/conftest.py | 25 +------------------------ tests/app/dao/test_services_dao.py | 5 +---- 4 files changed, 2 insertions(+), 44 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5f326b372..68f0ea781 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -23,7 +23,6 @@ from app.models import ( Notification, NotificationHistory, Permission, - ProviderStatistics, Service, ServicePermission, ServiceSmsSender, @@ -223,7 +222,6 @@ def delete_service_and_all_associated_db_objects(service): _delete_commit(TemplateRedacted.query.filter(TemplateRedacted.template_id.in_(subq))) _delete_commit(ServiceSmsSender.query.filter_by(service=service)) - _delete_commit(ProviderStatistics.query.filter_by(service=service)) _delete_commit(InvitedUser.query.filter_by(service=service)) _delete_commit(Permission.query.filter_by(service=service)) _delete_commit(NotificationHistory.query.filter_by(service=service)) diff --git a/app/models.py b/app/models.py index 2984edeab..d55fff3ca 100644 --- a/app/models.py +++ b/app/models.py @@ -864,20 +864,6 @@ NOTIFICATION_TYPE = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE] notification_types = db.Enum(*NOTIFICATION_TYPE, name='notification_type') -class ProviderStatistics(db.Model): - __tablename__ = 'provider_statistics' - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - day = db.Column(db.Date, nullable=False) - provider_id = db.Column(UUID(as_uuid=True), db.ForeignKey('provider_details.id'), index=True, nullable=False) - provider = db.relationship( - 'ProviderDetails', backref=db.backref('provider_stats', lazy='dynamic') - ) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service', backref=db.backref('service_provider_stats', lazy='dynamic')) - unit_count = db.Column(db.BigInteger, nullable=False) - - class ProviderRates(db.Model): __tablename__ = 'provider_rates' diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 9dad50c18..ed6d432c8 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,4 +1,4 @@ -from datetime import (datetime, date, timedelta) +from datetime import datetime, timedelta import json import uuid @@ -22,7 +22,6 @@ from app.models import ( NotificationHistory, InvitedUser, Permission, - ProviderStatistics, ProviderDetails, ProviderDetailsHistory, ProviderRates, @@ -839,28 +838,6 @@ def mmg_provider(): return ProviderDetails.query.filter_by(identifier='mmg').one() -@pytest.fixture(scope='function') -def sample_provider_statistics(notify_db, - notify_db_session, - sample_service, - provider=None, - day=None, - unit_count=1): - - if provider is None: - provider = ProviderDetails.query.filter_by(identifier='mmg').first() - if day is None: - day = date.today() - stats = ProviderStatistics( - service=sample_service, - provider_id=provider.id, - day=day, - unit_count=unit_count) - notify_db.session.add(stats) - notify_db.session.commit() - return stats - - @pytest.fixture(scope='function') def mock_firetext_client(mocker, statsd_client=None): client = FiretextClient() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index b0c5a0ccb..997920fd5 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -37,7 +37,6 @@ from app.dao.services_dao import ( from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( - ProviderStatistics, VerifyCode, ApiKey, Template, @@ -460,14 +459,12 @@ def test_delete_service_and_associated_objects(notify_db, sample_job, sample_notification, sample_invited_user, - sample_permission, - sample_provider_statistics): + sample_permission): assert ServicePermission.query.count() == len(( SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE )) delete_service_and_all_associated_db_objects(sample_service) - assert ProviderStatistics.query.count() == 0 assert VerifyCode.query.count() == 0 assert ApiKey.query.count() == 0 assert ApiKey.get_history_model().query.count() == 0 From 3d8ce399841f3a56cd78a6bd51ef2360965d09c5 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 21 May 2018 13:24:45 +0100 Subject: [PATCH 40/41] Migration to drop unused provider_statistics table --- .../versions/0192_drop_provider_statistics.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 migrations/versions/0192_drop_provider_statistics.py diff --git a/migrations/versions/0192_drop_provider_statistics.py b/migrations/versions/0192_drop_provider_statistics.py new file mode 100644 index 000000000..28753ca33 --- /dev/null +++ b/migrations/versions/0192_drop_provider_statistics.py @@ -0,0 +1,34 @@ +""" + +Revision ID: 0192_drop_provider_statistics +Revises: 0191_ft_billing_pkey +Create Date: 2018-05-21 15:18:43.871256 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0192_drop_provider_statistics' +down_revision = '0191_ft_billing_pkey' + + +def upgrade(): + op.drop_index('ix_provider_statistics_provider_id', table_name='provider_statistics') + op.drop_index('ix_provider_statistics_service_id', table_name='provider_statistics') + op.drop_table('provider_statistics') + + +def downgrade(): + op.create_table('provider_statistics', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('day', sa.DATE(), autoincrement=False, nullable=False), + sa.Column('service_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('unit_count', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('provider_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['provider_id'], ['provider_details.id'], name='provider_stats_to_provider_fk'), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], name='provider_statistics_service_id_fkey'), + sa.PrimaryKeyConstraint('id', name='provider_statistics_pkey') + ) + op.create_index('ix_provider_statistics_service_id', 'provider_statistics', ['service_id'], unique=False) + op.create_index('ix_provider_statistics_provider_id', 'provider_statistics', ['provider_id'], unique=False) From f6039d51e0bc0f018c4c465f4c659128e2353089 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 22 May 2018 14:19:51 +0100 Subject: [PATCH 41/41] Add timestamp columns to ft_billing table Added `created_at` and `updated_at` to the `ft_billing` table - having these columns makes it easier to track down issues with the data in this table. `created_at` is nullable initially, but will be changed to non-nullable once the column is populated and the DAO etc. have been updated. --- app/models.py | 2 ++ .../0193_add_ft_billing_timestamps.py | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 migrations/versions/0193_add_ft_billing_timestamps.py diff --git a/app/models.py b/app/models.py index d55fff3ca..1ed954c09 100644 --- a/app/models.py +++ b/app/models.py @@ -1774,6 +1774,8 @@ class FactBilling(db.Model): rate = db.Column(db.Numeric(), nullable=True) billable_units = db.Column(db.Integer(), nullable=True) notifications_sent = db.Column(db.Integer(), nullable=True) + created_at = db.Column(db.DateTime, nullable=True, default=datetime.datetime.utcnow) + updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) class DateTimeDimension(db.Model): diff --git a/migrations/versions/0193_add_ft_billing_timestamps.py b/migrations/versions/0193_add_ft_billing_timestamps.py new file mode 100644 index 000000000..f54586a79 --- /dev/null +++ b/migrations/versions/0193_add_ft_billing_timestamps.py @@ -0,0 +1,23 @@ +""" + +Revision ID: 0193_add_ft_billing_timestamps +Revises: 0192_drop_provider_statistics +Create Date: 2018-05-22 10:23:21.937262 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0193_add_ft_billing_timestamps' +down_revision = '0192_drop_provider_statistics' + + +def upgrade(): + op.add_column('ft_billing', sa.Column('updated_at', sa.DateTime(), nullable=True)) + op.add_column('ft_billing', sa.Column('created_at', sa.DateTime(), nullable=True)) + + +def downgrade(): + op.drop_column('ft_billing', 'created_at') + op.drop_column('ft_billing', 'updated_at')