Merge branch 'master' of https://github.com/alphagov/notifications-admin

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)

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

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()

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))
 

app/templates/views/dashboard/_usage.html

@@ -10,7 +10,7 @@
     <div class="keyline-block">
       {% if sms_chargeable %}
         {{ big_number(
-          (sms_chargeable * sms_rate),
+          total_sms_cost,
           'spent on text messages',
           currency="£",
           smaller=True

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() %}
-        <span class="file-list-filename" href="#">{{ item.user_number }}</span>
+        <span class="file-list-filename" href="#">{{ item.user_number | format_phone_number_human_readable }}</span>
         <span class="wide-left-hand-column">{{ item.content }}</span>
       {% endcall %}
       {% call field(align='right') %}
@@ -36,7 +36,8 @@
     {% endcall %}
     {% if messages %}
       <p class="table-show-more-link">
-        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' }}
       </p>
     {% endif %}
   </div>

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

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)

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')

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,
 ):

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):