Merge pull request #2010 from alphagov/one-off-fixxx

Add pagination to inbox page
This commit is contained in:
Chris Waszczuk
2018-04-09 10:30:24 +01:00
committed by GitHub
7 changed files with 137 additions and 64 deletions

View File

@@ -105,7 +105,7 @@ def get_sms_thread(service_id, user_number):
notification_api_client.get_notifications_for_service(service_id,
to=user_number,
template_type='sms')['notifications'] +
service_api_client.get_inbound_sms(service_id, user_number=user_number)
service_api_client.get_inbound_sms(service_id, user_number=user_number)['data']
), key=lambda notification: notification['created_at']):
is_inbound = ('notify_number' in notification)

View File

@@ -12,7 +12,6 @@ from flask import (
url_for,
)
from flask_login import login_required
from notifications_utils.recipients import format_phone_number_human_readable
from werkzeug.utils import redirect
from app import (
@@ -31,6 +30,8 @@ from app.utils import (
FAILURE_STATUSES,
REQUESTED_STATUSES,
Spreadsheet,
generate_next_dict,
generate_previous_dict,
get_current_financial_year,
user_has_permissions,
)
@@ -195,7 +196,7 @@ def inbox(service_id):
return render_template(
'views/dashboard/inbox.html',
partials=get_inbox_partials(service_id),
updates_url=url_for('.inbox_updates', service_id=service_id),
updates_url=url_for('.inbox_updates', service_id=service_id, page=request.args.get('page')),
)
@@ -221,7 +222,7 @@ def inbox_download(service_id):
message['user_number'],
message['content'].lstrip(('=+-@')),
format_datetime_numeric(message['created_at']),
] for message in service_api_client.get_inbound_sms(service_id)]
] for message in service_api_client.get_inbound_sms(service_id)['data']]
).as_csv_data,
mimetype='text/csv',
headers={
@@ -233,33 +234,32 @@ def inbox_download(service_id):
def get_inbox_partials(service_id):
page = int(request.args.get('page', 1))
if 'inbound_sms' not in current_service['permissions']:
abort(403)
inbound_messages = service_api_client.get_inbound_sms(service_id)
messages_to_show = {}
# get the most recent message for each number
for message in inbound_messages:
human_readable = format_phone_number_human_readable(message['user_number'])
if human_readable not in messages_to_show:
messages_to_show[human_readable] = message
count_of_users = len(messages_to_show)
messages_to_show = sorted(messages_to_show.values(), key=lambda x: x['created_at'], reverse=True)
inbound_messages_data = service_api_client.get_most_recent_inbound_sms(service_id, page=page)
inbound_messages = inbound_messages_data['data']
if not inbound_messages:
inbound_number = inbound_number_client.get_inbound_sms_number_for_service(service_id)['data']['number']
else:
inbound_number = None
prev_page = None
if page > 1:
prev_page = generate_previous_dict('main.inbox', service_id, page)
next_page = None
if inbound_messages_data['has_next']:
next_page = generate_next_dict('main.inbox', service_id, page)
return {'messages': render_template(
'views/dashboard/_inbox_messages.html',
messages=list(messages_to_show),
count_of_messages=len(inbound_messages),
count_of_users=count_of_users,
messages=inbound_messages,
inbound_number=inbound_number,
prev_page=prev_page,
next_page=next_page
)}

View File

@@ -261,13 +261,26 @@ class ServiceAPIClient(NotifyAdminAPIClient):
def update_whitelist(self, service_id, data):
return self.put(url='/service/{}/whitelist'.format(service_id), data=data)
def get_inbound_sms(self, service_id, user_number=''):
def get_inbound_sms(self, service_id, user_number='', page=None):
return self.get(
'/service/{}/inbound-sms?user_number={}'.format(
'/service/{}/inbound-sms'.format(
service_id,
user_number,
)
)['data']
),
params={
'user_number': user_number,
'page': page
}
)
def get_most_recent_inbound_sms(self, service_id, page=None):
return self.get(
'/service/{}/inbound-sms/most-recent'.format(
service_id,
),
params={
'page': page
}
)
def get_inbound_sms_by_id(self, service_id, notification_id):
return self.get(

View File

@@ -1,4 +1,5 @@
{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading, row_heading %}
{% from "components/previous-next-navigation.html" import previous_next_navigation %}
{% from "components/message-count-label.html" import message_count_label %}
<div class="ajax-block-container">
@@ -33,10 +34,7 @@
</span>
{% endcall %}
{% endcall %}
{% if messages %}
<p class="table-show-more-link">
{{ 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 %}
{{ previous_next_navigation(prev_page, next_page) }}
</div>

View File

@@ -196,13 +196,16 @@ def test_view_conversation_with_empty_inbound(
):
mock_get_inbound_sms = mocker.patch(
'app.main.views.conversation.service_api_client.get_inbound_sms',
return_value=[{
'user_number': '07900000001',
'notify_number': '07900000002',
'content': '',
'created_at': datetime.utcnow().isoformat(),
'id': fake_uuid
}]
return_value={
'has_next': False,
'data': [{
'user_number': '07900000001',
'notify_number': '07900000002',
'content': '',
'created_at': datetime.utcnow().isoformat(),
'id': fake_uuid
}]
}
)
page = client_request.get(

View File

@@ -164,10 +164,13 @@ def test_inbound_messages_shows_count_of_messages(
@pytest.mark.parametrize('index, expected_row', enumerate([
'07900 900000 message-1 1 hour ago',
'07900 900000 message-2 1 hour ago',
'07900 900000 message-3 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',
'07900 900008 message-8 9 hours ago',
]))
def test_inbox_showing_inbound_messages(
logged_in_client,
@@ -177,7 +180,7 @@ def test_inbox_showing_inbound_messages(
mock_get_detailed_service,
mock_get_template_statistics,
mock_get_usage,
mock_get_inbound_sms,
mock_get_most_recent_inbound_sms,
index,
expected_row,
):
@@ -189,17 +192,35 @@ def test_inbox_showing_inbound_messages(
assert response.status_code == 200
rows = page.select('tbody tr')
assert len(rows) == 5
assert len(rows) == 8
assert normalize_spaces(rows[index].text) == expected_row
assert normalize_spaces(page.select('.table-show-more-link')) == (
'8 messages from 5 users'
)
assert page.select_one('a[download]')['href'] == url_for(
'main.inbox_download',
service_id=SERVICE_ONE_ID,
)
def test_get_inbound_sms_shows_page_links(
logged_in_client,
service_one,
mock_get_service_templates_when_no_templates_exist,
mock_get_jobs,
mock_get_detailed_service,
mock_get_template_statistics,
mock_get_usage,
mock_get_most_recent_inbound_sms,
mock_get_inbound_number_for_service,
):
service_one['permissions'] = ['inbound_sms']
response = logged_in_client.get(url_for('main.inbox', service_id=SERVICE_ONE_ID, page=2))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert 'Next page' in page.find('li', {'class': 'next-page'}).text
assert 'Previous page' in page.find('li', {'class': 'previous-page'}).text
def test_empty_inbox(
logged_in_client,
service_one,
@@ -208,7 +229,7 @@ def test_empty_inbox(
mock_get_detailed_service,
mock_get_template_statistics,
mock_get_usage,
mock_get_inbound_sms_with_no_messages,
mock_get_most_recent_inbound_sms_with_no_messages,
mock_get_inbound_number_for_service,
):
@@ -222,6 +243,8 @@ def test_empty_inbox(
'When users text your services phone number (0781239871) youll see the messages here'
)
assert not page.select('a[download]')
assert not page.select('li.next-page')
assert not page.select('li.previous-page')
@pytest.mark.parametrize('endpoint', [
@@ -244,7 +267,7 @@ def test_anyone_can_see_inbox(
api_user_active,
service_one,
mocker,
mock_get_inbound_sms_with_no_messages,
mock_get_most_recent_inbound_sms_with_no_messages,
mock_get_inbound_number_for_service,
):
@@ -266,7 +289,7 @@ def test_view_inbox_updates(
logged_in_client,
service_one,
mocker,
mock_get_inbound_sms_with_no_messages,
mock_get_most_recent_inbound_sms_with_no_messages,
):
mock_get_partials = mocker.patch(
@@ -333,13 +356,16 @@ def test_download_inbox_strips_formulae(
mocker.patch(
'app.service_api_client.get_inbound_sms',
return_value=[{
'user_number': 'elevenchars',
'notify_number': 'foo',
'content': message_content,
'created_at': datetime.utcnow().isoformat(),
'id': fake_uuid,
}],
return_value={
'has_next': False,
'data': [{
'user_number': 'elevenchars',
'notify_number': 'foo',
'content': message_content,
'created_at': datetime.utcnow().isoformat(),
'id': fake_uuid,
}]
},
)
response = logged_in_client.get(
url_for('main.inbox_download', service_id=SERVICE_ONE_ID)

View File

@@ -1862,14 +1862,18 @@ def mock_get_inbound_sms(mocker):
def _get_inbound_sms(
service_id,
user_number=None,
page=1
):
return [{
'user_number': '0790090000' + str(i),
'notify_number': '07900000002',
'content': 'message-{}'.format(index + 1),
'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1), seconds=index)).isoformat(),
'id': sample_uuid(),
} for index, i in enumerate([0, 0, 0, 2, 4, 6, 8, 8])]
return {
'has_next': True,
'data': [{
'user_number': '0790090000' + str(i),
'notify_number': '07900000002',
'content': 'message-{}'.format(index + 1),
'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1), seconds=index)).isoformat(),
'id': sample_uuid(),
} for index, i in enumerate([0, 0, 0, 2, 4, 6, 8, 8])]
}
return mocker.patch(
'app.service_api_client.get_inbound_sms',
@@ -1878,15 +1882,44 @@ def mock_get_inbound_sms(mocker):
@pytest.fixture(scope='function')
def mock_get_inbound_sms_with_no_messages(mocker):
def _get_inbound_sms(
def mock_get_most_recent_inbound_sms(mocker):
def _get_most_recent_inbound_sms(
service_id,
user_number=None,
page=1
):
return []
return {
'has_next': True,
'data': [{
'user_number': '0790090000' + str(i),
'notify_number': '07900000002',
'content': 'message-{}'.format(index + 1),
'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1), seconds=index)).isoformat(),
'id': sample_uuid(),
} for index, i in enumerate([0, 0, 0, 2, 4, 6, 8, 8])]
}
return mocker.patch(
'app.service_api_client.get_inbound_sms',
side_effect=_get_inbound_sms,
'app.service_api_client.get_most_recent_inbound_sms',
side_effect=_get_most_recent_inbound_sms,
)
@pytest.fixture(scope='function')
def mock_get_most_recent_inbound_sms_with_no_messages(mocker):
def _get_most_recent_inbound_sms(
service_id,
user_number=None,
page=1
):
return {
'has_next': False,
'data': []
}
return mocker.patch(
'app.service_api_client.get_most_recent_inbound_sms',
side_effect=_get_most_recent_inbound_sms,
)