From 6153389e01f205bdd896381f6625939c9cb08ee4 Mon Sep 17 00:00:00 2001 From: Chris Waszczuk <31617728+gov-cjwaszczuk@users.noreply.github.com> Date: Wed, 4 Apr 2018 14:18:03 +0100 Subject: [PATCH] Revert "add pagination to inbox page" --- app/main/views/conversation.py | 2 +- app/main/views/dashboard.py | 21 ++-------- app/notify_client/service_api_client.py | 13 +++--- .../views/dashboard/_inbox_messages.html | 4 -- tests/app/main/views/test_conversation.py | 17 ++++---- tests/app/main/views/test_dashboard.py | 40 ++++--------------- tests/conftest.py | 25 ++++-------- 7 files changed, 32 insertions(+), 90 deletions(-) diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index 1aa958ca3..d3207e649 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -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)['data'] + service_api_client.get_inbound_sms(service_id, user_number=user_number) ), key=lambda notification: notification['created_at']): is_inbound = ('notify_number' in notification) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 1f9e89814..c1508fa59 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -31,8 +31,6 @@ from app.utils import ( FAILURE_STATUSES, REQUESTED_STATUSES, Spreadsheet, - generate_next_dict, - generate_previous_dict, get_current_financial_year, user_has_permissions, ) @@ -195,7 +193,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, page=request.args.get('page')), + updates_url=url_for('.inbox_updates', service_id=service_id), ) @@ -221,7 +219,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)['data']] + ] for message in service_api_client.get_inbound_sms(service_id)] ).as_csv_data, mimetype='text/csv', headers={ @@ -233,12 +231,11 @@ 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_data = service_api_client.get_inbound_sms(service_id, page=page) - inbound_messages = inbound_messages_data['data'] + inbound_messages = service_api_client.get_inbound_sms(service_id) messages_to_show = {} # get the most recent message for each number @@ -255,22 +252,12 @@ def get_inbox_partials(service_id): 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, inbound_number=inbound_number, - prev_page=prev_page, - next_page=next_page - )} diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 2ca439ead..5635dcb5f 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -261,16 +261,13 @@ 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='', page=None): + def get_inbound_sms(self, service_id, user_number=''): return self.get( - '/service/{}/inbound-sms'.format( + '/service/{}/inbound-sms?user_number={}'.format( service_id, - ), - params={ - 'user_number': user_number, - 'page': page - } - ) + user_number, + ) + )['data'] def get_inbound_sms_by_id(self, service_id, notification_id): return self.get( diff --git a/app/templates/views/dashboard/_inbox_messages.html b/app/templates/views/dashboard/_inbox_messages.html index 1a1a3108e..bba1e741d 100644 --- a/app/templates/views/dashboard/_inbox_messages.html +++ b/app/templates/views/dashboard/_inbox_messages.html @@ -1,5 +1,4 @@ {% 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 %}
@@ -40,7 +39,4 @@ from {{ count_of_users }} user{{ '' if 1 == count_of_users else 's' }}

{% endif %} - - {{ previous_next_navigation(prev_page, next_page) }}
- diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index c8d7df5f9..fbd24fbcf 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -196,16 +196,13 @@ 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={ - 'has_next': False, - 'data': [{ - 'user_number': '07900000001', - 'notify_number': '07900000002', - 'content': '', - 'created_at': datetime.utcnow().isoformat(), - 'id': fake_uuid - }] - } + return_value=[{ + 'user_number': '07900000001', + 'notify_number': '07900000002', + 'content': '', + 'created_at': datetime.utcnow().isoformat(), + 'id': fake_uuid + }] ) page = client_request.get( diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 556549dbe..47cb58fe3 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -200,27 +200,6 @@ def test_inbox_showing_inbound_messages( ) -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_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, @@ -243,8 +222,6 @@ def test_empty_inbox( 'When users text your service’s phone number (0781239871) you’ll 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', [ @@ -356,16 +333,13 @@ def test_download_inbox_strips_formulae( mocker.patch( 'app.service_api_client.get_inbound_sms', - return_value={ - 'has_next': False, - 'data': [{ - 'user_number': 'elevenchars', - 'notify_number': 'foo', - 'content': message_content, - 'created_at': datetime.utcnow().isoformat(), - 'id': fake_uuid, - }] - }, + return_value=[{ + '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) diff --git a/tests/conftest.py b/tests/conftest.py index b90ab28a4..e4b16f647 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1862,18 +1862,14 @@ def mock_get_inbound_sms(mocker): def _get_inbound_sms( service_id, user_number=None, - page=1 ): - 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 [{ + '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', @@ -1885,13 +1881,8 @@ def mock_get_inbound_sms(mocker): def mock_get_inbound_sms_with_no_messages(mocker): def _get_inbound_sms( service_id, - user_number=None, - page=1 ): - return { - 'has_next': False, - 'data': [] - } + return [] return mocker.patch( 'app.service_api_client.get_inbound_sms',