From 2a0669636d461514b9680784253a4a0a6896ac9c Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 19 May 2017 16:42:33 +0100 Subject: [PATCH 01/15] Add and test new DAO method that counts the billable units multiplied by rate multiplier for a given service for a given time period. Currently this is SMS only. Used by the dashboard for a headline figure. --- app/dao/notification_usage_dao.py | 18 ++ tests/app/dao/test_notification_usage_dao.py | 197 ++++++++++++++++++- 2 files changed, 212 insertions(+), 3 deletions(-) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 4f9e2ee8e..f4c233468 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -153,3 +153,21 @@ def rate_multiplier(): (NotificationHistory.rate_multiplier == None, literal_column("'1'")), # noqa (NotificationHistory.rate_multiplier != None, NotificationHistory.rate_multiplier), # noqa ]), Integer()) + + +@statsd(namespace="dao") +def get_total_billable_units_for_sent_sms_notifications_in_date_range(start_date, end_date, service_id): + result = db.session.query( + func.sum( + NotificationHistory.billable_units * func.coalesce(NotificationHistory.rate_multiplier, 1) + ).label('billable_units') + ).filter( + NotificationHistory.service_id == service_id, + NotificationHistory.notification_type == 'sms', + NotificationHistory.created_at >= start_date, + NotificationHistory.created_at <= end_date, + NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE) + ) + if result.scalar(): + return int(result.scalar()) + return 0 diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 5252b0f90..81cbd0202 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -1,11 +1,17 @@ import uuid -from datetime import datetime +from datetime import datetime, timedelta + +import pytest from app.dao.date_util import get_financial_year from app.dao.notification_usage_dao import (get_rates_for_year, get_yearly_billing_data, - get_monthly_billing_data) -from app.models import Rate + get_monthly_billing_data, + get_total_billable_units_for_sent_sms_notifications_in_date_range) +from app.models import Rate, NOTIFICATION_STATUS_SUCCESS, NOTIFICATION_DELIVERED, NOTIFICATION_STATUS_TYPES_BILLABLE, \ + NOTIFICATION_CREATED, NOTIFICATION_STATUS_TYPES_NON_BILLABLE +from tests.app.conftest import sample_notification, sample_email_template, sample_letter_template, sample_service from tests.app.db import create_notification +from freezegun import freeze_time def test_get_rates_for_year(notify_db, notify_db_session): @@ -248,3 +254,188 @@ def test_get_monthly_billing_data_with_no_notifications_for_year(notify_db, noti def set_up_rate(notify_db, start_date, value): rate = Rate(id=uuid.uuid4(), valid_from=start_date, rate=value, notification_type='sms') notify_db.session.add(rate) + + +@freeze_time("2016-01-10 12:00:00.000000") +def test_returns_total_billable_units_for_sms_notifications(notify_db, notify_db_session, sample_service): + sample_notification( + notify_db, notify_db_session, service=sample_service, billable_units=1, status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, notify_db_session, service=sample_service, billable_units=2, status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, notify_db_session, service=sample_service, billable_units=3, status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, notify_db_session, service=sample_service, billable_units=4, status=NOTIFICATION_DELIVERED) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 10 + + +@freeze_time("2016-01-10 12:00:00.000000") +def test_returns_total_billable_units_multiplied_by_multipler_for_sms_notifications( + notify_db, notify_db_session, sample_service +): + sample_notification( + notify_db, notify_db_session, service=sample_service, rate_multiplier=1.0, status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, notify_db_session, service=sample_service, rate_multiplier=2.0, status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, notify_db_session, service=sample_service, rate_multiplier=5.0, status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, notify_db_session, service=sample_service, rate_multiplier=10.0, status=NOTIFICATION_DELIVERED) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 18 + + +@freeze_time("2016-01-10 12:00:00.000000") +def test_returns_total_billable_units_for_sms_notifications_ignoring_letters_and_emails( + notify_db, notify_db_session, sample_service +): + email_template = sample_email_template(notify_db, notify_db_session, service=sample_service) + letter_template = sample_letter_template(sample_service) + + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + billable_units=2, + status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, + notify_db_session, + template=email_template, + service=sample_service, + billable_units=2, + status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, + notify_db_session, + template=letter_template, + service=sample_service, + billable_units=2, + status=NOTIFICATION_DELIVERED + ) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 2 + + +@freeze_time("2016-01-10 12:00:00.000000") +def test_returns_total_billable_units_for_sms_notifications_for_only_requested_service( + notify_db, notify_db_session +): + service_1 = sample_service(notify_db, notify_db_session, service_name=str(uuid.uuid4())) + service_2 = sample_service(notify_db, notify_db_session, service_name=str(uuid.uuid4())) + service_3 = sample_service(notify_db, notify_db_session, service_name=str(uuid.uuid4())) + + sample_notification( + notify_db, + notify_db_session, + service=service_1, + billable_units=2, + status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, + notify_db_session, + service=service_2, + billable_units=2, + status=NOTIFICATION_DELIVERED) + sample_notification( + notify_db, + notify_db_session, + service=service_3, + billable_units=2, + status=NOTIFICATION_DELIVERED + ) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, service_1.id) == 2 + + +@freeze_time("2016-01-10 12:00:00.000000") +def test_returns_total_billable_units_for_sms_notifications_handling_null_values( + notify_db, notify_db_session, sample_service +): + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + billable_units=2, + rate_multiplier=None, + status=NOTIFICATION_DELIVERED) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 2 + + +@pytest.mark.parametrize('billable_units, states', ([ + (len(NOTIFICATION_STATUS_TYPES_BILLABLE), NOTIFICATION_STATUS_TYPES_BILLABLE), + (0, NOTIFICATION_STATUS_TYPES_NON_BILLABLE) +])) +@freeze_time("2016-01-10 12:00:00.000000") +def test_ignores_non_billable_states_when_returning_billable_units_for_sms_notifications( + notify_db, notify_db_session, sample_service, billable_units, states +): + for state in states: + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + billable_units=1, + rate_multiplier=None, + status=state) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range( + start, end, sample_service.id + ) == billable_units + + +@freeze_time("2016-01-10 12:00:00.000000") +def test_restricts_to_time_period_when_returning_billable_units_for_sms_notifications( + notify_db, notify_db_session, sample_service +): + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + billable_units=1, + rate_multiplier=1.0, + created_at=datetime.utcnow() - timedelta(minutes=100), + status=NOTIFICATION_DELIVERED) + + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + billable_units=1, + rate_multiplier=1.0, + created_at=datetime.utcnow() - timedelta(minutes=5), + status=NOTIFICATION_DELIVERED) + + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 1 + + +def test_returns_zero_if_no_matching_rows_when_returning_billable_units_for_sms_notifications( + notify_db, notify_db_session, sample_service +): + start = datetime.utcnow() - timedelta(minutes=10) + end = datetime.utcnow() + timedelta(minutes=10) + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 0 + From 7268bc28fe4de692fb434009b132dbd93628cfce Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 19 May 2017 16:42:47 +0100 Subject: [PATCH 02/15] New array of non-billable states --- app/models.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models.py b/app/models.py index 5fff42027..50f3495d8 100644 --- a/app/models.py +++ b/app/models.py @@ -647,6 +647,7 @@ NOTIFICATION_STATUS_TYPES_BILLABLE = [ NOTIFICATION_PERMANENT_FAILURE, ] + NOTIFICATION_STATUS_TYPES = [ NOTIFICATION_CREATED, NOTIFICATION_SENDING, @@ -659,6 +660,8 @@ NOTIFICATION_STATUS_TYPES = [ NOTIFICATION_PERMANENT_FAILURE, ] +NOTIFICATION_STATUS_TYPES_NON_BILLABLE = list(set(NOTIFICATION_STATUS_TYPES) - set(NOTIFICATION_STATUS_TYPES_BILLABLE)) + NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type') From f0395e74963be0d0de0411ccf9352c28ee0f9aa3 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 19 May 2017 16:43:05 +0100 Subject: [PATCH 03/15] New endpoint to get the count of billable SMS units. --- app/service/rest.py | 24 ++++++ tests/app/service/test_rest.py | 153 ++++++++++++++++++++++++--------- 2 files changed, 134 insertions(+), 43 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 180740063..423d5dc9b 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -9,6 +9,7 @@ from flask import ( ) from sqlalchemy.orm.exc import NoResultFound +from app import redis_store from app.dao import notification_usage_dao from app.dao.dao_utils import dao_rollback from app.dao.api_key_dao import ( @@ -16,6 +17,8 @@ from app.dao.api_key_dao import ( get_model_api_keys, get_unsigned_secret, expire_api_key) +from app.dao.date_util import get_financial_year +from app.dao.notification_usage_dao import get_total_billable_units_for_sent_sms_notifications_in_date_range from app.dao.services_dao import ( dao_fetch_service_by_id, dao_fetch_all_services, @@ -58,6 +61,7 @@ from app.schemas import ( ) from app.utils import pagination_links from flask import Blueprint +from notifications_utils.clients.redis import sms_billable_units_cache_key service_blueprint = Blueprint('service', __name__) @@ -440,6 +444,26 @@ def get_monthly_template_stats(service_id): raise InvalidRequest('Year must be a number', status_code=400) +@service_blueprint.route('//yearly-usage-count') +def get_yearly_usage_count(service_id): + try: + cache_key = sms_billable_units_cache_key(service_id) + cached_value = redis_store.get(cache_key) + if cached_value: + return jsonify({'billable_sms_units': cached_value}) + else: + start_date, end_date = get_financial_year(int(request.args.get('year'))) + billable_units = get_total_billable_units_for_sent_sms_notifications_in_date_range( + start_date, + end_date, + service_id) + redis_store.set(cache_key, billable_units, ex=60) + return jsonify({'billable_sms_units': billable_units}) + + except (ValueError, TypeError): + return jsonify(result='error', message='No valid year provided'), 400 + + @service_blueprint.route('//yearly-usage') def get_yearly_billing_usage(service_id): try: diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index c12ca4eec..c7ad9495c 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -62,9 +62,9 @@ def test_get_service_list_with_only_active_flag(client, service_factory): def test_get_service_list_with_user_id_and_only_active_flag( - client, - sample_user, - service_factory + client, + sample_user, + service_factory ): other_user = create_user(email='foo@bar.gov.uk') @@ -663,7 +663,6 @@ def test_add_existing_user_to_another_service_with_all_permissions(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - # check which users part of service user_already_in_service = sample_service.users[0] auth_header = create_authorization_header() @@ -738,7 +737,6 @@ def test_add_existing_user_to_another_service_with_send_permissions(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - # they must exist in db first user_to_add = User( name='Invited User', @@ -782,7 +780,6 @@ def test_add_existing_user_to_another_service_with_manage_permissions(notify_api sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - # they must exist in db first user_to_add = User( name='Invited User', @@ -826,7 +823,6 @@ def test_add_existing_user_to_another_service_with_manage_api_keys(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - # they must exist in db first user_to_add = User( name='Invited User', @@ -867,7 +863,6 @@ def test_add_existing_user_to_non_existing_service_returns404(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - user_to_add = User( name='Invited User', email_address='invited@digital.cabinet-office.gov.uk', @@ -898,7 +893,6 @@ def test_add_existing_user_to_non_existing_service_returns404(notify_api, def test_add_existing_user_of_service_to_service_returns400(notify_api, notify_db, notify_db_session, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: - existing_user_id = sample_service.users[0].id data = {'permissions': ['send_messages', 'manage_service', 'manage_api_keys']} @@ -921,7 +915,6 @@ def test_add_existing_user_of_service_to_service_returns400(notify_api, notify_d def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db_session, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: - incorrect_id = 9876 data = {'permissions': ['send_messages', 'manage_service', 'manage_api_keys']} @@ -942,7 +935,7 @@ def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db def test_remove_user_from_service( - notify_db, notify_db_session, client, sample_user_service_permission + notify_db, notify_db_session, client, sample_user_service_permission ): second_user = create_user(email="new@digital.cabinet-office.gov.uk") # Simulates successfully adding a user to the service @@ -962,7 +955,7 @@ def test_remove_user_from_service( def test_remove_non_existant_user_from_service( - client, sample_user_service_permission + client, sample_user_service_permission ): second_user = create_user(email="new@digital.cabinet-office.gov.uk") endpoint = url_for( @@ -998,13 +991,11 @@ def test_cannot_remove_only_user_from_service(notify_api, # This test is just here verify get_service_and_api_key_history that is a temp solution # until proper ui is sorted out on admin app def test_get_service_and_api_key_history(notify_api, notify_db, notify_db_session, sample_service): - from tests.app.conftest import sample_api_key as create_sample_api_key api_key = create_sample_api_key(notify_db, notify_db_session, service=sample_service) with notify_api.test_request_context(): with notify_api.test_client() as client: - auth_header = create_authorization_header() response = client.get( path='/service/{}/history'.format(sample_service.id), @@ -1078,12 +1069,12 @@ def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notif ] ) def test_get_all_notifications_for_service_including_ones_made_by_jobs( - client, - notify_db, - notify_db_session, - sample_service, - include_from_test_key, - expected_count_of_notifications + client, + notify_db, + notify_db_session, + sample_service, + include_from_test_key, + expected_count_of_notifications ): with_job = sample_notification_with_job(notify_db, notify_db_session, service=sample_service) without_job = create_sample_notification(notify_db, notify_db_session, service=sample_service) @@ -1108,10 +1099,10 @@ def test_get_all_notifications_for_service_including_ones_made_by_jobs( def test_get_only_api_created_notifications_for_service( - client, - notify_db, - notify_db_session, - sample_service + client, + notify_db, + notify_db_session, + sample_service ): with_job = sample_notification_with_job(notify_db, notify_db_session, service=sample_service) without_job = create_sample_notification(notify_db, notify_db_session, service=sample_service) @@ -1211,19 +1202,19 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s @pytest.mark.parametrize( 'url, expected_status, expected_json', [ ( - '/service/{}/notifications/monthly?year=2001', - 200, - {'data': {'foo': 'bar'}}, + '/service/{}/notifications/monthly?year=2001', + 200, + {'data': {'foo': 'bar'}}, ), ( - '/service/{}/notifications/monthly?year=baz', - 400, - {'message': 'Year must be a number', 'result': 'error'}, + '/service/{}/notifications/monthly?year=baz', + 400, + {'message': 'Year must be a number', 'result': 'error'}, ), ( - '/service/{}/notifications/monthly', - 400, - {'message': 'Year must be a number', 'result': 'error'}, + '/service/{}/notifications/monthly', + 400, + {'message': 'Year must be a number', 'result': 'error'}, ), ] ) @@ -1452,11 +1443,11 @@ def test_get_notification_billable_unit_count_missing_year(client, sample_servic ('?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, + client, + sample_service, + query_string, + expected_status, + expected_json, ): response = client.get( '/service/{}/fragment/aggregate_statistics{}'.format(sample_service.id, query_string), @@ -1497,11 +1488,11 @@ def test_get_template_stats_by_month_returns_correct_data(notify_db, notify_db_s ('?year=abcd', 400, {'message': 'Year must be a number', 'result': 'error'}), ]) def test_get_template_stats_by_month_returns_error_for_incorrect_year( - client, - sample_service, - query_string, - expected_status, - expected_json + client, + sample_service, + query_string, + expected_status, + expected_json ): response = client.get( '/service/{}/notifications/templates/monthly{}'.format(sample_service.id, query_string), @@ -1729,3 +1720,79 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan assert resp.status_code == 200 assert not send_notification_mock.called + + +def test_get_yearly_billing_usage_count_returns_400_if_missing_year(client, sample_service): + response = client.get( + '/service/{}/yearly-usage-count'.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_count_returns_400_if_invalid_year(client, sample_service, mocker): + redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=None) + redis_set_mock = mocker.patch('app.service.rest.redis_store.set') + + response = client.get( + '/service/{}/yearly-usage-count?year=HAHAHAHAH'.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' + } + redis_get_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id))) + redis_set_mock.assert_not_called() + + +def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sample_service, mocker): + redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=None) + redis_set_mock = mocker.patch('app.service.rest.redis_store.set') + + start = datetime.utcnow() + end = datetime.utcnow() + timedelta(minutes=10) + mock_query = mocker.patch( + 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=100 + ) + mock_year = mocker.patch('app.service.rest.get_financial_year', return_value=(start, end)) + response = client.get( + '/service/{}/yearly-usage-count?year=2016'.format(sample_service.id), + headers=[create_authorization_header()] + ) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == { + 'billable_sms_units': 100 + } + mock_query.assert_called_once_with(start, end, sample_service.id) + mock_year.assert_called_once_with(2016) + redis_get_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id))) + redis_set_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id)), 100, ex=60) + + +def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sample_service, mocker): + redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=50) + redis_set_mock = mocker.patch('app.service.rest.redis_store.set') + mock_query = mocker.patch( + 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=50 + ) + + start = datetime.utcnow() + end = datetime.utcnow() + timedelta(minutes=10) + mock_year = mocker.patch('app.service.rest.get_financial_year', return_value=(start, end)) + + response = client.get( + '/service/{}/yearly-usage-count?year=2016'.format(sample_service.id), + headers=[create_authorization_header()] + ) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == { + 'billable_sms_units': 50 + } + redis_get_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id))) + mock_year.assert_not_called() + mock_query.assert_not_called() + redis_set_mock.assert_not_called() From 119f0532ab135ee59506f6d6889fd6ed8c5bfc88 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Mon, 22 May 2017 10:06:34 +0100 Subject: [PATCH 04/15] Renamed the API method/url --- app/service/rest.py | 8 +++--- tests/app/dao/test_notification_usage_dao.py | 1 - tests/app/service/test_rest.py | 26 ++++++++++---------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 423d5dc9b..f52b18174 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -444,13 +444,13 @@ def get_monthly_template_stats(service_id): raise InvalidRequest('Year must be a number', status_code=400) -@service_blueprint.route('//yearly-usage-count') -def get_yearly_usage_count(service_id): +@service_blueprint.route('//yearly-sms-billable-units') +def get_yearly_sms_billable_units(service_id): try: cache_key = sms_billable_units_cache_key(service_id) cached_value = redis_store.get(cache_key) if cached_value: - return jsonify({'billable_sms_units': cached_value}) + return jsonify({'billable_sms_units': int(cached_value)}) else: start_date, end_date = get_financial_year(int(request.args.get('year'))) billable_units = get_total_billable_units_for_sent_sms_notifications_in_date_range( @@ -460,7 +460,7 @@ def get_yearly_usage_count(service_id): redis_store.set(cache_key, billable_units, ex=60) return jsonify({'billable_sms_units': billable_units}) - except (ValueError, TypeError): + except (ValueError, TypeError) as e: return jsonify(result='error', message='No valid year provided'), 400 diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 81cbd0202..7dfd4bdfb 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -438,4 +438,3 @@ def test_returns_zero_if_no_matching_rows_when_returning_billable_units_for_sms_ start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 0 - diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index c7ad9495c..57d152b9b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1202,19 +1202,19 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s @pytest.mark.parametrize( 'url, expected_status, expected_json', [ ( - '/service/{}/notifications/monthly?year=2001', - 200, - {'data': {'foo': 'bar'}}, + '/service/{}/notifications/monthly?year=2001', + 200, + {'data': {'foo': 'bar'}}, ), ( - '/service/{}/notifications/monthly?year=baz', - 400, - {'message': 'Year must be a number', 'result': 'error'}, + '/service/{}/notifications/monthly?year=baz', + 400, + {'message': 'Year must be a number', 'result': 'error'}, ), ( - '/service/{}/notifications/monthly', - 400, - {'message': 'Year must be a number', 'result': 'error'}, + '/service/{}/notifications/monthly', + 400, + {'message': 'Year must be a number', 'result': 'error'}, ), ] ) @@ -1724,7 +1724,7 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan def test_get_yearly_billing_usage_count_returns_400_if_missing_year(client, sample_service): response = client.get( - '/service/{}/yearly-usage-count'.format(sample_service.id), + '/service/{}/yearly-sms-billable-units'.format(sample_service.id), headers=[create_authorization_header()] ) assert response.status_code == 400 @@ -1738,7 +1738,7 @@ def test_get_yearly_billing_usage_count_returns_400_if_invalid_year(client, samp redis_set_mock = mocker.patch('app.service.rest.redis_store.set') response = client.get( - '/service/{}/yearly-usage-count?year=HAHAHAHAH'.format(sample_service.id), + '/service/{}/yearly-sms-billable-units?year=HAHAHAHAH'.format(sample_service.id), headers=[create_authorization_header()] ) assert response.status_code == 400 @@ -1760,7 +1760,7 @@ def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sam ) mock_year = mocker.patch('app.service.rest.get_financial_year', return_value=(start, end)) response = client.get( - '/service/{}/yearly-usage-count?year=2016'.format(sample_service.id), + '/service/{}/yearly-sms-billable-units?year=2016'.format(sample_service.id), headers=[create_authorization_header()] ) assert response.status_code == 200 @@ -1785,7 +1785,7 @@ def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sa mock_year = mocker.patch('app.service.rest.get_financial_year', return_value=(start, end)) response = client.get( - '/service/{}/yearly-usage-count?year=2016'.format(sample_service.id), + '/service/{}/yearly-sms-billable-units?year=2016'.format(sample_service.id), headers=[create_authorization_header()] ) assert response.status_code == 200 From 35af759f8736deb15dd06fe4ae356b01ca070cbf Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 23 May 2017 13:54:51 +0100 Subject: [PATCH 05/15] Adding rates to the billable units query --- app/dao/notification_usage_dao.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index f4c233468..35d3646e4 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -157,6 +157,9 @@ def rate_multiplier(): @statsd(namespace="dao") def get_total_billable_units_for_sent_sms_notifications_in_date_range(start_date, end_date, service_id): + rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + print(rates) + result = db.session.query( func.sum( NotificationHistory.billable_units * func.coalesce(NotificationHistory.rate_multiplier, 1) From daa6d2d6f2ab63b5886c6262ef21c30eb7a5b5bf Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 08:55:59 +0100 Subject: [PATCH 06/15] No redis in dev mode --- app/config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/app/config.py b/app/config.py index 4bdec85b6..4e09c3aaa 100644 --- a/app/config.py +++ b/app/config.py @@ -204,6 +204,7 @@ class Config(object): ###################### class Development(Config): + REDIS_ENABLED = False SQLALCHEMY_ECHO = False NOTIFY_EMAIL_DOMAIN = 'notify.tools' CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' From 0bb289a1f2115ee4682a78246be0e10d7ea9d9f3 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 08:56:34 +0100 Subject: [PATCH 07/15] Redis enable via config --- app/config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/config.py b/app/config.py index 4e09c3aaa..4bdec85b6 100644 --- a/app/config.py +++ b/app/config.py @@ -204,7 +204,6 @@ class Config(object): ###################### class Development(Config): - REDIS_ENABLED = False SQLALCHEMY_ECHO = False NOTIFY_EMAIL_DOMAIN = 'notify.tools' CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' From 9dd604194404d461ba06d8994c206b2a9e46dd82 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 08:56:59 +0100 Subject: [PATCH 08/15] Usage DAO can now return rates and billable amount, alongside units. --- app/dao/notification_usage_dao.py | 78 ++++++-- tests/app/dao/test_notification_usage_dao.py | 193 +++++++++++++++++-- 2 files changed, 242 insertions(+), 29 deletions(-) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 35d3646e4..f40c6fe8b 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -157,20 +157,66 @@ def rate_multiplier(): @statsd(namespace="dao") def get_total_billable_units_for_sent_sms_notifications_in_date_range(start_date, end_date, service_id): - rates = get_rates_for_year(start_date, end_date, SMS_TYPE) - print(rates) - result = db.session.query( - func.sum( - NotificationHistory.billable_units * func.coalesce(NotificationHistory.rate_multiplier, 1) - ).label('billable_units') - ).filter( - NotificationHistory.service_id == service_id, - NotificationHistory.notification_type == 'sms', - NotificationHistory.created_at >= start_date, - NotificationHistory.created_at <= end_date, - NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE) - ) - if result.scalar(): - return int(result.scalar()) - return 0 + billable_units = 0 + total_cost = 0.0 + + rate_boundaries = discover_rate_bounds_for_billing_query(start_date, end_date) + for rate_boundary in rate_boundaries: + result = db.session.query( + func.sum( + NotificationHistory.billable_units * func.coalesce(NotificationHistory.rate_multiplier, 1) + ).label('billable_units') + ).filter( + NotificationHistory.service_id == service_id, + NotificationHistory.notification_type == 'sms', + NotificationHistory.created_at >= rate_boundary['start_date'], + NotificationHistory.created_at < rate_boundary['end_date'], + NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE) + ) + billable_units_by_rate_boundry = result.scalar() + if billable_units_by_rate_boundry: + billable_units += int(billable_units_by_rate_boundry)vi end_date + total_cost += int(billable_units_by_rate_boundry) * rate_boundary['rate'] + + return billable_units, total_cost + + +def discover_rate_bounds_for_billing_query(start_date, end_date): + bounds = [] + rates = get_rates_for_year(start_date, end_date, SMS_TYPE) + + def current_valid_from(index): + return rates[index].valid_from + + def next_valid_from(index): + return rates[index + 1].valid_from + + def current_rate(index): + return rates[index].rate + + def append_rate(rate_start_date, rate_end_date, rate): + bounds.append({ + 'start_date': rate_start_date, + 'end_date': rate_end_date, + 'rate': rate + }) + + if len(rates) == 1: + append_rate(start_date, end_date, current_rate(0)) + return bounds + + for i in range(len(rates)): + # first boundary + if i == 0: + append_rate(start_date, next_valid_from(i), current_rate(i)) + + # last boundary + elif i == (len(rates) - 1): + append_rate(current_valid_from(i), end_date, current_rate(i)) + + # other boundaries + else: + append_rate(current_valid_from(i), next_valid_from(i), current_rate(i)) + + return bounds diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index 7dfd4bdfb..c1c668d4c 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -4,11 +4,19 @@ from datetime import datetime, timedelta import pytest from app.dao.date_util import get_financial_year -from app.dao.notification_usage_dao import (get_rates_for_year, get_yearly_billing_data, - get_monthly_billing_data, - get_total_billable_units_for_sent_sms_notifications_in_date_range) -from app.models import Rate, NOTIFICATION_STATUS_SUCCESS, NOTIFICATION_DELIVERED, NOTIFICATION_STATUS_TYPES_BILLABLE, \ - NOTIFICATION_CREATED, NOTIFICATION_STATUS_TYPES_NON_BILLABLE +from app.dao.notification_usage_dao import ( + get_rates_for_year, + get_yearly_billing_data, + get_monthly_billing_data, + get_total_billable_units_for_sent_sms_notifications_in_date_range, + discover_rate_bounds_for_billing_query +) +from app.models import ( + Rate, + NOTIFICATION_DELIVERED, + NOTIFICATION_STATUS_TYPES_BILLABLE, + NOTIFICATION_STATUS_TYPES_NON_BILLABLE, + Notification) from tests.app.conftest import sample_notification, sample_email_template, sample_letter_template, sample_service from tests.app.db import create_notification from freezegun import freeze_time @@ -258,6 +266,8 @@ def set_up_rate(notify_db, start_date, value): @freeze_time("2016-01-10 12:00:00.000000") def test_returns_total_billable_units_for_sms_notifications(notify_db, notify_db_session, sample_service): + set_up_rate(notify_db, datetime(2016, 1, 1), 0.016) + sample_notification( notify_db, notify_db_session, service=sample_service, billable_units=1, status=NOTIFICATION_DELIVERED) sample_notification( @@ -270,13 +280,16 @@ def test_returns_total_billable_units_for_sms_notifications(notify_db, notify_db start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 10 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 10 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 0.16 @freeze_time("2016-01-10 12:00:00.000000") def test_returns_total_billable_units_multiplied_by_multipler_for_sms_notifications( notify_db, notify_db_session, sample_service ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + sample_notification( notify_db, notify_db_session, service=sample_service, rate_multiplier=1.0, status=NOTIFICATION_DELIVERED) sample_notification( @@ -289,13 +302,94 @@ def test_returns_total_billable_units_multiplied_by_multipler_for_sms_notificati start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 18 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 18 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 45 + + +def test_returns_total_billable_units_multiplied_by_multipler_for_sms_notifications_for_several_rates( + notify_db, notify_db_session, sample_service +): + set_up_rate(notify_db, datetime(2016, 1, 1), 2) + set_up_rate(notify_db, datetime(2016, 10, 1), 4) + set_up_rate(notify_db, datetime(2017, 1, 1), 6) + + eligble_rate_1 = datetime(2016, 2, 1) + eligble_rate_2 = datetime(2016, 11, 1) + eligble_rate_3 = datetime(2017, 2, 1) + + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + rate_multiplier=1.0, + status=NOTIFICATION_DELIVERED, + created_at=eligble_rate_1) + + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + rate_multiplier=2.0, + status=NOTIFICATION_DELIVERED, + created_at=eligble_rate_2) + + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + rate_multiplier=5.0, + status=NOTIFICATION_DELIVERED, + created_at=eligble_rate_3) + + start = datetime(2016, 1, 1) + end = datetime(2018, 1, 1) + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 8 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 40 + + +def test_returns_total_billable_units_for_sms_notifications_for_several_rates_where_dates_match_rate_boundary( + notify_db, notify_db_session, sample_service +): + set_up_rate(notify_db, datetime(2016, 1, 1), 2) + set_up_rate(notify_db, datetime(2016, 10, 1), 4) + set_up_rate(notify_db, datetime(2017, 1, 1), 6) + + eligble_rate_1_start = datetime(2016, 1, 1, 0, 0, 0, 0) + eligble_rate_1_end = datetime(2016, 9, 30, 23, 59, 59, 999) + eligble_rate_2_start = datetime(2016, 10, 1, 0, 0, 0, 0) + eligble_rate_2_end = datetime(2016, 12, 31, 23, 59, 59, 999) + eligble_rate_3_start = datetime(2017, 1, 1, 0, 0, 0, 0) + eligble_rate_3_whenever = datetime(2017, 12, 12, 0, 0, 0, 0) + + def make_notification(created_at): + sample_notification( + notify_db, + notify_db_session, + service=sample_service, + rate_multiplier=1.0, + status=NOTIFICATION_DELIVERED, + created_at=created_at) + + make_notification(eligble_rate_1_start) + make_notification(eligble_rate_1_end) + make_notification(eligble_rate_2_start) + make_notification(eligble_rate_2_end) + make_notification(eligble_rate_3_start) + make_notification(eligble_rate_3_whenever) + + start = datetime(2016, 1, 1) + end = datetime(2018, 1, 1) + + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 6 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 24.0 @freeze_time("2016-01-10 12:00:00.000000") def test_returns_total_billable_units_for_sms_notifications_ignoring_letters_and_emails( notify_db, notify_db_session, sample_service ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + email_template = sample_email_template(notify_db, notify_db_session, service=sample_service) letter_template = sample_letter_template(sample_service) @@ -324,13 +418,16 @@ def test_returns_total_billable_units_for_sms_notifications_ignoring_letters_and start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 2 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 2 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 5 @freeze_time("2016-01-10 12:00:00.000000") def test_returns_total_billable_units_for_sms_notifications_for_only_requested_service( notify_db, notify_db_session ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + service_1 = sample_service(notify_db, notify_db_session, service_name=str(uuid.uuid4())) service_2 = sample_service(notify_db, notify_db_session, service_name=str(uuid.uuid4())) service_3 = sample_service(notify_db, notify_db_session, service_name=str(uuid.uuid4())) @@ -358,13 +455,16 @@ def test_returns_total_billable_units_for_sms_notifications_for_only_requested_s start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, service_1.id) == 2 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, service_1.id)[0] == 2 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, service_1.id)[1] == 5 @freeze_time("2016-01-10 12:00:00.000000") def test_returns_total_billable_units_for_sms_notifications_handling_null_values( notify_db, notify_db_session, sample_service ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + sample_notification( notify_db, notify_db_session, @@ -376,7 +476,8 @@ def test_returns_total_billable_units_for_sms_notifications_handling_null_values start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 2 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 2 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 5 @pytest.mark.parametrize('billable_units, states', ([ @@ -387,6 +488,8 @@ def test_returns_total_billable_units_for_sms_notifications_handling_null_values def test_ignores_non_billable_states_when_returning_billable_units_for_sms_notifications( notify_db, notify_db_session, sample_service, billable_units, states ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + for state in states: sample_notification( notify_db, @@ -401,13 +504,18 @@ def test_ignores_non_billable_states_when_returning_billable_units_for_sms_notif assert get_total_billable_units_for_sent_sms_notifications_in_date_range( start, end, sample_service.id - ) == billable_units + )[0] == billable_units + assert get_total_billable_units_for_sent_sms_notifications_in_date_range( + start, end, sample_service.id + )[1] == billable_units * 2.5 @freeze_time("2016-01-10 12:00:00.000000") def test_restricts_to_time_period_when_returning_billable_units_for_sms_notifications( notify_db, notify_db_session, sample_service ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + sample_notification( notify_db, notify_db_session, @@ -429,12 +537,71 @@ def test_restricts_to_time_period_when_returning_billable_units_for_sms_notifica start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 1 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 1 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 2.5 def test_returns_zero_if_no_matching_rows_when_returning_billable_units_for_sms_notifications( notify_db, notify_db_session, sample_service ): + set_up_rate(notify_db, datetime(2016, 1, 1), 2.5) + start = datetime.utcnow() - timedelta(minutes=10) end = datetime.utcnow() + timedelta(minutes=10) - assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id) == 0 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[0] == 0 + assert get_total_billable_units_for_sent_sms_notifications_in_date_range(start, end, sample_service.id)[1] == 0.0 + + +def test_should_calculate_rate_boundaries_for_billing_query_for_single_relevant_rate(notify_db, notify_db_session): + start_date, end_date = get_financial_year(2016) + set_up_rate(notify_db, datetime(2016, 1, 1), 0.016) + rate_boundaries = discover_rate_bounds_for_billing_query(start_date, end_date) + print(rate_boundaries) + assert len(rate_boundaries) == 1 + assert rate_boundaries[0]['start_date'] == start_date + assert rate_boundaries[0]['end_date'] == end_date + assert rate_boundaries[0]['rate'] == 0.016 + + +def test_should_calculate_rate_boundaries_for_billing_query_for_two_relevant_rates(notify_db, notify_db_session): + start_date, end_date = get_financial_year(2016) + + rate_1_valid_from = datetime(2016, 1, 1) + rate_2_valid_from = datetime(2017, 1, 1) + + set_up_rate(notify_db, rate_1_valid_from, 0.02) + set_up_rate(notify_db, rate_2_valid_from, 0.04) + rate_boundaries = discover_rate_bounds_for_billing_query(start_date, end_date) + assert len(rate_boundaries) == 2 + assert rate_boundaries[0]['start_date'] == start_date + assert rate_boundaries[0]['end_date'] == rate_2_valid_from + assert rate_boundaries[0]['rate'] == 0.02 + + assert rate_boundaries[1]['start_date'] == rate_2_valid_from + assert rate_boundaries[1]['end_date'] == end_date + assert rate_boundaries[1]['rate'] == 0.04 + + +def test_should_calculate_rate_boundaries_for_billing_query_for_three_relevant_rates(notify_db, notify_db_session): + start_date, end_date = get_financial_year(2016) + rate_1_valid_from = datetime(2016, 1, 1) + rate_2_valid_from = datetime(2017, 1, 1) + rate_3_valid_from = datetime(2017, 2, 1) + + set_up_rate(notify_db, rate_1_valid_from, 0.02) + set_up_rate(notify_db, rate_2_valid_from, 0.04) + set_up_rate(notify_db, rate_3_valid_from, 0.06) + rate_boundaries = discover_rate_bounds_for_billing_query(start_date, end_date) + assert len(rate_boundaries) == 3 + + assert rate_boundaries[0]['start_date'] == start_date + assert rate_boundaries[0]['end_date'] == rate_2_valid_from + assert rate_boundaries[0]['rate'] == 0.02 + + assert rate_boundaries[1]['start_date'] == rate_2_valid_from + assert rate_boundaries[1]['end_date'] == rate_3_valid_from + assert rate_boundaries[1]['rate'] == 0.04 + + assert rate_boundaries[2]['start_date'] == rate_3_valid_from + assert rate_boundaries[2]['end_date'] == end_date + assert rate_boundaries[2]['rate'] == 0.06 From 511e143ace9b5f41d55273ca705fe5368d50369f Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 08:57:11 +0100 Subject: [PATCH 09/15] toString on the rates object --- app/models.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models.py b/app/models.py index 66fa6a75c..79a873e95 100644 --- a/app/models.py +++ b/app/models.py @@ -1069,6 +1069,12 @@ class Rate(db.Model): rate = db.Column(db.Float(asdecimal=False), nullable=False) notification_type = db.Column(notification_types, index=True, nullable=False) + def __str__(self): + the_string = "{}".format(self.rate) + the_string += " {}".format(self.notification_type) + the_string += " {}".format(self.valid_from) + return the_string + class JobStatistics(db.Model): __tablename__ = 'job_statistics' From 78a55bafe0f5d3e4b405a25622d235ec122f7d1f Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 08:57:41 +0100 Subject: [PATCH 10/15] Added new cost field to yearly billable sms endpoint. --- app/service/rest.py | 17 ++++++++++++----- tests/app/service/test_rest.py | 13 ++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index f52b18174..ad0c39d12 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -448,19 +448,26 @@ def get_monthly_template_stats(service_id): def get_yearly_sms_billable_units(service_id): try: cache_key = sms_billable_units_cache_key(service_id) - cached_value = redis_store.get(cache_key) - if cached_value: - return jsonify({'billable_sms_units': int(cached_value)}) + cached_billable_sms_units = redis_store.get(cache_key) + if cached_billable_sms_units: + return jsonify({ + 'billable_sms_units': int(cached_billable_sms_units[0]), + 'total_cost': float(cached_billable_sms_units[1]) + }) else: start_date, end_date = get_financial_year(int(request.args.get('year'))) - billable_units = get_total_billable_units_for_sent_sms_notifications_in_date_range( + billable_units, total_cost = get_total_billable_units_for_sent_sms_notifications_in_date_range( start_date, end_date, service_id) redis_store.set(cache_key, billable_units, ex=60) - return jsonify({'billable_sms_units': billable_units}) + return jsonify({ + 'billable_sms_units': billable_units, + 'total_cost': total_cost + }) except (ValueError, TypeError) as e: + print(e) return jsonify(result='error', message='No valid year provided'), 400 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4d9882e82..4231de249 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1758,7 +1758,7 @@ def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sam start = datetime.utcnow() end = datetime.utcnow() + timedelta(minutes=10) mock_query = mocker.patch( - 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=100 + 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=(100, 200.0) ) mock_year = mocker.patch('app.service.rest.get_financial_year', return_value=(start, end)) response = client.get( @@ -1767,7 +1767,8 @@ def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sam ) assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == { - 'billable_sms_units': 100 + 'billable_sms_units': 100, + 'total_cost': 200.0 } mock_query.assert_called_once_with(start, end, sample_service.id) mock_year.assert_called_once_with(2016) @@ -1776,10 +1777,10 @@ def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sam def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sample_service, mocker): - redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=50) + redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=(50, 100.0)) redis_set_mock = mocker.patch('app.service.rest.redis_store.set') mock_query = mocker.patch( - 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=50 + 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=(50, 100.0) ) start = datetime.utcnow() @@ -1790,9 +1791,11 @@ def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sa '/service/{}/yearly-sms-billable-units?year=2016'.format(sample_service.id), headers=[create_authorization_header()] ) + print(response.get_data(as_text=True)) assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == { - 'billable_sms_units': 50 + 'billable_sms_units': 50, + 'total_cost': 100.0 } redis_get_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id))) mock_year.assert_not_called() From 0db8297693c8e24bb359866c3d8c12f981e04869 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 09:59:07 +0100 Subject: [PATCH 11/15] Removed print statement --- tests/app/dao/test_notification_usage_dao.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/dao/test_notification_usage_dao.py b/tests/app/dao/test_notification_usage_dao.py index c1c668d4c..d4aec8531 100644 --- a/tests/app/dao/test_notification_usage_dao.py +++ b/tests/app/dao/test_notification_usage_dao.py @@ -556,7 +556,6 @@ def test_should_calculate_rate_boundaries_for_billing_query_for_single_relevant_ start_date, end_date = get_financial_year(2016) set_up_rate(notify_db, datetime(2016, 1, 1), 0.016) rate_boundaries = discover_rate_bounds_for_billing_query(start_date, end_date) - print(rate_boundaries) assert len(rate_boundaries) == 1 assert rate_boundaries[0]['start_date'] == start_date assert rate_boundaries[0]['end_date'] == end_date From 03346f467f214235948aadff04f76e58164d2c92 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 09:59:37 +0100 Subject: [PATCH 12/15] updated cache to store map not single value, to accommodate the billable units and the total cost. --- app/service/rest.py | 48 +++++++++++++++++++--------------- tests/app/service/test_rest.py | 21 ++++++++++----- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index ad0c39d12..e8c585d1f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -446,29 +446,35 @@ def get_monthly_template_stats(service_id): @service_blueprint.route('//yearly-sms-billable-units') def get_yearly_sms_billable_units(service_id): - try: - cache_key = sms_billable_units_cache_key(service_id) - cached_billable_sms_units = redis_store.get(cache_key) - if cached_billable_sms_units: - return jsonify({ - 'billable_sms_units': int(cached_billable_sms_units[0]), - 'total_cost': float(cached_billable_sms_units[1]) - }) - else: + cache_key = sms_billable_units_cache_key(service_id) + cached_billable_sms_units = redis_store.get_all_from_hash(cache_key) + if cached_billable_sms_units: + return jsonify({ + 'billable_sms_units': int(cached_billable_sms_units[b'billable_units']), + 'total_cost': float(cached_billable_sms_units[b'total_cost']) + }) + else: + try: start_date, end_date = get_financial_year(int(request.args.get('year'))) - billable_units, total_cost = get_total_billable_units_for_sent_sms_notifications_in_date_range( - start_date, - end_date, - service_id) - redis_store.set(cache_key, billable_units, ex=60) - return jsonify({ - 'billable_sms_units': billable_units, - 'total_cost': total_cost - }) + except (ValueError, TypeError) as e: + current_app.logger.exception(e) + return jsonify(result='error', message='No valid year provided'), 400 - except (ValueError, TypeError) as e: - print(e) - return jsonify(result='error', message='No valid year provided'), 400 + billable_units, total_cost = get_total_billable_units_for_sent_sms_notifications_in_date_range( + start_date, + end_date, + service_id) + + cached_values = { + 'billable_units': billable_units, + 'total_cost': total_cost + } + + redis_store.set_hash_and_expire(cache_key, cached_values, expire_in_seconds=60) + return jsonify({ + 'billable_sms_units': billable_units, + 'total_cost': total_cost + }) @service_blueprint.route('//yearly-usage') diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4231de249..691f0895c 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1736,8 +1736,8 @@ def test_get_yearly_billing_usage_count_returns_400_if_missing_year(client, samp def test_get_yearly_billing_usage_count_returns_400_if_invalid_year(client, sample_service, mocker): - redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=None) - redis_set_mock = mocker.patch('app.service.rest.redis_store.set') + redis_get_mock = mocker.patch('app.service.rest.redis_store.get_all_from_hash', return_value=None) + redis_set_mock = mocker.patch('app.service.rest.redis_store.set_hash_and_expire') response = client.get( '/service/{}/yearly-sms-billable-units?year=HAHAHAHAH'.format(sample_service.id), @@ -1752,8 +1752,8 @@ def test_get_yearly_billing_usage_count_returns_400_if_invalid_year(client, samp def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sample_service, mocker): - redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=None) - redis_set_mock = mocker.patch('app.service.rest.redis_store.set') + redis_get_mock = mocker.patch('app.service.rest.redis_store.get_all_from_hash', return_value=None) + redis_set_mock = mocker.patch('app.service.rest.redis_store.set_hash_and_expire') start = datetime.utcnow() end = datetime.utcnow() + timedelta(minutes=10) @@ -1773,12 +1773,19 @@ def test_get_yearly_billing_usage_count_returns_200_if_year_provided(client, sam mock_query.assert_called_once_with(start, end, sample_service.id) mock_year.assert_called_once_with(2016) redis_get_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id))) - redis_set_mock.assert_called_once_with("{}-sms_billable_units".format(str(sample_service.id)), 100, ex=60) + redis_set_mock.assert_called_once_with( + "{}-sms_billable_units".format(str(sample_service.id)), + {'billable_units': 100, 'total_cost': 200.0}, + expire_in_seconds=60 + ) def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sample_service, mocker): - redis_get_mock = mocker.patch('app.service.rest.redis_store.get', return_value=(50, 100.0)) - redis_set_mock = mocker.patch('app.service.rest.redis_store.set') + redis_get_mock = mocker.patch( + 'app.service.rest.redis_store.get_all_from_hash', + return_value={b'total_cost': 100.0, b'billable_units': 50} + ) + redis_set_mock = mocker.patch('app.service.rest.redis_store.set_hash_and_expire') mock_query = mocker.patch( 'app.service.rest.get_total_billable_units_for_sent_sms_notifications_in_date_range', return_value=(50, 100.0) ) From 517dc6be8b04509f0dcba609c75a218c1ba75080 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 09:59:51 +0100 Subject: [PATCH 13/15] Typo removed. Wrong window focus --- app/dao/notification_usage_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index f40c6fe8b..b3b582ed3 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -176,7 +176,7 @@ def get_total_billable_units_for_sent_sms_notifications_in_date_range(start_date ) billable_units_by_rate_boundry = result.scalar() if billable_units_by_rate_boundry: - billable_units += int(billable_units_by_rate_boundry)vi end_date + billable_units += int(billable_units_by_rate_boundry) total_cost += int(billable_units_by_rate_boundry) * rate_boundary['rate'] return billable_units, total_cost From 390935e82c2051078bf61399ed23f6473714ff59 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 10:49:34 +0100 Subject: [PATCH 14/15] Bumped utils version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index fe25533c1..4cc9907bb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,6 +28,6 @@ notifications-python-client>=3.1,<3.2 awscli>=1.11,<1.12 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@17.1.0#egg=notifications-utils==17.1.0 +git+https://github.com/alphagov/notifications-utils.git@17.1.3#egg=notifications-utils==17.1.3 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 From c29f95381e8ed202409e69a09c712f3f261fa116 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 1 Jun 2017 14:57:46 +0100 Subject: [PATCH 15/15] Remved test re-added as part of a merge --- tests/app/service/test_rest.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4ce78fcee..f2b0a8247 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2002,22 +2002,6 @@ def test_get_yearly_billing_usage_count_returns_from_cache_if_present(client, sa redis_set_mock.assert_not_called() -def test_update_service_works_when_sms_sender_is_null(sample_service, client, mocker): - sample_service.sms_sender = None - data = {'name': 'new name'} - - resp = client.post( - 'service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[create_authorization_header()], - content_type='application/json' - ) - - assert resp.status_code == 200 - # make sure it wasn't changed to not-null under the hood - assert sample_service.sms_sender is None - - def test_search_for_notification_by_to_field_filters_by_status(client, notify_db, notify_db_session): create_notification = partial( create_sample_notification,