From 313f669690bd0be16872ade7817f5f4e9eb6f97b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 6 Jun 2017 12:57:55 +0100 Subject: [PATCH] Roll up messages in inbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inbox should work a bit like the one on your phone. You shouldn’t see all the messages, but the latest one from each of your ‘contacts’ only. --- app/__init__.py | 7 ++++++- app/main/views/dashboard.py | 16 +++++++++++++++- app/templates/views/dashboard/inbox.html | 7 ++++--- requirements.txt | 2 +- tests/app/main/views/test_dashboard.py | 13 ++++++++----- tests/conftest.py | 4 ++-- 6 files changed, 36 insertions(+), 13 deletions(-) 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..02292b400 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), ) 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..b05299f01 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -131,11 +131,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 +159,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( diff --git a/tests/conftest.py b/tests/conftest.py index 958aac94d..71d6e8588 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1139,9 +1139,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',