From 49533d77925580fa13510646d29740d630f3a9dc Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 17 Feb 2020 11:12:54 +0000 Subject: [PATCH 1/7] Fix typo in function name --- app/dao/fact_billing_dao.py | 1 + app/dao/organisation_dao.py | 2 +- app/status/healthcheck.py | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 93dca2d61..3878b1f45 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -186,6 +186,7 @@ def fetch_letter_line_items_for_all_services(start_date, end_date): @statsd(namespace="dao") def fetch_billing_totals_for_year(service_id, year): + print("fetch_billing_totals_for_year", year) year_start_date, year_end_date = get_financial_year(year) """ Billing for email: only record the total number of emails. diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index f9757664f..8dc4e3bc0 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -17,7 +17,7 @@ def dao_get_organisations(): ).all() -def dao_count_organsations_with_live_services(): +def dao_count_organisations_with_live_services(): return db.session.query(Organisation.id).join(Organisation.services).filter( Service.active.is_(True), Service.restricted.is_(False), diff --git a/app/status/healthcheck.py b/app/status/healthcheck.py index 4b248ae26..49f80980e 100644 --- a/app/status/healthcheck.py +++ b/app/status/healthcheck.py @@ -6,7 +6,7 @@ from flask import ( from app import db, version from app.dao.services_dao import dao_count_live_services -from app.dao.organisation_dao import dao_count_organsations_with_live_services +from app.dao.organisation_dao import dao_count_organisations_with_live_services status = Blueprint('status', __name__) @@ -28,7 +28,7 @@ def show_status(): @status.route('/_status/live-service-and-organisation-counts') def live_service_and_organisation_counts(): return jsonify( - organisations=dao_count_organsations_with_live_services(), + organisations=dao_count_organisations_with_live_services(), services=dao_count_live_services(), ), 200 From 18f272dc2b205fda55e79e6f2fb4c843182c9dcb Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 17 Feb 2020 16:34:17 +0000 Subject: [PATCH 2/7] Add queries to handle returning usage for all services associated to a given organisation. --- app/dao/fact_billing_dao.py | 1 + app/organisation/rest.py | 7 +++++++ tests/app/dao/test_ft_billing_dao.py | 21 +++++++++++++++++---- tests/app/db.py | 9 ++++++++- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 3878b1f45..10b980c81 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -12,6 +12,7 @@ from app.dao.date_util import ( get_financial_year, get_financial_year_for_datetime ) +from app.dao.organisation_dao import dao_get_organisation_services from app.models import ( FactBilling, diff --git a/app/organisation/rest.py b/app/organisation/rest.py index cbc405025..913df584a 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -125,6 +125,13 @@ def get_organisation_services(organisation_id): return jsonify([s.serialize_for_org_dashboard() for s in sorted_services]) +@organisation_blueprint.route('//services-with-usage', methods=['GET']) +def get_organisation_services_usage(organisation_id): + services = dao_get_organisation_services(organisation_id) + sorted_services = sorted(services, key=lambda s: (-s.active, s.name)) + return jsonify([s.serialize_for_org_dashboard() for s in sorted_services]) + + @organisation_blueprint.route('//users/', methods=['POST']) def add_user_to_organisation(organisation_id, user_id): new_org_user = dao_add_user_to_organisation(organisation_id, user_id) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 6d07de2a1..548ad5da4 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -18,7 +18,8 @@ from app.dao.fact_billing_dao import ( get_rates_for_billing, fetch_sms_free_allowance_remainder, fetch_sms_billing_for_all_services, - fetch_letter_costs_for_all_services, fetch_letter_line_items_for_all_services) + fetch_letter_costs_for_all_services, fetch_letter_line_items_for_all_services, fetch_usage_year_for_organisation +) from app.dao.organisation_dao import dao_add_service_to_organisation from app.models import ( FactBilling, @@ -592,7 +593,8 @@ def test_fetch_sms_billing_for_all_services_with_remainder(notify_db_session): def test_fetch_sms_billing_for_all_services_without_an_organisation_appears(notify_db_session): - org, org_2, service, service_2, service_3, service_sms_only = set_up_usage_data(datetime(2019, 5, 1)) + org, org_2, service, service_2, service_3, service_sms_only, \ + org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) results = fetch_sms_billing_for_all_services(datetime(2019, 5, 1), datetime(2019, 5, 31)) assert len(results) == 2 @@ -604,7 +606,8 @@ def test_fetch_sms_billing_for_all_services_without_an_organisation_appears(noti def test_fetch_letter_costs_for_all_services(notify_db_session): - org, org_2, service, service_2, service_3, service_sms_only = set_up_usage_data(datetime(2019, 6, 1)) + org, org_2, service, service_2, service_3, service_sms_only, \ + org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 6, 1)) results = fetch_letter_costs_for_all_services(datetime(2019, 6, 1), datetime(2019, 9, 30)) @@ -615,7 +618,8 @@ def test_fetch_letter_costs_for_all_services(notify_db_session): def test_fetch_letter_line_items_for_all_service(notify_db_session): - org_1, org_2, service_1, service_2, service_3, service_sms_only = set_up_usage_data(datetime(2019, 6, 1)) + org_1, org_2, service_1, service_2, service_3, service_sms_only, \ + org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 6, 1)) results = fetch_letter_line_items_for_all_services(datetime(2019, 6, 1), datetime(2019, 9, 30)) @@ -625,3 +629,12 @@ def test_fetch_letter_line_items_for_all_service(notify_db_session): assert results[2] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal("0.65"), 'second', 20) assert results[3] == (org_2.name, org_2.id, service_2.name, service_2.id, Decimal("0.50"), 'first', 2) assert results[4] == (None, None, service_3.name, service_3.id, Decimal("0.55"), 'second', 15) + + +def test_fetch_usage_year_for_organisation(notify_db_session): + org, org_2, service, service_2, service_3, service_sms_only, \ + org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) + + results = fetch_usage_year_for_organisation(org.id, 2019) + results = fetch_usage_year_for_organisation(org_2.id, 2019) + results = fetch_usage_year_for_organisation(org_with_emails.id, 2019) diff --git a/tests/app/db.py b/tests/app/db.py index 4b4d9688b..29e36c04b 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -906,6 +906,11 @@ def set_up_usage_data(start_date): org = create_organisation(name="Org for {}".format(service.name)) dao_add_service_to_organisation(service=service, organisation_id=org.id) + service_2 = create_service(service_name='b - emails') + email_template = create_template(service=service_2, template_type='email') + org_2 = create_organisation(name='Org for {}'.format(service_2)) + dao_add_service_to_organisation(service=service_2, organisation_id=org_2.id) + service_3 = create_service(service_name='c - letters only') letter_template_3 = create_template(service=service_3, template_type='letter') org_3 = create_organisation(name="Org for {}".format(service_3.name)) @@ -942,7 +947,9 @@ def set_up_usage_data(start_date): create_ft_billing(bst_date=two_days_later, template=letter_template_4, notifications_sent=15, billable_unit=4, rate=.55, postage='second') - return org, org_3, service, service_3, service_4, service_sms_only + create_ft_billing(bst_date=start_date, template=email_template, notifications_sent=10) + + return org, org_3, service, service_3, service_4, service_sms_only, org_2, service_2 def create_returned_letter(service=None, reported_at=None, notification_id=None): From b1b457eea0fecb45241167bc47b52a832ba54f56 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 24 Feb 2020 14:19:12 +0000 Subject: [PATCH 3/7] Only return the usage data for live services. The list of trail mode services is only for platform admins, therefore the usage isn't needed for that page. --- app/dao/fact_billing_dao.py | 154 ++++++++++++++++++++++++++- app/dao/organisation_dao.py | 7 ++ tests/app/dao/test_ft_billing_dao.py | 61 ++++++++++- 3 files changed, 217 insertions(+), 5 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 10b980c81..1706be8e3 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -12,7 +12,7 @@ from app.dao.date_util import ( get_financial_year, get_financial_year_for_datetime ) -from app.dao.organisation_dao import dao_get_organisation_services +from app.dao.organisation_dao import dao_get_organisation_live_services from app.models import ( FactBilling, @@ -187,7 +187,6 @@ def fetch_letter_line_items_for_all_services(start_date, end_date): @statsd(namespace="dao") def fetch_billing_totals_for_year(service_id, year): - print("fetch_billing_totals_for_year", year) year_start_date, year_end_date = get_financial_year(year) """ Billing for email: only record the total number of emails. @@ -541,3 +540,154 @@ def create_billing_record(data, rate, process_day): postage=data.postage, ) return billing_record + + +def fetch_letter_costs_for_organisation(organisation_id, start_date, end_date): + query = db.session.query( + Service.name.label("service_name"), + Service.id.label("service_id"), + func.sum(FactBilling.notifications_sent * FactBilling.rate).label("letter_cost") + ).select_from( + Service + ).join( + FactBilling, FactBilling.service_id == Service.id, + ).filter( + FactBilling.service_id == Service.id, + FactBilling.bst_date >= start_date, + FactBilling.bst_date <= end_date, + FactBilling.notification_type == LETTER_TYPE, + Service.organisation_id == organisation_id + ).group_by( + Service.id, + Service.name, + ).order_by( + Service.name + ) + + return query.all() + + +def fetch_email_usage_for_organisation(organisation_id, start_date, end_date): + query = db.session.query( + Service.name.label("service_name"), + Service.id.label("service_id"), + func.sum(FactBilling.notifications_sent).label("emails_sent") + ).select_from( + Service + ).join( + FactBilling, FactBilling.service_id == Service.id, + ).filter( + FactBilling.service_id == Service.id, + FactBilling.bst_date >= start_date, + FactBilling.bst_date <= end_date, + FactBilling.notification_type == EMAIL_TYPE, + Service.organisation_id == organisation_id + ).group_by( + Service.id, + Service.name, + ).order_by( + Service.name + ) + + return query.all() + + +@statsd(namespace="dao") +def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date): + + # ASSUMPTION: AnnualBilling has been populated for year. + free_allowance_remainder = fetch_sms_free_allowance_remainder(start_date).subquery() + + sms_billable_units = func.sum(FactBilling.billable_units * FactBilling.rate_multiplier) + sms_remainder = func.coalesce( + free_allowance_remainder.c.sms_remainder, + free_allowance_remainder.c.free_sms_fragment_limit + ) + chargeable_sms = func.greatest(sms_billable_units - sms_remainder, 0) + sms_cost = chargeable_sms * FactBilling.rate + + query = db.session.query( + Service.name.label("service_name"), + Service.id.label("service_id"), + free_allowance_remainder.c.free_sms_fragment_limit, + FactBilling.rate.label('sms_rate'), + sms_remainder.label("sms_remainder"), + sms_billable_units.label('sms_billable_units'), + chargeable_sms.label("chargeable_billable_sms"), + sms_cost.label('sms_cost'), + ).select_from( + Service + ).outerjoin( + free_allowance_remainder, Service.id == free_allowance_remainder.c.service_id + ).join( + FactBilling, FactBilling.service_id == Service.id, + ).filter( + FactBilling.bst_date >= start_date, + FactBilling.bst_date <= end_date, + FactBilling.notification_type == SMS_TYPE, + Service.organisation_id == organisation_id + ).group_by( + Service.id, + Service.name, + free_allowance_remainder.c.free_sms_fragment_limit, + free_allowance_remainder.c.sms_remainder, + FactBilling.rate, + ).order_by( + Service.name + ) + + return query.all() + + +def fetch_usage_year_for_organisation(organisation_id, year): + year_start_datetime, year_end_datetime = get_financial_year(year) + + year_start_date = convert_utc_to_bst(year_start_datetime).date() + year_end_date = convert_utc_to_bst(year_end_datetime).date() + + today = convert_utc_to_bst(datetime.utcnow()).date() + services = dao_get_organisation_live_services(organisation_id) + # 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 >= today: + yesterday = today - timedelta(days=1) + for service in services: + for day in [yesterday, today]: + data = fetch_billing_data_for_day(process_day=day, service_id=service.id) + for d in data: + update_fact_billing(data=d, process_day=day) + service_with_usage = {} + # initialise results + for service in services: + service_with_usage[str(service.id)] = { + 'service_id': service.id, + 'service_name': service.name, + 'free_sms_limit': 0, + 'sms_remainder': 0, + 'sms_billable_units': 0, + 'chargeable_billable_sms': 0, + 'sms_cost': 0, + 'letter_cost': 0, + 'emails_sent': 0 + } + sms_usages = fetch_sms_billing_for_organisation(organisation_id, year_start_date, year_end_date) + letter_usages = fetch_letter_costs_for_organisation(organisation_id, year_start_date, year_end_date) + email_usages = fetch_email_usage_for_organisation(organisation_id, year_start_date, year_end_date) + + for usage in sms_usages: + service_with_usage[str(usage.service_id)] = { + 'service_id': usage.service_id, + 'service_name': usage.service_name, + 'free_sms_limit': usage.free_sms_fragment_limit, + 'sms_remainder': usage.sms_remainder, + 'sms_billable_units': usage.sms_billable_units, + 'chargeable_billable_sms': usage.chargeable_billable_sms, + 'sms_cost': usage.sms_cost, + 'letter_cost': 0, + 'emails_sent': 0 + } + for letter_usage in letter_usages: + service_with_usage[str(letter_usage.service_id)]['letter_cost'] = letter_usage.letter_cost + for email_usage in email_usages: + service_with_usage[str(email_usage.service_id)]['emails_sent'] = email_usage.emails_sent + + return service_with_usage diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index 8dc4e3bc0..a71ae3603 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -31,6 +31,13 @@ def dao_get_organisation_services(organisation_id): ).one().services +def dao_get_organisation_live_services(organisation_id): + return Service.query.filter_by( + organisation_id=organisation_id, + restricted=False + ).all() + + def dao_get_organisation_by_id(organisation_id): return Organisation.query.filter_by(id=organisation_id).one() diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 548ad5da4..2bcb5053f 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -634,7 +634,62 @@ def test_fetch_letter_line_items_for_all_service(notify_db_session): def test_fetch_usage_year_for_organisation(notify_db_session): org, org_2, service, service_2, service_3, service_sms_only, \ org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) - + service_with_emails_for_org = create_service(service_name='Service with emails for org') + dao_add_service_to_organisation(service=service_with_emails_for_org, organisation_id=org.id) + template = create_template(service=service_with_emails_for_org, template_type='email') + create_ft_billing(bst_date=datetime(2019, 5, 1), + template=template, + notifications_sent=1100) results = fetch_usage_year_for_organisation(org.id, 2019) - results = fetch_usage_year_for_organisation(org_2.id, 2019) - results = fetch_usage_year_for_organisation(org_with_emails.id, 2019) + + assert len(results) == 2 + first_row = results[str(service.id)] + assert first_row['service_id'] == service.id + assert first_row['service_name'] == service.name + assert first_row['free_sms_limit'] == 10 + assert first_row['sms_remainder'] == 10 + assert first_row['chargeable_billable_sms'] == 0 + assert first_row['sms_cost'] == Decimal('0.0') + assert first_row['letter_cost'] == Decimal('3.40') + assert first_row['emails_sent'] == 0 + + second_row = results[str(service_with_emails_for_org.id)] + assert second_row['service_id'] == service_with_emails_for_org.id + assert second_row['service_name'] == service_with_emails_for_org.name + assert second_row['free_sms_limit'] == 0 + assert second_row['sms_remainder'] == 0 + assert second_row['chargeable_billable_sms'] == 0 + assert second_row['sms_cost'] == 0 + assert second_row['letter_cost'] == 0 + assert second_row['emails_sent'] == 1100 + + +def test_fetch_usage_year_for_organisation_populates_ft_billing_for_today(notify_db_session): + create_letter_rate(start_date=datetime.utcnow() - timedelta(days=1)) + create_rate(start_date=datetime.utcnow() - timedelta(days=1), value=0.65, notification_type='sms') + new_org = create_organisation(name='New organisation') + service = create_service() + template = create_template(service=service) + dao_add_service_to_organisation(service=service, organisation_id=new_org.id) + current_year = datetime.utcnow().year + create_annual_billing(service_id=service.id, free_sms_fragment_limit=10, financial_year_start=current_year) + + assert FactBilling.query.count() == 0 + + create_notification(template=template, status='delivered') + + results = fetch_usage_year_for_organisation(organisation_id=new_org.id, year=current_year) + assert len(results) == 1 + assert FactBilling.query.count() == 1 + + +def test_fetch_usage_year_for_organisation_only_returns_data_for_live_services(notify_db_session): + org = create_organisation(name='Organisation without live services') + service = create_service(restricted=True) + template = create_template(service=service) + dao_add_service_to_organisation(service=service, organisation_id=org.id) + create_ft_billing(bst_date=datetime.utcnow().date(), template=template, billable_unit=19, notifications_sent=19) + + results = fetch_usage_year_for_organisation(organisation_id=org.id, year=datetime.utcnow().year) + + assert len(results) == 0 From a2d18f85987909f607c5e84b70c83ae05b6faa89 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 24 Feb 2020 17:02:25 +0000 Subject: [PATCH 4/7] Update the organsition usage endpoint to use the new query. This endpoint may need to change, but we'd like to see how this performs, so we'll test this with a real data set. Then come back to make sure the format is correct and check for missing tests for the endpoint, --- app/dao/fact_billing_dao.py | 19 +++++++++---------- app/organisation/rest.py | 9 ++++++--- tests/app/organisation/test_rest.py | 17 +++++++++++++++++ tests/app/platform_stats/test_rest.py | 3 ++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 1706be8e3..7ffbda915 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -594,7 +594,6 @@ def fetch_email_usage_for_organisation(organisation_id, start_date, end_date): @statsd(namespace="dao") def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date): - # ASSUMPTION: AnnualBilling has been populated for year. free_allowance_remainder = fetch_sms_free_allowance_remainder(start_date).subquery() @@ -675,15 +674,15 @@ def fetch_usage_year_for_organisation(organisation_id, year): for usage in sms_usages: service_with_usage[str(usage.service_id)] = { - 'service_id': usage.service_id, - 'service_name': usage.service_name, - 'free_sms_limit': usage.free_sms_fragment_limit, - 'sms_remainder': usage.sms_remainder, - 'sms_billable_units': usage.sms_billable_units, - 'chargeable_billable_sms': usage.chargeable_billable_sms, - 'sms_cost': usage.sms_cost, - 'letter_cost': 0, - 'emails_sent': 0 + 'service_id': usage.service_id, + 'service_name': usage.service_name, + 'free_sms_limit': usage.free_sms_fragment_limit, + 'sms_remainder': usage.sms_remainder, + 'sms_billable_units': usage.sms_billable_units, + 'chargeable_billable_sms': usage.chargeable_billable_sms, + 'sms_cost': usage.sms_cost, + 'letter_cost': 0, + 'emails_sent': 0 } for letter_usage in letter_usages: service_with_usage[str(letter_usage.service_id)]['letter_cost'] = letter_usage.letter_cost diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 913df584a..bf7aaebe5 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,7 +1,10 @@ +from datetime import datetime + from flask import abort, Blueprint, jsonify, request, current_app from sqlalchemy.exc import IntegrityError from app.config import QueueNames +from app.dao.fact_billing_dao import fetch_usage_year_for_organisation from app.dao.organisation_dao import ( dao_create_organisation, dao_get_organisations, @@ -127,9 +130,9 @@ def get_organisation_services(organisation_id): @organisation_blueprint.route('//services-with-usage', methods=['GET']) def get_organisation_services_usage(organisation_id): - services = dao_get_organisation_services(organisation_id) - sorted_services = sorted(services, key=lambda s: (-s.active, s.name)) - return jsonify([s.serialize_for_org_dashboard() for s in sorted_services]) + services = fetch_usage_year_for_organisation(organisation_id, datetime.utcnow().year) + + return jsonify(services=services) @organisation_blueprint.route('//users/', methods=['POST']) diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 38df2b986..ce22c056b 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -1,3 +1,5 @@ +from datetime import datetime + import uuid import pytest @@ -11,6 +13,8 @@ from tests.app.db import ( create_organisation, create_service, create_user, + create_template, + create_ft_billing ) @@ -737,3 +741,16 @@ def test_is_organisation_name_unique_returns_400_when_name_does_not_exist(admin_ assert response["message"][0]["org_id"] == ["Can't be empty"] assert response["message"][1]["name"] == ["Can't be empty"] + + +def test_get_organisation_services_usage(admin_request, notify_db_session): + org = create_organisation(name='Organisation without live services') + service = create_service() + template = create_template(service=service) + dao_add_service_to_organisation(service=service, organisation_id=org.id) + create_ft_billing(bst_date=datetime.utcnow().date(), template=template, billable_unit=19, notifications_sent=19) + response = admin_request.get( + 'organisation.get_organisation_services_usage', + organisation_id=org.id + ) + assert len(response) == 1 diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index 45348e9bd..93b4eda11 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -122,7 +122,8 @@ def test_validate_date_is_within_a_financial_year_when_input_is_not_a_date(start def test_get_usage_for_all_services(notify_db_session, admin_request): - org, org_2, service, service_2, service_3, service_sms_only = set_up_usage_data(datetime(2019, 5, 1)) + org, org_2, service, service_2, service_3, service_sms_only, \ + org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) response = admin_request.get("platform_stats.get_usage_for_all_services", start_date='2019-05-01', end_date='2019-06-30') From 67c64a87158047fb1015f52ddb30040738c0f250 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Feb 2020 17:34:03 +0000 Subject: [PATCH 5/7] Format the response to a more managable list. Add a sort order --- app/dao/fact_billing_dao.py | 9 +++------ app/organisation/rest.py | 12 ++++++++---- tests/app/dao/test_ft_billing_dao.py | 4 ++-- tests/app/organisation/test_rest.py | 18 +++++++++++++++++- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 7ffbda915..5689d4aee 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -577,7 +577,6 @@ def fetch_email_usage_for_organisation(organisation_id, start_date, end_date): ).join( FactBilling, FactBilling.service_id == Service.id, ).filter( - FactBilling.service_id == Service.id, FactBilling.bst_date >= start_date, FactBilling.bst_date <= end_date, FactBilling.notification_type == EMAIL_TYPE, @@ -588,7 +587,6 @@ def fetch_email_usage_for_organisation(organisation_id, start_date, end_date): ).order_by( Service.name ) - return query.all() @@ -671,7 +669,6 @@ def fetch_usage_year_for_organisation(organisation_id, year): sms_usages = fetch_sms_billing_for_organisation(organisation_id, year_start_date, year_end_date) letter_usages = fetch_letter_costs_for_organisation(organisation_id, year_start_date, year_end_date) email_usages = fetch_email_usage_for_organisation(organisation_id, year_start_date, year_end_date) - for usage in sms_usages: service_with_usage[str(usage.service_id)] = { 'service_id': usage.service_id, @@ -679,13 +676,13 @@ def fetch_usage_year_for_organisation(organisation_id, year): 'free_sms_limit': usage.free_sms_fragment_limit, 'sms_remainder': usage.sms_remainder, 'sms_billable_units': usage.sms_billable_units, - 'chargeable_billable_sms': usage.chargeable_billable_sms, - 'sms_cost': usage.sms_cost, + 'chargeable_billable_sms': float(usage.chargeable_billable_sms), + 'sms_cost': float(usage.sms_cost), 'letter_cost': 0, 'emails_sent': 0 } for letter_usage in letter_usages: - service_with_usage[str(letter_usage.service_id)]['letter_cost'] = letter_usage.letter_cost + service_with_usage[str(letter_usage.service_id)]['letter_cost'] = float(letter_usage.letter_cost) for email_usage in email_usages: service_with_usage[str(email_usage.service_id)]['emails_sent'] = email_usage.emails_sent diff --git a/app/organisation/rest.py b/app/organisation/rest.py index bf7aaebe5..0cdf51b3c 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,4 +1,3 @@ -from datetime import datetime from flask import abort, Blueprint, jsonify, request, current_app from sqlalchemy.exc import IntegrityError @@ -130,9 +129,14 @@ def get_organisation_services(organisation_id): @organisation_blueprint.route('//services-with-usage', methods=['GET']) def get_organisation_services_usage(organisation_id): - services = fetch_usage_year_for_organisation(organisation_id, datetime.utcnow().year) - - return jsonify(services=services) + try: + year = int(request.args.get('year')) + except ValueError: + return jsonify(result='error', message='No valid year provided'), 400 + services = fetch_usage_year_for_organisation(organisation_id, year) + list_services = services.values() + sorted_services = sorted(list_services, key=lambda s: s['service_name'].lower()) + return jsonify(services=sorted_services) @organisation_blueprint.route('//users/', methods=['POST']) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 2bcb5053f..c01409656 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -649,8 +649,8 @@ def test_fetch_usage_year_for_organisation(notify_db_session): assert first_row['free_sms_limit'] == 10 assert first_row['sms_remainder'] == 10 assert first_row['chargeable_billable_sms'] == 0 - assert first_row['sms_cost'] == Decimal('0.0') - assert first_row['letter_cost'] == Decimal('3.40') + assert first_row['sms_cost'] == 0.0 + assert first_row['letter_cost'] == 3.4 assert first_row['emails_sent'] == 0 second_row = results[str(service_with_emails_for_org.id)] diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index ce22c056b..6a332ba0a 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -3,6 +3,7 @@ from datetime import datetime import uuid import pytest +from freezegun import freeze_time from app.models import Organisation from app.dao.organisation_dao import dao_add_service_to_organisation, dao_add_user_to_organisation @@ -743,6 +744,7 @@ def test_is_organisation_name_unique_returns_400_when_name_does_not_exist(admin_ assert response["message"][1]["name"] == ["Can't be empty"] +@freeze_time('2020-02-24 13:30') def test_get_organisation_services_usage(admin_request, notify_db_session): org = create_organisation(name='Organisation without live services') service = create_service() @@ -751,6 +753,20 @@ def test_get_organisation_services_usage(admin_request, notify_db_session): create_ft_billing(bst_date=datetime.utcnow().date(), template=template, billable_unit=19, notifications_sent=19) response = admin_request.get( 'organisation.get_organisation_services_usage', - organisation_id=org.id + organisation_id=org.id, + **{"year": 2019} ) assert len(response) == 1 + assert len(response['services']) == 1 + print(response) + assert response['services'][0]['service_id'] == str(service.id) + + +def test_get_organisation_services_usage_returns_400_if_year_is_invalid(admin_request): + response = admin_request.get( + 'organisation.get_organisation_services_usage', + organisation_id=uuid.uuid4(), + **{"year": 'year'}, + _expected_status=400 + ) + assert response['message'] == 'No valid year provided' From f7a564a17c19b4389d5d69a5c5723af250a2306e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 25 Feb 2020 17:47:03 +0000 Subject: [PATCH 6/7] Add more realistic test Add statsd Fix imports --- app/dao/fact_billing_dao.py | 12 +++++++----- tests/app/dao/test_ft_billing_dao.py | 6 +++--- tests/app/organisation/test_rest.py | 19 +++++++++++++++---- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 5689d4aee..594ad004e 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -542,6 +542,7 @@ def create_billing_record(data, rate, process_day): return billing_record +@statsd(namespace="dao") def fetch_letter_costs_for_organisation(organisation_id, start_date, end_date): query = db.session.query( Service.name.label("service_name"), @@ -552,7 +553,6 @@ def fetch_letter_costs_for_organisation(organisation_id, start_date, end_date): ).join( FactBilling, FactBilling.service_id == Service.id, ).filter( - FactBilling.service_id == Service.id, FactBilling.bst_date >= start_date, FactBilling.bst_date <= end_date, FactBilling.notification_type == LETTER_TYPE, @@ -567,6 +567,7 @@ def fetch_letter_costs_for_organisation(organisation_id, start_date, end_date): return query.all() +@statsd(namespace="dao") def fetch_email_usage_for_organisation(organisation_id, start_date, end_date): query = db.session.query( Service.name.label("service_name"), @@ -636,6 +637,7 @@ def fetch_sms_billing_for_organisation(organisation_id, start_date, end_date): return query.all() +@statsd(namespace="dao") def fetch_usage_year_for_organisation(organisation_id, year): year_start_datetime, year_end_datetime = get_financial_year(year) @@ -662,8 +664,8 @@ def fetch_usage_year_for_organisation(organisation_id, year): 'sms_remainder': 0, 'sms_billable_units': 0, 'chargeable_billable_sms': 0, - 'sms_cost': 0, - 'letter_cost': 0, + 'sms_cost': 0.0, + 'letter_cost': 0.0, 'emails_sent': 0 } sms_usages = fetch_sms_billing_for_organisation(organisation_id, year_start_date, year_end_date) @@ -676,9 +678,9 @@ def fetch_usage_year_for_organisation(organisation_id, year): 'free_sms_limit': usage.free_sms_fragment_limit, 'sms_remainder': usage.sms_remainder, 'sms_billable_units': usage.sms_billable_units, - 'chargeable_billable_sms': float(usage.chargeable_billable_sms), + 'chargeable_billable_sms': usage.chargeable_billable_sms, 'sms_cost': float(usage.sms_cost), - 'letter_cost': 0, + 'letter_cost': 0.0, 'emails_sent': 0 } for letter_usage in letter_usages: diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index c01409656..1a173d428 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -18,7 +18,9 @@ from app.dao.fact_billing_dao import ( get_rates_for_billing, fetch_sms_free_allowance_remainder, fetch_sms_billing_for_all_services, - fetch_letter_costs_for_all_services, fetch_letter_line_items_for_all_services, fetch_usage_year_for_organisation + fetch_letter_costs_for_all_services, + fetch_letter_line_items_for_all_services, + fetch_usage_year_for_organisation ) from app.dao.organisation_dao import dao_add_service_to_organisation from app.models import ( @@ -583,8 +585,6 @@ def test_fetch_sms_billing_for_all_services_with_remainder(notify_db_session): results = fetch_sms_billing_for_all_services(datetime(2019, 5, 1), datetime(2019, 5, 31)) assert len(results) == 3 - # (organisation_name, organisation_id, service_name, free_sms_fragment_limit, sms_rate, - # sms_remainder, sms_billable_units, chargeable_billable_sms, sms_cost) assert results[0] == (org.name, org.id, service.name, service.id, 10, Decimal('0.11'), 8, 3, 0, Decimal('0')) assert results[1] == (org_2.name, org_2.id, service_2.name, service_2.id, 10, Decimal('0.11'), 0, 3, 3, Decimal('0.33')) diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 6a332ba0a..fc741c89d 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -15,7 +15,8 @@ from tests.app.db import ( create_service, create_user, create_template, - create_ft_billing + create_ft_billing, + create_annual_billing ) @@ -750,7 +751,9 @@ def test_get_organisation_services_usage(admin_request, notify_db_session): service = create_service() template = create_template(service=service) dao_add_service_to_organisation(service=service, organisation_id=org.id) - create_ft_billing(bst_date=datetime.utcnow().date(), template=template, billable_unit=19, notifications_sent=19) + create_annual_billing(service_id=service.id, free_sms_fragment_limit=10, financial_year_start=2019) + create_ft_billing(bst_date=datetime.utcnow().date(), template=template, billable_unit=19, rate=0.060, + notifications_sent=19) response = admin_request.get( 'organisation.get_organisation_services_usage', organisation_id=org.id, @@ -758,8 +761,16 @@ def test_get_organisation_services_usage(admin_request, notify_db_session): ) assert len(response) == 1 assert len(response['services']) == 1 - print(response) - assert response['services'][0]['service_id'] == str(service.id) + service_usage = response['services'][0] + assert service_usage['service_id'] == str(service.id) + assert service_usage['service_name'] == service.name + assert service_usage['chargeable_billable_sms'] == 9.0 + assert service_usage['emails_sent'] == 0 + assert service_usage['free_sms_limit'] == 10 + assert service_usage['letter_cost'] == 0 + assert service_usage['sms_billable_units'] == 19 + assert service_usage['sms_remainder'] == 10 + assert service_usage['sms_cost'] == 0.54 def test_get_organisation_services_usage_returns_400_if_year_is_invalid(admin_request): From c91f37ff4ce889064feb848bf141290f5be1376e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 26 Feb 2020 17:38:20 +0000 Subject: [PATCH 7/7] Change the updates to only look at today, and not yesterday. --- app/dao/fact_billing_dao.py | 16 ++++++---------- app/organisation/rest.py | 2 +- tests/app/dao/test_ft_billing_dao.py | 1 + tests/app/db.py | 2 +- tests/app/organisation/test_rest.py | 11 ++++++++++- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 594ad004e..eced55218 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -244,11 +244,9 @@ def fetch_monthly_billing_for_year(service_id, year): today = convert_utc_to_bst(datetime.utcnow()).date() # 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 >= today: - yesterday = today - timedelta(days=1) - for day in [yesterday, today]: - data = fetch_billing_data_for_day(process_day=day, service_id=service_id, check_permissions=True) - for d in data: - update_fact_billing(data=d, process_day=day) + data = fetch_billing_data_for_day(process_day=today, service_id=service_id, check_permissions=True) + for d in data: + update_fact_billing(data=d, process_day=today) email_and_letters = db.session.query( func.date_trunc('month', FactBilling.bst_date).cast(Date).label("month"), @@ -648,12 +646,10 @@ def fetch_usage_year_for_organisation(organisation_id, year): services = dao_get_organisation_live_services(organisation_id) # 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 >= today: - yesterday = today - timedelta(days=1) for service in services: - for day in [yesterday, today]: - data = fetch_billing_data_for_day(process_day=day, service_id=service.id) - for d in data: - update_fact_billing(data=d, process_day=day) + data = fetch_billing_data_for_day(process_day=today, service_id=service.id) + for d in data: + update_fact_billing(data=d, process_day=today) service_with_usage = {} # initialise results for service in services: diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 0cdf51b3c..b1eb613df 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -130,7 +130,7 @@ def get_organisation_services(organisation_id): @organisation_blueprint.route('//services-with-usage', methods=['GET']) def get_organisation_services_usage(organisation_id): try: - year = int(request.args.get('year')) + year = int(request.args.get('year', 'none')) except ValueError: return jsonify(result='error', message='No valid year provided'), 400 services = fetch_usage_year_for_organisation(organisation_id, year) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 1a173d428..b12c68fad 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -631,6 +631,7 @@ def test_fetch_letter_line_items_for_all_service(notify_db_session): assert results[4] == (None, None, service_3.name, service_3.id, Decimal("0.55"), 'second', 15) +@freeze_time('2019-06-01 13:30') def test_fetch_usage_year_for_organisation(notify_db_session): org, org_2, service, service_2, service_3, service_sms_only, \ org_with_emails, service_with_emails = set_up_usage_data(datetime(2019, 5, 1)) diff --git a/tests/app/db.py b/tests/app/db.py index 29e36c04b..0c847b920 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -908,7 +908,7 @@ def set_up_usage_data(start_date): service_2 = create_service(service_name='b - emails') email_template = create_template(service=service_2, template_type='email') - org_2 = create_organisation(name='Org for {}'.format(service_2)) + org_2 = create_organisation(name='Org for {}'.format(service_2.name)) dao_add_service_to_organisation(service=service_2, organisation_id=org_2.id) service_3 = create_service(service_name='c - letters only') diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index fc741c89d..2f697871b 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -777,7 +777,16 @@ def test_get_organisation_services_usage_returns_400_if_year_is_invalid(admin_re response = admin_request.get( 'organisation.get_organisation_services_usage', organisation_id=uuid.uuid4(), - **{"year": 'year'}, + **{"year": 'not-a-valid-year'}, + _expected_status=400 + ) + assert response['message'] == 'No valid year provided' + + +def test_get_organisation_services_usage_returns_400_if_year_is_empty(admin_request): + response = admin_request.get( + 'organisation.get_organisation_services_usage', + organisation_id=uuid.uuid4(), _expected_status=400 ) assert response['message'] == 'No valid year provided'