From 52326539d6f2b6c31473c7b039a5946b1754a00b Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 24 May 2017 10:35:52 +0100 Subject: [PATCH 1/4] Wire in the new API method that calculates the total cost and total billable units. - Used on dashboard to calculate free tier/cost - Update tests to mock new method - Two new tests to check output on dashboard page --- app/main/views/dashboard.py | 14 +++++- app/notify_client/service_api_client.py | 6 +++ app/templates/views/dashboard/_usage.html | 2 +- tests/app/main/views/test_dashboard.py | 54 +++++++++++++++++++++++ tests/app/main/views/test_sign_out.py | 1 + tests/conftest.py | 9 ++++ 6 files changed, 84 insertions(+), 2 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 2d59d822f..d0869fdff 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -186,7 +186,7 @@ def get_dashboard_partials(service_id): 'has_jobs': bool(immediate_jobs), 'usage': render_template( 'views/dashboard/_usage.html', - **calculate_usage(service_api_client.get_service_usage( + **calculate_fee_tier_usage(service_api_client.get_yearly_sms_unit_count_and_cost( service_id, get_current_financial_year(), )) @@ -201,6 +201,18 @@ def get_dashboard_totals(statistics): return statistics +def calculate_fee_tier_usage(usage): + # TODO: Don't hardcode these - get em from the API + sms_free_allowance = 250000 + + return({ + 'sms_chargeable': max(0, usage['billable_sms_units'] - sms_free_allowance), + 'total_sms_bill': usage['billable_sms_units'], + 'total_sms_cost': usage['total_cost'], + 'sms_allowance_remaining': sms_free_allowance - int(usage['billable_sms_units']) + }) + + def calculate_usage(usage): # TODO: Don't hardcode these - get em from the API sms_free_allowance = 250000 diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index dc48d0043..fd672a464 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -221,6 +221,12 @@ class ServiceAPIClient(NotifyAdminAPIClient): params=dict(year=year) ) + def get_yearly_sms_unit_count_and_cost(self, service_id, year=None): + return self.get( + '/service/{0}/yearly-sms-billable-units'.format(service_id), + params=dict(year=year) + ) + def get_monthly_notification_stats(self, service_id, year): return self.get(url='/service/{}/notifications/monthly?year={}'.format(service_id, year)) diff --git a/app/templates/views/dashboard/_usage.html b/app/templates/views/dashboard/_usage.html index ff0530549..7a908d53c 100644 --- a/app/templates/views/dashboard/_usage.html +++ b/app/templates/views/dashboard/_usage.html @@ -10,7 +10,7 @@
{% if sms_chargeable %} {{ big_number( - (sms_chargeable * sms_rate), + total_sms_cost, 'spent on text messages', currency="£", smaller=True diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index b6c8b9404..731e5df03 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -44,6 +44,7 @@ def test_get_started( mock_get_jobs, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', return_value=copy.deepcopy(stub_template_stats)) @@ -62,6 +63,7 @@ def test_get_started_is_hidden_once_templates_exist( mock_get_jobs, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', return_value=copy.deepcopy(stub_template_stats)) @@ -79,6 +81,7 @@ def test_should_show_recent_templates_on_dashboard( mock_get_jobs, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', return_value=copy.deepcopy(stub_template_stats)) @@ -144,6 +147,7 @@ def test_should_show_upcoming_jobs_on_dashboard( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -174,6 +178,7 @@ def test_should_show_recent_jobs_on_dashboard( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -292,6 +297,7 @@ def test_menu_send_messages( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -322,6 +328,7 @@ def test_menu_manage_service( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -351,6 +358,7 @@ def test_menu_manage_api_keys( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -380,6 +388,7 @@ def test_menu_all_services_for_platform_admin_user( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -409,6 +418,7 @@ def test_route_for_service_permissions( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): validate_route_permission( @@ -445,6 +455,7 @@ def test_service_dashboard_updates_gets_dashboard_totals( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): mocker.patch('app.main.views.dashboard.get_dashboard_totals', return_value={ 'email': {'requested': 123, 'delivered': 0, 'failed': 0}, @@ -668,6 +679,7 @@ def test_should_show_all_jobs_with_valid_statuses( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_yearly_sms_unit_count_and_cost ): get_dashboard_partials(service_id=SERVICE_ONE_ID) @@ -684,3 +696,45 @@ def test_should_show_all_jobs_with_valid_statuses( 'ready to send', 'sent to dvla' }) + + +def test_should_show_remaining_fee_tier_count( + logged_in_client, + mock_get_service_templates, + mock_get_template_statistics, + mock_get_detailed_service, + mock_get_jobs, + mock_get_usage, + mocker +): + mocker.patch( + 'app.service_api_client.get_yearly_sms_unit_count_and_cost', + return_value={"billable_sms_units": 100, "total_cost": 200.0} + ) + + response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) + + assert response.status_code == 200 + assert '249,900' in response.get_data(as_text=True) + assert 'free text messages left' in response.get_data(as_text=True) + + +def test_should_show_cost_if_exceeded_fee_tier_count( + logged_in_client, + mock_get_service_templates, + mock_get_template_statistics, + mock_get_detailed_service, + mock_get_jobs, + mock_get_usage, + mocker +): + mocker.patch( + 'app.service_api_client.get_yearly_sms_unit_count_and_cost', + return_value={"billable_sms_units": 300000, "total_cost": 1500.50} + ) + + response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) + + assert response.status_code == 200 + assert '£1,500.50' in response.get_data(as_text=True) + assert 'spent on text messages' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index d4dc88b46..9c7a21f04 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -23,6 +23,7 @@ def test_sign_out_user( mock_has_permissions, mock_get_template_statistics, mock_get_detailed_service, + mock_get_yearly_sms_unit_count_and_cost, mock_get_usage, ): with logged_in_client.session_transaction() as session: diff --git a/tests/conftest.py b/tests/conftest.py index 37dda3ed3..29a1969f8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1332,6 +1332,15 @@ def mock_get_usage(mocker, service_one, fake_uuid): 'app.service_api_client.get_service_usage', side_effect=_get_usage) +@pytest.fixture(scope='function') +def mock_get_yearly_sms_unit_count_and_cost(mocker, service_one, fake_uuid): + def _get_usage(service_id, year=None): + return {"billable_sms_units": 100, "total_cost": 200.0} + + return mocker.patch( + 'app.service_api_client.get_yearly_sms_unit_count_and_cost', side_effect=_get_usage) + + @pytest.fixture(scope='function') def mock_get_billable_units(mocker): def _get_usage(service_id, year): From 74e79e175b70f6438c5a445f3fdd0abdace0e880 Mon Sep 17 00:00:00 2001 From: minglis Date: Thu, 1 Jun 2017 16:53:36 +0100 Subject: [PATCH 2/4] Fix typo --- tests/app/main/views/test_dashboard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 731e5df03..a6ff4e6a8 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -698,7 +698,7 @@ def test_should_show_all_jobs_with_valid_statuses( }) -def test_should_show_remaining_fee_tier_count( +def test_should_show_remaining_free_tier_count( logged_in_client, mock_get_service_templates, mock_get_template_statistics, From 8f70dcf549cc8ee49e0191cd85a70efabd97b456 Mon Sep 17 00:00:00 2001 From: minglis Date: Thu, 1 Jun 2017 16:55:49 +0100 Subject: [PATCH 3/4] Fix typo --- tests/app/main/views/test_dashboard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index a6ff4e6a8..cda5b9f21 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -719,7 +719,7 @@ def test_should_show_remaining_free_tier_count( assert 'free text messages left' in response.get_data(as_text=True) -def test_should_show_cost_if_exceeded_fee_tier_count( +def test_should_show_cost_if_exceeded_free_tier_count( logged_in_client, mock_get_service_templates, mock_get_template_statistics, From e236cc4cbe48541374d7aa1ae265fd67610e58fb Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 1 Jun 2017 17:04:41 +0100 Subject: [PATCH 4/4] Fixed typo and moved the sms free limit into the config. --- app/config.py | 2 ++ app/main/views/dashboard.py | 11 +++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/config.py b/app/config.py index 53eb1e8ac..32cbe76d4 100644 --- a/app/config.py +++ b/app/config.py @@ -59,6 +59,8 @@ class Config(object): ACTIVITY_STATS_LIMIT_DAYS = 7 TEST_MESSAGE_FILENAME = 'Test message' + SMS_FREE_TIER_AMOUNT = 250000 + STATSD_ENABLED = False STATSD_HOST = "statsd.hostedgraphite.com" STATSD_PORT = 8125 diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index d0869fdff..9091a2036 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -2,6 +2,7 @@ from datetime import datetime from functools import partial from flask import ( render_template, + current_app, url_for, session, jsonify, @@ -186,7 +187,7 @@ def get_dashboard_partials(service_id): 'has_jobs': bool(immediate_jobs), 'usage': render_template( 'views/dashboard/_usage.html', - **calculate_fee_tier_usage(service_api_client.get_yearly_sms_unit_count_and_cost( + **calculate_free_tier_usage(service_api_client.get_yearly_sms_unit_count_and_cost( service_id, get_current_financial_year(), )) @@ -201,9 +202,8 @@ def get_dashboard_totals(statistics): return statistics -def calculate_fee_tier_usage(usage): - # TODO: Don't hardcode these - get em from the API - sms_free_allowance = 250000 +def calculate_free_tier_usage(usage): + sms_free_allowance = current_app.config['SMS_FREE_TIER_AMOUNT'] return({ 'sms_chargeable': max(0, usage['billable_sms_units'] - sms_free_allowance), @@ -214,8 +214,7 @@ def calculate_fee_tier_usage(usage): def calculate_usage(usage): - # TODO: Don't hardcode these - get em from the API - sms_free_allowance = 250000 + sms_free_allowance = current_app.config['SMS_FREE_TIER_AMOUNT'] sms_rate = 0 if len(usage) == 0 else usage[0].get("rate", 0) sms_sent = get_sum_billing_units(breakdown for breakdown in usage if breakdown['notification_type'] == 'sms')