From a74da6d14ed8503d9adf5f7636d81f9be94a8c77 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 26 Apr 2019 16:47:16 +0100 Subject: [PATCH 1/2] Show monthly usage by SMS provider on Providers page This adds a '% monthly traffic' column to the SMS table on the Providers page to let us see how much traffic is being sent to each SMS provider. --- app/main/views/providers.py | 13 +++++ app/templates/views/providers/providers.html | 4 +- tests/app/main/views/test_providers.py | 50 ++++++++++++++++---- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/app/main/views/providers.py b/app/main/views/providers.py index c1edda49a..09b6558bb 100644 --- a/app/main/views/providers.py +++ b/app/main/views/providers.py @@ -22,6 +22,8 @@ def view_providers(): elif provider['notification_type'] == 'email': domestic_email_providers.append(provider) + add_monthly_traffic(domestic_sms_providers) + return render_template( 'views/providers/providers.html', email_providers=domestic_email_providers, @@ -30,6 +32,17 @@ def view_providers(): ) +def add_monthly_traffic(domestic_sms_providers): + total_sms_sent = sum(provider.get('current_month_billable_sms', 0) for provider in domestic_sms_providers) + + for provider in domestic_sms_providers: + if provider.get('current_month_billable_sms'): + percentage = (provider['current_month_billable_sms'] / total_sms_sent * 100) if total_sms_sent else 0 + provider['monthly_traffic'] = round(percentage) + else: + provider['monthly_traffic'] = 0 + + @main.route("/provider//edit", methods=['GET', 'POST']) @login_required @user_is_platform_admin diff --git a/app/templates/views/providers/providers.html b/app/templates/views/providers/providers.html index b7320fa87..a2ea71b2c 100644 --- a/app/templates/views/providers/providers.html +++ b/app/templates/views/providers/providers.html @@ -17,7 +17,7 @@ Providers caption="Domestic SMS providers", caption_visible=False, empty_message='No domestic sms providers', - field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], + field_headings=['Provider', 'Priority', '% monthly traffic', 'Active', 'Last Updated', 'Updated By'], field_headings_visible=True ) %} @@ -25,6 +25,8 @@ Providers {{ text_field(item.priority) }} + {{ text_field(item.monthly_traffic) }} + {{ text_field(item.active) }} {% if item.updated_at %} diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py index 7b847f57d..618f8b557 100644 --- a/tests/app/main/views/test_providers.py +++ b/tests/app/main/views/test_providers.py @@ -6,6 +6,7 @@ from bs4 import BeautifulSoup from flask import url_for import app +from app.main.views.providers import add_monthly_traffic stub_providers = { 'provider_details': [ @@ -23,7 +24,8 @@ stub_providers = { 'name': 'Test User', 'id': '7cc1dddb-bcbc-4739-8fc1-61bedde3332a' }, - 'supports_international': False + 'supports_international': False, + 'current_month_billable_sms': 5020, }, { 'id': '0bd529cd-a0fd-43e5-80ee-b95ef6b0d51f', @@ -35,7 +37,8 @@ stub_providers = { 'updated_at': None, 'version': 1, 'created_by': None, - 'supports_international': False + 'supports_international': False, + 'current_month_billable_sms': 6891, }, { 'id': '6005e192-4738-4962-beec-ebd982d0b03a', @@ -170,10 +173,11 @@ def test_should_show_all_providers( assert table_data[0].find_all("a")[0]['href'] == '/provider/6005e192-4738-4962-beec-ebd982d0b03f' assert table_data[0].text.strip() == "Domestic SMS Provider" assert table_data[1].text.strip() == "1" - assert table_data[2].text.strip() == "True" - assert table_data[3].text.strip() == "16 January at 3:20pm" - assert table_data[4].text.strip() == "Test User" - assert table_data[5].find_all("a")[0]['href'] == '/provider/6005e192-4738-4962-beec-ebd982d0b03f/edit' + assert table_data[2].text.strip() == "42" + assert table_data[3].text.strip() == "True" + assert table_data[4].text.strip() == "16 January at 3:20pm" + assert table_data[5].text.strip() == "Test User" + assert table_data[6].find_all("a")[0]['href'] == '/provider/6005e192-4738-4962-beec-ebd982d0b03f/edit' domestic_sms_second_row = domestic_sms_table.tbody.find_all('tr')[1] table_data = domestic_sms_second_row.find_all('td') @@ -181,10 +185,11 @@ def test_should_show_all_providers( assert table_data[0].find_all("a")[0]['href'] == '/provider/0bd529cd-a0fd-43e5-80ee-b95ef6b0d51f' assert table_data[0].text.strip() == "Second Domestic SMS Provider" assert table_data[1].text.strip() == "2" - assert table_data[2].text.strip() == "True" - assert table_data[3].text.strip() == "None" + assert table_data[2].text.strip() == "58" + assert table_data[3].text.strip() == "True" assert table_data[4].text.strip() == "None" - assert table_data[5].find_all("a")[0]['href'] == '/provider/0bd529cd-a0fd-43e5-80ee-b95ef6b0d51f/edit' + assert table_data[5].text.strip() == "None" + assert table_data[6].find_all("a")[0]['href'] == '/provider/0bd529cd-a0fd-43e5-80ee-b95ef6b0d51f/edit' domestic_email_first_row = domestic_email_table.tbody.find_all('tr')[0] domestic_email_table_data = domestic_email_first_row.find_all('td') @@ -222,6 +227,33 @@ def test_should_show_all_providers( assert table_data[5].find_all("a")[0]['href'] == '/provider/67c770f5-918e-4afa-a5ff-880b9beb161d/edit' +def test_add_monthly_traffic(): + domestic_sms_providers = [{ + 'identifier': 'mmg', + 'current_month_billable_sms': 27, + }, { + 'identifier': 'firetext', + 'current_month_billable_sms': 5, + }, { + 'identifier': 'loadtesting', + }] + + add_monthly_traffic(domestic_sms_providers) + + assert domestic_sms_providers == [{ + 'identifier': 'mmg', + 'current_month_billable_sms': 27, + 'monthly_traffic': 84 + }, { + 'identifier': 'firetext', + 'current_month_billable_sms': 5, + 'monthly_traffic': 16 + }, { + 'identifier': 'loadtesting', + 'monthly_traffic': 0 + }] + + def test_should_show_edit_provider_form( client, platform_admin_user, From 259a82e06649f7620d52fc7d3a15b707d78ad962 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 2 May 2019 09:25:57 +0100 Subject: [PATCH 2/2] Update for new format of data returned from api - API will now send through `created_by_name` instead of `created_by`. - API will always send through `current_month_billable_sms` but this can now be `0` instead of `None`. --- app/main/views/providers.py | 9 +++------ app/templates/views/providers/providers.html | 20 ++++---------------- tests/app/main/views/test_providers.py | 18 ++++++++++-------- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/app/main/views/providers.py b/app/main/views/providers.py index 09b6558bb..3d3611d84 100644 --- a/app/main/views/providers.py +++ b/app/main/views/providers.py @@ -33,14 +33,11 @@ def view_providers(): def add_monthly_traffic(domestic_sms_providers): - total_sms_sent = sum(provider.get('current_month_billable_sms', 0) for provider in domestic_sms_providers) + total_sms_sent = sum(provider['current_month_billable_sms'] for provider in domestic_sms_providers) for provider in domestic_sms_providers: - if provider.get('current_month_billable_sms'): - percentage = (provider['current_month_billable_sms'] / total_sms_sent * 100) if total_sms_sent else 0 - provider['monthly_traffic'] = round(percentage) - else: - provider['monthly_traffic'] = 0 + percentage = (provider['current_month_billable_sms'] / total_sms_sent * 100) if total_sms_sent else 0 + provider['monthly_traffic'] = round(percentage) @main.route("/provider//edit", methods=['GET', 'POST']) diff --git a/app/templates/views/providers/providers.html b/app/templates/views/providers/providers.html index a2ea71b2c..027afc951 100644 --- a/app/templates/views/providers/providers.html +++ b/app/templates/views/providers/providers.html @@ -1,5 +1,5 @@ {% extends "views/platform-admin/_base_template.html" %} -{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading %} +{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, optional_text_field %} {% from "components/show-more.html" import show_more %} {% block per_page_title %} @@ -35,11 +35,7 @@ Providers {{ text_field('None') }} {% endif %} - {% if item.created_by %} - {{ text_field(item.created_by.name) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {{ optional_text_field(item.created_by_name, default='None') }} {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} @@ -68,11 +64,7 @@ Providers {{ text_field('None') }} {% endif %} - {% if item.created_by %} - {{ text_field(item.created_by.name) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {{ optional_text_field(item.created_by_name, default='None') }} {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} @@ -101,11 +93,7 @@ Providers {{ text_field('None') }} {% endif %} - {% if item.created_by %} - {{ text_field(item.created_by.name) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {{ optional_text_field(item.created_by_name, default='None') }} {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py index 618f8b557..ded40c2b2 100644 --- a/tests/app/main/views/test_providers.py +++ b/tests/app/main/views/test_providers.py @@ -19,11 +19,7 @@ stub_providers = { 'notification_type': 'sms', 'updated_at': datetime(2017, 1, 16, 15, 20, 40).isoformat(), 'version': 1, - 'created_by': { - 'email_address': 'test@foo.bar', - 'name': 'Test User', - 'id': '7cc1dddb-bcbc-4739-8fc1-61bedde3332a' - }, + 'created_by_name': 'Test User', 'supports_international': False, 'current_month_billable_sms': 5020, }, @@ -50,7 +46,8 @@ stub_providers = { 'updated_at': None, 'version': 1, 'created_by': None, - 'supports_international': False + 'supports_international': False, + 'current_month_billable_sms': 0, }, { 'active': True, @@ -62,7 +59,8 @@ stub_providers = { 'updated_at': None, 'version': 1, 'created_by': None, - 'supports_international': False + 'supports_international': False, + 'current_month_billable_sms': 0, }, { 'id': '67c770f5-918e-4afa-a5ff-880b9beb161d', @@ -74,7 +72,8 @@ stub_providers = { 'updated_at': None, 'version': 1, 'created_by': None, - 'supports_international': True + 'supports_international': True, + 'current_month_billable_sms': 0, }, { 'id': '67c770f5-918e-4afa-a5ff-880b9beb161d', @@ -86,6 +85,7 @@ stub_providers = { 'updated_at': None, 'version': 1, 'created_by': None, + 'current_month_billable_sms': 0, } ] } @@ -236,6 +236,7 @@ def test_add_monthly_traffic(): 'current_month_billable_sms': 5, }, { 'identifier': 'loadtesting', + 'current_month_billable_sms': 0, }] add_monthly_traffic(domestic_sms_providers) @@ -250,6 +251,7 @@ def test_add_monthly_traffic(): 'monthly_traffic': 16 }, { 'identifier': 'loadtesting', + 'current_month_billable_sms': 0, 'monthly_traffic': 0 }]