diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index 1527f8c88..93dca2d61 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -4,7 +4,8 @@ from flask import current_app from notifications_utils.statsd_decorators import statsd from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst from sqlalchemy.dialects.postgresql import insert -from sqlalchemy import func, case, desc, Date, Integer, and_ +from sqlalchemy import func, desc, Date, Integer, and_ +from sqlalchemy.sql.expression import literal from app import db from app.dao.date_util import ( @@ -20,9 +21,10 @@ from app.models import ( SMS_TYPE, Rate, LetterRate, - NOTIFICATION_STATUS_TYPES_BILLABLE, NotificationHistory, EMAIL_TYPE, + NOTIFICATION_STATUS_TYPES_BILLABLE_SMS, + NOTIFICATION_STATUS_TYPES_SENT_EMAILS, NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS, AnnualBilling, Organisation, @@ -243,7 +245,7 @@ def fetch_monthly_billing_for_year(service_id, year): 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) + 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) @@ -308,7 +310,7 @@ def delete_billing_data_for_service_for_day(process_day, service_id): @statsd(namespace="dao") -def fetch_billing_data_for_day(process_day, service_id=None): +def fetch_billing_data_for_day(process_day, service_id=None, check_permissions=False): 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)) current_app.logger.info("Populate ft_billing for {} to {}".format(start_date, end_date)) @@ -320,65 +322,112 @@ def fetch_billing_data_for_day(process_day, service_id=None): for service in services: for notification_type in (SMS_TYPE, EMAIL_TYPE, LETTER_TYPE): - table = get_notification_table_to_use(service, notification_type, process_day, has_delete_task_run=False) - - results = _query_for_billing_data( - table=table, - notification_type=notification_type, - start_date=start_date, - end_date=end_date, - service_id=service.id - ) - - transit_data += results + if (not check_permissions) or service.has_permission(notification_type): + table = get_notification_table_to_use(service, notification_type, process_day, + has_delete_task_run=False) + results = _query_for_billing_data( + table=table, + notification_type=notification_type, + start_date=start_date, + end_date=end_date, + service=service + ) + transit_data += results return transit_data -def _query_for_billing_data(table, notification_type, start_date, end_date, service_id): - billable_type_list = { - SMS_TYPE: NOTIFICATION_STATUS_TYPES_BILLABLE, - EMAIL_TYPE: NOTIFICATION_STATUS_TYPES_BILLABLE, - LETTER_TYPE: NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS - } - sent_by_func = func.coalesce(table.sent_by, case( - [(table.notification_type == 'letter', 'dvla'), - (table.notification_type == 'sms', 'unknown'), - (table.notification_type == 'email', 'ses')]), ) - letter_page_count = case([(table.notification_type == 'letter', table.billable_units), ]) +def _query_for_billing_data(table, notification_type, start_date, end_date, service): + def _email_query(): + return db.session.query( + table.template_id, + literal(service.crown).label('crown'), + literal(service.id).label('service_id'), + literal(notification_type).label('notification_type'), + literal('ses').label('sent_by'), + literal(0).label('rate_multiplier'), + literal(False).label('international'), + literal(None).label('letter_page_count'), + literal('none').label('postage'), + literal(0).label('billable_units'), + func.count().label('notifications_sent'), + ).filter( + table.status.in_(NOTIFICATION_STATUS_TYPES_SENT_EMAILS), + table.key_type != KEY_TYPE_TEST, + table.created_at >= start_date, + table.created_at < end_date, + table.notification_type == notification_type, + table.service_id == service.id + ).group_by( + table.template_id, + ) - query = db.session.query( - table.template_id, - table.service_id, - table.notification_type, - sent_by_func.label('sent_by'), - func.coalesce(table.rate_multiplier, 1).cast(Integer).label('rate_multiplier'), - func.coalesce(table.international, False).label('international'), - letter_page_count.label('letter_page_count'), - func.sum(table.billable_units).label('billable_units'), - func.count().label('notifications_sent'), - Service.crown, - func.coalesce(table.postage, 'none').label('postage') - ).filter( - table.status.in_(billable_type_list[notification_type]), - table.key_type != KEY_TYPE_TEST, - table.created_at >= start_date, - table.created_at < end_date, - table.notification_type == notification_type, - table.service_id == service_id - ).group_by( - table.template_id, - table.service_id, - table.notification_type, - sent_by_func, - letter_page_count, - table.rate_multiplier, - table.international, - Service.crown, - table.postage, - ).join( - Service - ) + def _sms_query(): + sent_by = func.coalesce(table.sent_by, 'unknown') + rate_multiplier = func.coalesce(table.rate_multiplier, 1).cast(Integer) + international = func.coalesce(table.international, False) + return db.session.query( + table.template_id, + literal(service.crown).label('crown'), + literal(service.id).label('service_id'), + literal(notification_type).label('notification_type'), + sent_by.label('sent_by'), + rate_multiplier.label('rate_multiplier'), + international.label('international'), + literal(None).label('letter_page_count'), + literal('none').label('postage'), + func.sum(table.billable_units).label('billable_units'), + func.count().label('notifications_sent'), + ).filter( + table.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE_SMS), + table.key_type != KEY_TYPE_TEST, + table.created_at >= start_date, + table.created_at < end_date, + table.notification_type == notification_type, + table.service_id == service.id + ).group_by( + table.template_id, + sent_by, + rate_multiplier, + international, + ) + + def _letter_query(): + rate_multiplier = func.coalesce(table.rate_multiplier, 1).cast(Integer) + postage = func.coalesce(table.postage, 'none') + return db.session.query( + table.template_id, + literal(service.crown).label('crown'), + literal(service.id).label('service_id'), + literal(notification_type).label('notification_type'), + literal('dvla').label('sent_by'), + rate_multiplier.label('rate_multiplier'), + literal(False).label('international'), # todo: this may change in the future. + table.billable_units.label('letter_page_count'), + postage.label('postage'), + func.sum(table.billable_units).label('billable_units'), + func.count().label('notifications_sent'), + ).filter( + table.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS), + table.key_type != KEY_TYPE_TEST, + table.created_at >= start_date, + table.created_at < end_date, + table.notification_type == notification_type, + table.service_id == service.id + ).group_by( + table.template_id, + rate_multiplier, + table.billable_units, + postage + ) + + query_funcs = { + SMS_TYPE: _sms_query, + EMAIL_TYPE: _email_query, + LETTER_TYPE: _letter_query + } + + query = query_funcs[notification_type]() return query.all() diff --git a/app/models.py b/app/models.py index 23c0f9cd6..a2df80f5a 100644 --- a/app/models.py +++ b/app/models.py @@ -1287,12 +1287,6 @@ NOTIFICATION_STATUS_SUCCESS = [ NOTIFICATION_DELIVERED ] -NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS = [ - NOTIFICATION_SENDING, - NOTIFICATION_DELIVERED, - NOTIFICATION_RETURNED_LETTER, -] - NOTIFICATION_STATUS_TYPES_BILLABLE = [ NOTIFICATION_SENDING, NOTIFICATION_SENT, @@ -1304,6 +1298,29 @@ NOTIFICATION_STATUS_TYPES_BILLABLE = [ NOTIFICATION_RETURNED_LETTER, ] +NOTIFICATION_STATUS_TYPES_BILLABLE_SMS = [ + NOTIFICATION_SENDING, + NOTIFICATION_SENT, # internationally + NOTIFICATION_DELIVERED, + NOTIFICATION_PENDING, + NOTIFICATION_TEMPORARY_FAILURE, + NOTIFICATION_PERMANENT_FAILURE, +] + +NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS = [ + NOTIFICATION_SENDING, + NOTIFICATION_DELIVERED, + NOTIFICATION_RETURNED_LETTER, +] +# we don't really have a concept of billable emails - however the ft billing table only includes emails that we have +# actually sent. +NOTIFICATION_STATUS_TYPES_SENT_EMAILS = [ + NOTIFICATION_SENDING, + NOTIFICATION_DELIVERED, + NOTIFICATION_TEMPORARY_FAILURE, + NOTIFICATION_PERMANENT_FAILURE, +] + NOTIFICATION_STATUS_TYPES = [ NOTIFICATION_CANCELLED, NOTIFICATION_CREATED, diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index c0fc69074..c1eb4d298 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -166,6 +166,7 @@ def test_create_nightly_billing_for_day_different_templates( multiplier = [0, 1] billable_units = [0, 1] rate = [0, Decimal(1.33)] + for i, record in enumerate(records): assert record.bst_date == datetime.date(yesterday) assert record.rate == rate[i] diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index b533cbdbc..6d07de2a1 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -70,6 +70,34 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_key_type(noti assert results[0].notifications_sent == 2 +@pytest.mark.parametrize("notification_type", ["email", "sms", "letter"]) +def test_fetch_billing_data_for_day_only_calls_query_for_permission_type(notify_db_session, notification_type): + service = create_service(service_permissions=[notification_type]) + email_template = create_template(service=service, template_type="email") + sms_template = create_template(service=service, template_type="sms") + letter_template = create_template(service=service, template_type="letter") + create_notification(template=email_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=letter_template, status='delivered') + today = convert_utc_to_bst(datetime.utcnow()) + results = fetch_billing_data_for_day(process_day=today.date(), check_permissions=True) + assert len(results) == 1 + + +@pytest.mark.parametrize("notification_type", ["email", "sms", "letter"]) +def test_fetch_billing_data_for_day_only_calls_query_for_all_channels(notify_db_session, notification_type): + service = create_service(service_permissions=[notification_type]) + email_template = create_template(service=service, template_type="email") + sms_template = create_template(service=service, template_type="sms") + letter_template = create_template(service=service, template_type="letter") + create_notification(template=email_template, status='delivered') + create_notification(template=sms_template, status='delivered') + create_notification(template=letter_template, status='delivered') + today = convert_utc_to_bst(datetime.utcnow()) + results = fetch_billing_data_for_day(process_day=today.date(), check_permissions=False) + assert len(results) == 3 + + @freeze_time('2018-04-02 01:20:00') def test_fetch_billing_data_for_today_includes_data_with_the_right_date(notify_db_session): process_day = datetime(2018, 4, 1, 13, 30, 0) @@ -170,7 +198,7 @@ def test_fetch_billing_data_for_day_is_grouped_by_notification_type(notify_db_se today = convert_utc_to_bst(datetime.utcnow()) results = fetch_billing_data_for_day(today.date()) assert len(results) == 3 - notification_types = [x[2] for x in results if x[2] in ['email', 'sms', 'letter']] + notification_types = [x.notification_type for x in results] assert len(notification_types) == 3 @@ -280,12 +308,13 @@ def test_fetch_billing_data_for_day_bills_correctly_for_status(notify_db_session today = convert_utc_to_bst(datetime.utcnow()) results = fetch_billing_data_for_day(process_day=today.date(), service_id=service.id) - sms_results = [x for x in results if x[2] == 'sms'] - email_results = [x for x in results if x[2] == 'email'] - letter_results = [x for x in results if x[2] == 'letter'] - assert 8 == sms_results[0][7] - assert 8 == email_results[0][7] - assert 3 == letter_results[0][7] + sms_results = [x for x in results if x.notification_type == 'sms'] + email_results = [x for x in results if x.notification_type == 'email'] + letter_results = [x for x in results if x.notification_type == 'letter'] + # we expect as many rows as we check for notification types + assert 6 == sms_results[0].notifications_sent + assert 4 == email_results[0].notifications_sent + assert 3 == letter_results[0].notifications_sent def test_get_rates_for_billing(notify_db_session):