diff --git a/app/__init__.py b/app/__init__.py index 795987bd5..ef10702ba 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -29,7 +29,11 @@ from functools import partial from notifications_python_client.errors import HTTPError from notifications_utils import logging, request_id, formatters from notifications_utils.clients.statsd.statsd_client import StatsdClient -from notifications_utils.recipients import validate_phone_number, InvalidPhoneError +from notifications_utils.recipients import ( + validate_phone_number, + InvalidPhoneError, + format_phone_number_human_readable, +) from notifications_utils.formatters import formatted_list from pygments import highlight from pygments.formatters.html import HtmlFormatter @@ -139,6 +143,7 @@ def create_app(): application.add_template_filter(format_notification_status_as_url) application.add_template_filter(formatted_list) application.add_template_filter(nl2br) + application.add_template_filter(format_phone_number_human_readable) application.after_request(useful_headers_after_request) application.after_request(save_service_after_request) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index b78fadc13..4efd8272f 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -10,6 +10,8 @@ from flask import ( ) from flask_login import login_required +from notifications_utils.recipients import format_phone_number_human_readable + from app.main import main from app import ( current_service, @@ -144,9 +146,21 @@ def inbox(service_id): if 'inbound_sms' not in current_service['permissions']: abort(403) + messages_to_show = list() + inbound_messages = service_api_client.get_inbound_sms(service_id) + + for message in inbound_messages: + if format_phone_number_human_readable(message['user_number']) not in { + format_phone_number_human_readable(message['user_number']) + for message in messages_to_show + }: + messages_to_show.append(message) + return render_template( 'views/dashboard/inbox.html', - messages=service_api_client.get_inbound_sms(service_id), + messages=messages_to_show, + count_of_messages=len(inbound_messages), + count_of_users=len(messages_to_show), ) @@ -208,10 +222,10 @@ 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_free_tier_usage(service_api_client.get_yearly_sms_unit_count_and_cost( service_id, get_current_financial_year(), - )) + ), service) ), } @@ -223,6 +237,16 @@ def get_dashboard_totals(statistics): return statistics +def calculate_free_tier_usage(usage, service): + sms_free_allowance = service['data']['free_sms_fragment_limit'] + 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/main/views/feedback.py b/app/main/views/feedback.py index 775d79878..6f1631a2f 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -8,6 +8,11 @@ from app.main.forms import SupportType, Feedback, Problem, Triage from datetime import datetime +@main.route('/feedback', methods=['GET']) +def old_feedback(): + return redirect(url_for('.support')) + + @main.route('/support', methods=['GET', 'POST']) def support(): form = SupportType() diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index b5c82656c..da9bb9d34 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -223,6 +223,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/app/templates/views/dashboard/inbox.html b/app/templates/views/dashboard/inbox.html index ace1cafbd..6d5612bb4 100644 --- a/app/templates/views/dashboard/inbox.html +++ b/app/templates/views/dashboard/inbox.html @@ -4,7 +4,7 @@ {% extends "withnav_template.html" %} {% block service_page_title %} - Inbox + Received text messages {% endblock %} {% block maincolumn_content %} @@ -25,7 +25,7 @@ field_headings_visible=False ) %} {% call field() %} - {{ item.user_number }} + {{ item.user_number | format_phone_number_human_readable }} {{ item.content }} {% endcall %} {% call field(align='right') %} @@ -36,7 +36,8 @@ {% endcall %} {% if messages %}

- Showing all {{ messages | length }} messages + {{ count_of_messages }} message{{ '' if 1 == count_of_messages else 's' }} + from {{ count_of_users }} user{{ '' if 1 == count_of_users else 's' }}

