From cf7997d925b80f84d8281c77f18c4d062fafc663 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 2 May 2019 09:26:49 +0100 Subject: [PATCH] Change get_providers endpoint to return no of SMS sent by each provider In addition to the existing provider data, we also want return the number of billable units (muliplied by the rate multiplier) that each SMS provider sent this month. This will be used on the platform admin providers page. Since we can no longer get all the information we need from the provider details schema, this makes a new DAO function to get all the data for the endpoint. --- app/dao/provider_details_dao.py | 48 ++++++++++++++++--- app/provider_details/rest.py | 21 +++++++-- tests/app/dao/test_provider_details_dao.py | 55 ++++++++++++++++++++-- tests/app/provider_details/test_rest.py | 13 +++-- 4 files changed, 120 insertions(+), 17 deletions(-) diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index 1e2c11727..a6b8063b6 100644 --- a/app/dao/provider_details_dao.py +++ b/app/dao/provider_details_dao.py @@ -1,6 +1,7 @@ from datetime import datetime -from sqlalchemy import asc, desc +from notifications_utils.timezones import convert_utc_to_bst +from sqlalchemy import asc, desc, func from app.dao.dao_utils import transactional from app.provider_details.switch_providers import ( @@ -8,14 +9,10 @@ from app.provider_details.switch_providers import ( provider_is_primary, switch_providers ) -from app.models import ProviderDetails, ProviderDetailsHistory +from app.models import FactBilling, ProviderDetails, ProviderDetailsHistory, SMS_TYPE, User from app import db -def get_provider_details(): - return ProviderDetails.query.order_by(asc(ProviderDetails.priority), asc(ProviderDetails.notification_type)).all() - - def get_provider_details_by_id(provider_details_id): return ProviderDetails.query.get(provider_details_id) @@ -110,3 +107,42 @@ def dao_get_sms_provider_with_equal_priority(identifier, priority): ).first() return provider + + +def dao_get_provider_stats(): + # this query does not include the current day since the task to populate ft_billing runs overnight + + current_bst_datetime = convert_utc_to_bst(datetime.utcnow()) + first_day_of_the_month = current_bst_datetime.date().replace(day=1) + + subquery = db.session.query( + FactBilling.provider, + func.sum(FactBilling.billable_units * FactBilling.rate_multiplier).label('current_month_billable_sms') + ).filter( + FactBilling.notification_type == SMS_TYPE, + FactBilling.bst_date >= first_day_of_the_month + ).group_by( + FactBilling.provider + ).subquery() + + result = db.session.query( + ProviderDetails.id, + ProviderDetails.display_name, + ProviderDetails.identifier, + ProviderDetails.priority, + ProviderDetails.notification_type, + ProviderDetails.active, + ProviderDetails.updated_at, + ProviderDetails.supports_international, + User.name.label('created_by_name'), + func.coalesce(subquery.c.current_month_billable_sms, 0).label('current_month_billable_sms') + ).outerjoin( + subquery, ProviderDetails.identifier == subquery.c.provider + ).outerjoin( + User, ProviderDetails.created_by_id == User.id + ).order_by( + ProviderDetails.notification_type, + ProviderDetails.priority, + ).all() + + return result diff --git a/app/provider_details/rest.py b/app/provider_details/rest.py index a86860b6e..c5a58127b 100644 --- a/app/provider_details/rest.py +++ b/app/provider_details/rest.py @@ -2,9 +2,9 @@ from flask import Blueprint, jsonify, request from app.schemas import provider_details_schema, provider_details_history_schema from app.dao.provider_details_dao import ( - get_provider_details, get_provider_details_by_id, dao_update_provider_details, + dao_get_provider_stats, dao_get_provider_versions ) from app.dao.users_dao import get_user_by_id @@ -19,8 +19,23 @@ register_errors(provider_details) @provider_details.route('', methods=['GET']) def get_providers(): - data = provider_details_schema.dump(get_provider_details(), many=True).data - return jsonify(provider_details=data) + data = dao_get_provider_stats() + + provider_details = [ + {'id': row.id, + 'display_name': row.display_name, + 'identifier': row.identifier, + 'priority': row.priority, + 'notification_type': row.notification_type, + 'active': row.active, + 'updated_at': row.updated_at, + 'supports_international': row.supports_international, + 'created_by_name': row.created_by_name, + 'current_month_billable_sms': row.current_month_billable_sms} + for row in data + ] + + return jsonify(provider_details=provider_details) @provider_details.route('/', methods=['GET']) diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index d5ff8318a..31a058831 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -9,15 +9,20 @@ from app import clients from app.dao.provider_details_dao import ( get_alternative_sms_provider, get_current_provider, - get_provider_details, get_provider_details_by_identifier, get_provider_details_by_notification_type, dao_switch_sms_provider_to_provider_with_identifier, dao_toggle_sms_provider, dao_update_provider_details, + dao_get_provider_stats, dao_get_provider_versions, dao_get_sms_provider_with_equal_priority ) +from tests.app.db import ( + create_ft_billing, + create_service, + create_template, +) def set_primary_sms_provider(identifier): @@ -31,10 +36,6 @@ def set_primary_sms_provider(identifier): dao_update_provider_details(secondary_provider) -def test_can_get_all_providers(restore_provider_details): - assert len(get_provider_details()) == 5 - - def test_can_get_sms_non_international_providers(restore_provider_details): sms_providers = get_provider_details_by_notification_type('sms') assert len(sms_providers) == 3 @@ -287,3 +288,47 @@ def test_get_current_sms_provider_returns_active_only(restore_provider_details): new_current_provider = get_current_provider('sms') assert current_provider.identifier != new_current_provider.identifier + + +@freeze_time('2018-06-28 12:00') +def test_dao_get_provider_stats(notify_db_session): + service_1 = create_service(service_name='1') + service_2 = create_service(service_name='2') + sms_template_1 = create_template(service_1, 'sms') + sms_template_2 = create_template(service_2, 'sms') + + create_ft_billing('2017-06-05', 'sms', sms_template_2, service_1, provider='firetext', billable_unit=4) + create_ft_billing('2018-05-31', 'sms', sms_template_1, service_1, provider='mmg', billable_unit=1) + create_ft_billing('2018-06-01', 'sms', sms_template_1, service_1, provider='mmg', + rate_multiplier=2, billable_unit=1) + create_ft_billing('2018-06-03', 'sms', sms_template_2, service_1, provider='firetext', billable_unit=4) + create_ft_billing('2018-06-15', 'sms', sms_template_1, service_2, provider='firetext', billable_unit=1) + create_ft_billing('2018-06-28', 'sms', sms_template_2, service_2, provider='mmg', billable_unit=2) + + result = dao_get_provider_stats() + + assert len(result) == 5 + + assert result[0].identifier == 'ses' + assert result[0].display_name == 'AWS SES' + assert result[0].created_by_name is None + assert result[0].current_month_billable_sms == 0 + + assert result[1].identifier == 'mmg' + assert result[1].display_name == 'MMG' + assert result[1].supports_international is True + assert result[1].active is True + assert result[1].current_month_billable_sms == 4 + + assert result[2].identifier == 'firetext' + assert result[2].notification_type == 'sms' + assert result[2].supports_international is False + assert result[2].active is True + assert result[2].current_month_billable_sms == 5 + + assert result[3].identifier == 'loadtesting' + assert result[3].current_month_billable_sms == 0 + + assert result[4].identifier == 'dvla' + assert result[4].current_month_billable_sms == 0 + assert result[4].supports_international is False diff --git a/tests/app/provider_details/test_rest.py b/tests/app/provider_details/test_rest.py index 36a40d2a0..fd9428e47 100644 --- a/tests/app/provider_details/test_rest.py +++ b/tests/app/provider_details/test_rest.py @@ -1,9 +1,11 @@ import pytest from flask import json +from freezegun import freeze_time from app.models import ProviderDetails, ProviderDetailsHistory from tests import create_authorization_header +from tests.app.db import create_ft_billing def test_get_provider_details_in_type_and_identifier_order(client, notify_db): @@ -38,17 +40,22 @@ def test_get_provider_details_by_id(client, notify_db): assert provider['identifier'] == json_resp[0]['identifier'] -def test_get_provider_details_contains_correct_fields(client, notify_db): +@freeze_time('2018-06-28 12:00') +def test_get_provider_contains_correct_fields(client, sample_service, sample_template): + create_ft_billing('2018-06-01', 'sms', sample_template, sample_service, provider='mmg', billable_unit=1) + response = client.get( '/provider-details', headers=[create_authorization_header()] ) json_resp = json.loads(response.get_data(as_text=True))['provider_details'] allowed_keys = { - "id", "created_by", "display_name", + "id", "created_by_name", "display_name", "identifier", "priority", 'notification_type', - "active", "version", "updated_at", "supports_international" + "active", "updated_at", "supports_international", + "current_month_billable_sms" } + assert len(json_resp) == 5 assert allowed_keys == set(json_resp[0].keys())