{% endif %}
diff --git a/requirements.txt b/requirements.txt index 98ed8f5bf..03158b9fd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,4 @@ 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.2#egg=notifications-utils==17.1.2 +git+https://github.com/alphagov/notifications-utils.git@17.2.0#egg=notifications-utils==17.2.0 diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 152d0a9d0..28c29a63d 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -50,6 +50,7 @@ def test_get_started( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + 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)) @@ -69,6 +70,7 @@ def test_get_started_is_hidden_once_templates_exist( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + 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)) @@ -88,6 +90,7 @@ def test_inbound_messages_not_visible_to_service_without_permissions( mock_get_template_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): service_one['permissions'] = [] @@ -115,6 +118,7 @@ def test_inbound_messages_shows_count_of_messages( mock_get_usage, inbound_summary_mock, expected_text, + mock_get_yearly_sms_unit_count_and_cost ): service_one['permissions'] = ['inbound_sms'] @@ -131,11 +135,11 @@ def test_inbound_messages_shows_count_of_messages( @pytest.mark.parametrize('index, expected_row', enumerate([ - '07900900000 foo 1 hour ago', - '07900900001 foo 2 hours ago', - '07900900002 foo 3 hours ago', - '07900900003 foo 4 hours ago', - '07900900004 foo 5 hours ago', + '07900 900000 message-1 1 hour ago', + '07900 900002 message-4 3 hours ago', + '07900 900004 message-5 5 hours ago', + '07900 900006 message-6 7 hours ago', + '07900 900008 message-7 9 hours ago', ])) def test_inbox_showing_inbound_messages( logged_in_client, @@ -159,6 +163,9 @@ def test_inbox_showing_inbound_messages( rows = page.select('tbody tr') assert len(rows) == 5 assert normalize_spaces(rows[index].text) == expected_row + assert normalize_spaces(page.select('.table-show-more-link')) == ( + '8 messages from 5 users' + ) def test_empty_inbox( @@ -201,6 +208,7 @@ def test_should_show_recent_templates_on_dashboard( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + 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)) @@ -267,6 +275,7 @@ def test_should_show_upcoming_jobs_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -298,6 +307,7 @@ def test_should_show_recent_jobs_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -417,6 +427,7 @@ def test_menu_send_messages( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -448,6 +459,7 @@ def test_menu_manage_service( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -478,6 +490,7 @@ def test_menu_manage_api_keys( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -508,6 +521,7 @@ def test_menu_all_services_for_platform_admin_user( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -538,6 +552,7 @@ def test_route_for_service_permissions( mock_get_detailed_service, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): validate_route_permission( @@ -575,6 +590,7 @@ def test_service_dashboard_updates_gets_dashboard_totals( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + 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}, @@ -800,6 +816,7 @@ def test_should_show_all_jobs_with_valid_statuses( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost ): logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -816,3 +833,45 @@ def test_should_show_all_jobs_with_valid_statuses( 'ready to send', 'sent to dvla' }) + + +def test_should_show_remaining_free_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_free_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_feedback.py b/tests/app/main/views/test_feedback.py index 6b4bfa0f2..97e1ae0d7 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -17,9 +17,18 @@ def no_redirect(): return lambda _external=True: None -def test_get_support_index_page(client): - resp = client.get(url_for('main.support')) - assert resp.status_code == 200 +@pytest.mark.parametrize('endpoint', [ + 'main.old_feedback', + 'main.support', +]) +def test_get_support_index_page( + client, + endpoint, +): + response = client.get(url_for('main.support'), follow_redirects=True) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Support' @freeze_time('2016-12-12 12:00:00.000000') diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 3b550f377..c17f5fa4a 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, mock_get_inbound_sms_summary, ): diff --git a/tests/conftest.py b/tests/conftest.py index 958aac94d..e8dffef64 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -92,6 +92,7 @@ def mock_get_detailed_service(mocker, api_user_active): return { 'data': { 'id': service_id, + 'free_sms_fragment_limit': 250000, 'statistics': { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} @@ -109,6 +110,7 @@ def mock_get_detailed_service_for_today(mocker, api_user_active): return { 'data': { 'id': service_id, + 'free_sms_fragment_limit': 250000, 'statistics': { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} @@ -1139,9 +1141,9 @@ def mock_get_inbound_sms(mocker): ): return [{ 'user_number': '0790090000' + str(i), - 'content': 'foo', + 'content': 'message-{}'.format(index + 1), 'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1))).isoformat() - } for i in range(5)] + } for index, i in enumerate([0, 0, 0, 2, 4, 6, 8, 8])] return mocker.patch( 'app.service_api_client.get_inbound_sms', @@ -1398,6 +1400,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):